Skip to content

Commit f4efcac

Browse files
SteveSandersonMSSteveSandersonMS
committed
Switch to native .NET logging APIs
1 parent 27ffa72 commit f4efcac

File tree

8 files changed

+46
-54
lines changed

8 files changed

+46
-54
lines changed

‎src/Microsoft.AspNetCore.NodeServices/Configuration/Configuration.cs‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22
using Microsoft.Extensions.DependencyInjection;
33
using Microsoft.AspNetCore.Hosting;
44
using Microsoft.AspNetCore.NodeServices.HostingModels;
5+
using Microsoft.Extensions.Logging;
6+
using Microsoft.Extensions.Logging.Console;
57

68
namespace Microsoft.AspNetCore.NodeServices
79
{
810
public static class Configuration
911
{
12+
const string LogCategoryName = "Microsoft.AspNetCore.NodeServices";
13+
1014
public static void AddNodeServices(this IServiceCollection serviceCollection)
1115
=> AddNodeServices(serviceCollection, new NodeServicesOptions());
1216

@@ -15,13 +19,24 @@ public static void AddNodeServices(this IServiceCollection serviceCollection, No
1519
serviceCollection.AddSingleton(typeof(INodeServices), serviceProvider =>
1620
{
1721
// Since this instance is being created through DI, we can access the IHostingEnvironment
18-
// to populate options.ProjectPath if it wasn't explicitly specified.
19-
var hostEnv = serviceProvider.GetRequiredService<IHostingEnvironment>();
22+
// to populate options.ProjectPath if it wasn't explicitly specified.
2023
if (string.IsNullOrEmpty(options.ProjectPath))
2124
{
25+
var hostEnv = serviceProvider.GetRequiredService<IHostingEnvironment>();
2226
options.ProjectPath = hostEnv.ContentRootPath;
2327
}
2428

29+
// Likewise, if no logger was specified explicitly, we should use the one from DI.
30+
// If it doesn't provide one, CreateNodeInstance will set up a default.
31+
if (options.NodeInstanceOutputLogger == null)
32+
{
33+
var loggerFactory = serviceProvider.GetService<ILoggerFactory>();
34+
if (loggerFactory != null)
35+
{
36+
options.NodeInstanceOutputLogger = loggerFactory.CreateLogger(LogCategoryName);
37+
}
38+
}
39+
2540
return new NodeServicesImpl(options, () => CreateNodeInstance(options));
2641
});
2742
}
@@ -33,6 +48,13 @@ public static INodeServices CreateNodeServices(NodeServicesOptions options)
3348

3449
private static INodeInstance CreateNodeInstance(NodeServicesOptions options)
3550
{
51+
// If you've specified no logger, fall back on a default console logger
52+
var logger = options.NodeInstanceOutputLogger;
53+
if (logger == null)
54+
{
55+
logger = new ConsoleLogger(LogCategoryName, null, false);
56+
}
57+
3658
if (options.NodeInstanceFactory != null)
3759
{
3860
// If you've explicitly supplied an INodeInstance factory, we'll use that. This is useful for
@@ -46,10 +68,10 @@ private static INodeInstance CreateNodeInstance(NodeServicesOptions options)
4668
switch (options.HostingModel)
4769
{
4870
case NodeHostingModel.Http:
49-
return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, /* port */ 0, options.NodeInstanceOutputLogger);
71+
return new HttpNodeInstance(options.ProjectPath, options.WatchFileExtensions, logger, /* port */ 0);
5072
case NodeHostingModel.Socket:
5173
var pipeName = "pni-" + Guid.NewGuid().ToString("D"); // Arbitrary non-clashing string
52-
return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, options.NodeInstanceOutputLogger);
74+
return new SocketNodeInstance(options.ProjectPath, options.WatchFileExtensions, pipeName, logger);
5375
default:
5476
throw new ArgumentException("Unknown hosting model: " + options.HostingModel);
5577
}

‎src/Microsoft.AspNetCore.NodeServices/Configuration/NodeServicesOptions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
using System;
22
using Microsoft.AspNetCore.NodeServices.HostingModels;
3-
using Microsoft.AspNetCore.NodeServices.Util;
3+
using Microsoft.Extensions.Logging;
44

55
namespace Microsoft.AspNetCore.NodeServices
66
{
@@ -20,6 +20,6 @@ public NodeServicesOptions()
2020
public Func<INodeInstance> NodeInstanceFactory { get; set; }
2121
public string ProjectPath { get; set; }
2222
public string[] WatchFileExtensions { get; set; }
23-
public INodeInstanceOutputLogger NodeInstanceOutputLogger { get; set; }
23+
public ILogger NodeInstanceOutputLogger { get; set; }
2424
}
2525
}

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/HttpNodeInstance.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.Text;
55
using System.Text.RegularExpressions;
66
using System.Threading.Tasks;
7-
using Microsoft.AspNetCore.NodeServices.Util;
7+
using Microsoft.Extensions.Logging;
88
using Newtonsoft.Json;
99
using Newtonsoft.Json.Serialization;
1010

@@ -33,7 +33,7 @@ internal class HttpNodeInstance : OutOfProcessNodeInstance
3333
private bool _disposed;
3434
private int _portNumber;
3535

36-
public HttpNodeInstance(string projectPath, string[] watchFileExtensions, int port = 0, INodeInstanceOutputLogger nodeInstanceOutputLogger = null)
36+
public HttpNodeInstance(string projectPath, string[] watchFileExtensions, ILogger nodeInstanceOutputLogger, int port = 0)
3737
: base(
3838
EmbeddedResourceReader.Read(
3939
typeof(HttpNodeInstance),

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/OutOfProcessNodeInstance.cs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
using System.IO;
44
using System.Linq;
55
using System.Threading.Tasks;
6-
using Microsoft.AspNetCore.NodeServices.Util;
6+
using Microsoft.Extensions.Logging;
77

88
namespace Microsoft.AspNetCore.NodeServices.HostingModels
99
{
@@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.NodeServices.HostingModels
1919
/// <seealso cref="Microsoft.AspNetCore.NodeServices.HostingModels.INodeInstance" />
2020
public abstract class OutOfProcessNodeInstance : INodeInstance
2121
{
22+
protected readonly ILogger OutputLogger;
2223
private const string ConnectionEstablishedMessage = "[Microsoft.AspNetCore.NodeServices:Listening]";
2324
private readonly TaskCompletionSource<object> _connectionIsReadySource = new TaskCompletionSource<object>();
2425
private bool _disposed;
@@ -27,18 +28,20 @@ public abstract class OutOfProcessNodeInstance : INodeInstance
2728
private readonly Process _nodeProcess;
2829
private bool _nodeProcessNeedsRestart;
2930
private readonly string[] _watchFileExtensions;
30-
private INodeInstanceOutputLogger _nodeInstanceOutputLogger;
3131

3232
public OutOfProcessNodeInstance(
3333
string entryPointScript,
3434
string projectPath,
3535
string[] watchFileExtensions,
3636
string commandLineArguments,
37-
INodeInstanceOutputLogger nodeOutputLogger)
37+
ILogger nodeOutputLogger)
3838
{
39-
_nodeInstanceOutputLogger = nodeOutputLogger;
40-
if(_nodeInstanceOutputLogger == null)
41-
_nodeInstanceOutputLogger = new ConsoleNodeInstanceOutputLogger();
39+
if (nodeOutputLogger == null)
40+
{
41+
throw new ArgumentNullException(nameof(nodeOutputLogger));
42+
}
43+
44+
OutputLogger = nodeOutputLogger;
4245
_entryPointScript = new StringAsTempFile(entryPointScript);
4346

4447
var startInfo = PrepareNodeProcessStartInfo(_entryPointScript.FileName, projectPath, commandLineArguments);
@@ -112,12 +115,12 @@ protected virtual ProcessStartInfo PrepareNodeProcessStartInfo(
112115

113116
protected virtual void OnOutputDataReceived(string outputData)
114117
{
115-
_nodeInstanceOutputLogger.LogOutputData(outputData);
118+
OutputLogger.LogInformation(outputData);
116119
}
117120

118121
protected virtual void OnErrorDataReceived(string errorData)
119122
{
120-
_nodeInstanceOutputLogger.LogErrorData(errorData);
123+
OutputLogger.LogError(errorData);
121124
}
122125

123126
protected virtual void Dispose(bool disposing)
@@ -255,8 +258,7 @@ private bool IsFilenameBeingWatched(string fullPath)
255258

256259
private void RestartDueToFileChange(string fullPath)
257260
{
258-
// TODO: Use proper logger
259-
Console.WriteLine($"Node will restart because file changed: {fullPath}");
261+
OutputLogger.LogInformation($"Node will restart because file changed: {fullPath}");
260262

261263
_nodeProcessNeedsRestart = true;
262264

‎src/Microsoft.AspNetCore.NodeServices/HostingModels/SocketNodeInstance.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
using System.Threading.Tasks;
66
using Microsoft.AspNetCore.NodeServices.HostingModels.PhysicalConnections;
77
using Microsoft.AspNetCore.NodeServices.HostingModels.VirtualConnections;
8-
using Microsoft.AspNetCore.NodeServices.Util;
8+
using Microsoft.Extensions.Logging;
99
using Newtonsoft.Json;
1010
using Newtonsoft.Json.Serialization;
1111

@@ -37,7 +37,7 @@ internal class SocketNodeInstance : OutOfProcessNodeInstance
3737
private string _socketAddress;
3838
private VirtualConnectionClient _virtualConnectionClient;
3939

40-
public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress, INodeInstanceOutputLogger nodeInstanceOutputLogger = null) : base(
40+
public SocketNodeInstance(string projectPath, string[] watchFileExtensions, string socketAddress, ILogger nodeInstanceOutputLogger) : base(
4141
EmbeddedResourceReader.Read(
4242
typeof(SocketNodeInstance),
4343
"/Content/Node/entrypoint-socket.js"),
@@ -125,9 +125,7 @@ private async Task EnsureVirtualConnectionClientCreated()
125125
// failure, this Node instance is no longer usable and should be discarded.
126126
_connectionHasFailed = true;
127127

128-
// TODO: Log the exception properly. Need to change the chain of calls up to this point to supply
129-
// an ILogger or IServiceProvider etc.
130-
Console.WriteLine(ex.Message);
128+
OutputLogger.LogError(0, ex, ex.Message);
131129
};
132130
}
133131
}

‎src/Microsoft.AspNetCore.NodeServices/Util/ConsoleNodeInstanceOutputLogger.cs‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎src/Microsoft.AspNetCore.NodeServices/Util/INodeInstanceOutputLogger.cs‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎src/Microsoft.AspNetCore.NodeServices/project.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"Microsoft.AspNetCore.Hosting.Abstractions": "1.0.0",
1010
"Microsoft.Extensions.Configuration.Json": "1.0.0",
1111
"Microsoft.Extensions.DependencyInjection.Abstractions": "1.0.0",
12+
"Microsoft.Extensions.Logging.Console": "1.0.0",
1213
"Microsoft.Extensions.PlatformAbstractions": "1.0.0",
1314
"Newtonsoft.Json": "9.0.1"
1415
},

0 commit comments

Comments
 (0)