From f4efcacd40ce67483ee6a7f4d31b32cd9a408012 Mon Sep 17 00:00:00 2001 From: SteveSandersonMS Date: Mon, 18 Jul 2016 15:56:45 +0100 Subject: [PATCH] Switch to native .NET logging APIs --- .../Configuration/Configuration.cs | 30 ++++++++++++++++--- .../Configuration/NodeServicesOptions.cs | 4 +-- .../HostingModels/HttpNodeInstance.cs | 4 +-- .../HostingModels/OutOfProcessNodeInstance.cs | 22 +++++++------- .../HostingModels/SocketNodeInstance.cs | 8 ++--- .../Util/ConsoleNodeInstanceOutputLogger.cs | 17 ----------- .../Util/INodeInstanceOutputLogger.cs | 14 --------- .../project.json | 1 + 8 files changed, 46 insertions(+), 54 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.NodeServices/Util/ConsoleNodeInstanceOutputLogger.cs delete mode 100644 src/Microsoft.AspNetCore.NodeServices/Util/INodeInstanceOutputLogger.cs diff --git a/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs b/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs index 8d41762..bbae3d2 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs +++ b/src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs @@ -2,11 +2,15 @@ using System; using Microsoft.Extensions.DependencyInjection; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.NodeServices.HostingModels; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Console; namespace Microsoft.AspNetCore.NodeServices { public static class Configuration { + const string LogCategoryName = "Microsoft.AspNetCore.NodeServices"; + public static void AddNodeServices(this IServiceCollection serviceCollection) => AddNodeServices(serviceCollection, new NodeServicesOptions()); @@ -15,13 +19,24 @@ namespace Microsoft.AspNetCore.NodeServices serviceCollection.AddSingleton(typeof(INodeServices), serviceProvider => { // Since this instance is being created through DI, we can access the IHostingEnvironment - // to populate options.ProjectPath if it wasn't explicitly specified. - var hostEnv = serviceProvider.GetRequiredService(); + // to populate options.ProjectPath if it wasn't explicitly specified. if (string.IsNullOrEmpty(options.ProjectPath)) { + var hostEnv = serviceProvider.GetRequiredService(); options.ProjectPath = hostEnv.ContentRootPath; } + // Likewise, if no logger was specified explicitly, we should use the one from DI. + // If it doesn't provide one, CreateNodeInstance will set up a default. + if (options.NodeInstanceOutputLogger == null) + { + var loggerFactory = serviceProvider.GetService(); + if (loggerFactory != null) + { + options.NodeInstanceOutputLogger = loggerFactory.CreateLogger(LogCategoryName); + } + } + return new NodeServicesImpl(options, () => CreateNodeInstance(options)); }); } @@ -33,6 +48,13 @@ namespace Microsoft.AspNetCore.NodeServices private static INodeInstance CreateNodeInstance(NodeServicesOptions options) { + // If you've specified no logger, fall back on a default console logger + var logger = options.NodeInstanceOutputLogger; + if (logger == null) + { + logger = new ConsoleLogger(LogCategoryName, null, false); + } + if (options.NodeInstanceFactory != null) { // If you've explicitly supplied an INodeInstance factory, we'll use that. This is useful for @@ -46,10 +68,10 @@ namespace Microsoft.AspNetCore.NodeServices switch (options.HostingModel) { case NodeHostingModel.Http: - return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, /* port */ 0, options.NodeInstanceOutputLogger); + return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, logger, /* port */ 0); case NodeHostingModel.Socket: var pipeName = "pni-" + Guid.NewGuid().ToString("D"); // Arbitrary non-clashing string - return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, options.NodeInstanceOutputLogger); + return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, logger); default: throw new ArgumentException("Unknown hosting model: " + options.HostingModel); } diff --git a/src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs b/src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs index d20419f..98c50ec 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs +++ b/src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs @@ -1,6 +1,6 @@ using System; using Microsoft.AspNetCore.NodeServices.HostingModels; -using Microsoft.AspNetCore.NodeServices.Util; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.NodeServices { @@ -20,6 +20,6 @@ namespace Microsoft.AspNetCore.NodeServices public Func NodeInstanceFactory { get; set; } public string ProjectPath { get; set; } public string[] WatchFileExtensions { get; set; } - public INodeInstanceOutputLogger NodeInstanceOutputLogger { get; set; } + public ILogger NodeInstanceOutputLogger { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs index 53c50b5..53dabf9 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs @@ -4,7 +4,7 @@ using System.Net.Http; using System.Text; using System.Text.RegularExpressions; using System.Threading.Tasks; -using Microsoft.AspNetCore.NodeServices.Util; +using Microsoft.Extensions.Logging; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; @@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private bool _disposed; private int _portNumber; - public HttpNodeInstance(string projectPath, string[] watchFileExtensions, int port = 0, INodeInstanceOutputLogger nodeInstanceOutputLogger = null) + public HttpNodeInstance(string projectPath, string[] watchFileExtensions, ILogger nodeInstanceOutputLogger, int port = 0) : base( EmbeddedResourceReader.Read( typeof(HttpNodeInstance), diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs index 558ca07..0cba1cd 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs @@ -3,7 +3,7 @@ using System.Diagnostics; using System.IO; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.NodeServices.Util; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.NodeServices.HostingModels { @@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels /// public abstract class OutOfProcessNodeInstance : INodeInstance { + protected readonly ILogger OutputLogger; private const string ConnectionEstablishedMessage = "[Microsoft.AspNetCore.NodeServices:Listening]"; private readonly TaskCompletionSource _connectionIsReadySource = new TaskCompletionSource(); private bool _disposed; @@ -27,18 +28,20 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private readonly Process _nodeProcess; private bool _nodeProcessNeedsRestart; private readonly string[] _watchFileExtensions; - private INodeInstanceOutputLogger _nodeInstanceOutputLogger; public OutOfProcessNodeInstance( string entryPointScript, string projectPath, string[] watchFileExtensions, string commandLineArguments, - INodeInstanceOutputLogger nodeOutputLogger) + ILogger nodeOutputLogger) { - _nodeInstanceOutputLogger = nodeOutputLogger; - if(_nodeInstanceOutputLogger == null) - _nodeInstanceOutputLogger = new ConsoleNodeInstanceOutputLogger(); + if (nodeOutputLogger == null) + { + throw new ArgumentNullException(nameof(nodeOutputLogger)); + } + + OutputLogger = nodeOutputLogger; _entryPointScript = new StringAsTempFile(entryPointScript); var startInfo = PrepareNodeProcessStartInfo(_entryPointScript.FileName, projectPath, commandLineArguments); @@ -112,12 +115,12 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels protected virtual void OnOutputDataReceived(string outputData) { - _nodeInstanceOutputLogger.LogOutputData(outputData); + OutputLogger.LogInformation(outputData); } protected virtual void OnErrorDataReceived(string errorData) { - _nodeInstanceOutputLogger.LogErrorData(errorData); + OutputLogger.LogError(errorData); } protected virtual void Dispose(bool disposing) @@ -255,8 +258,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private void RestartDueToFileChange(string fullPath) { - // TODO: Use proper logger - Console.WriteLine($"Node will restart because file changed: {fullPath}"); + OutputLogger.LogInformation($"Node will restart because file changed: {fullPath}"); _nodeProcessNeedsRestart = true; diff --git a/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs b/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs index b6ba106..f5bbce1 100644 --- a/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs +++ b/src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs @@ -5,7 +5,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.NodeServices.HostingModels.PhysicalConnections; using Microsoft.AspNetCore.NodeServices.HostingModels.VirtualConnections; -using Microsoft.AspNetCore.NodeServices.Util; +using Microsoft.Extensions.Logging; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; @@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels private string _socketAddress; private VirtualConnectionClient _virtualConnectionClient; - public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress, INodeInstanceOutputLogger nodeInstanceOutputLogger = null) : base( + public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress, ILogger nodeInstanceOutputLogger) : base( EmbeddedResourceReader.Read( typeof(SocketNodeInstance), "/Content/Node/entrypoint-socket.js"), @@ -125,9 +125,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels // failure, this Node instance is no longer usable and should be discarded. _connectionHasFailed = true; - // TODO: Log the exception properly. Need to change the chain of calls up to this point to supply - // an ILogger or IServiceProvider etc. - Console.WriteLine(ex.Message); + OutputLogger.LogError(0, ex, ex.Message); }; } } diff --git a/src/Microsoft.AspNetCore.NodeServices/Util/ConsoleNodeInstanceOutputLogger.cs b/src/Microsoft.AspNetCore.NodeServices/Util/ConsoleNodeInstanceOutputLogger.cs deleted file mode 100644 index 6c02d5e..0000000 --- a/src/Microsoft.AspNetCore.NodeServices/Util/ConsoleNodeInstanceOutputLogger.cs +++ /dev/null @@ -1,17 +0,0 @@ -using System; - -namespace Microsoft.AspNetCore.NodeServices.Util -{ - public class ConsoleNodeInstanceOutputLogger : INodeInstanceOutputLogger - { - public void LogOutputData(string outputData) - { - Console.WriteLine("[Node] " + outputData); - } - - public void LogErrorData(string errorData) - { - Console.WriteLine("[Node] " + errorData); - } - } -} diff --git a/src/Microsoft.AspNetCore.NodeServices/Util/INodeInstanceOutputLogger.cs b/src/Microsoft.AspNetCore.NodeServices/Util/INodeInstanceOutputLogger.cs deleted file mode 100644 index a41b349..0000000 --- a/src/Microsoft.AspNetCore.NodeServices/Util/INodeInstanceOutputLogger.cs +++ /dev/null @@ -1,14 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; - -namespace Microsoft.AspNetCore.NodeServices.Util -{ - public interface INodeInstanceOutputLogger - { - void LogOutputData(string outputData); - - void LogErrorData(string errorData); - } -} diff --git a/src/Microsoft.AspNetCore.NodeServices/project.json b/src/Microsoft.AspNetCore.NodeServices/project.json index 0da37fe..415aff7 100644 --- a/src/Microsoft.AspNetCore.NodeServices/project.json +++ b/src/Microsoft.AspNetCore.NodeServices/project.json @@ -9,6 +9,7 @@ "Microsoft.AspNetCore.Hosting.Abstractions": "1.0.0", "Microsoft.Extensions.Configuration.Json": "1.0.0", "Microsoft.Extensions.DependencyInjection.Abstractions": "1.0.0", + "Microsoft.Extensions.Logging.Console": "1.0.0", "Microsoft.Extensions.PlatformAbstractions": "1.0.0", "Newtonsoft.Json": "9.0.1" },