Skip to content

Commit 2086370

Browse files
bjorkstrommdavid-driscoll
bjorkstromm
authored and
david-driscoll
committed
Refactor logging +semver:minor (#182)
* Refactor logging - Expose ILoggingBuilder instead of accepting a ILoggerFactory * Fixup sample, it works. * Update sample code. * Fix null reference exception when handler does logging in it's constructor.
1 parent 7f63c73 commit 2086370

File tree

10 files changed

+106
-80
lines changed

10 files changed

+106
-80
lines changed

‎sample/SampleServer/Program.cs

+41-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
using System;
22
using System.Diagnostics;
33
using System.Threading.Tasks;
4+
using Microsoft.Extensions.DependencyInjection;
45
using Microsoft.Extensions.Logging;
56
using OmniSharp.Extensions.LanguageServer;
67
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
78
using OmniSharp.Extensions.LanguageServer.Server;
9+
using Serilog;
810

911
namespace SampleServer
1012
{
@@ -22,19 +24,56 @@ static async Task MainAsync(string[] args)
2224
// await Task.Delay(100);
2325
// }
2426

27+
Log.Logger = new LoggerConfiguration()
28+
.Enrich.FromLogContext()
29+
.WriteTo.File("log.txt", rollingInterval: RollingInterval.Day)
30+
.CreateLogger();
31+
32+
Log.Logger.Information("This only goes file...");
33+
2534
var server = await LanguageServer.From(options =>
2635
options
2736
.WithInput(Console.OpenStandardInput())
2837
.WithOutput(Console.OpenStandardOutput())
29-
.WithLoggerFactory(new LoggerFactory())
38+
.ConfigureLogging(x => x.AddSerilog())
3039
.AddDefaultLoggingProvider()
31-
.WithMinimumLogLevel(LogLevel.Trace)
3240
.WithHandler<TextDocumentHandler>()
3341
.WithHandler<DidChangeWatchedFilesHandler>()
3442
.WithHandler<FoldingRangeHandler>()
43+
.WithServices(services => {
44+
services.AddSingleton<Foo>(provider => {
45+
var loggerFactory = provider.GetService<ILoggerFactory>();
46+
var logger = loggerFactory.CreateLogger<Foo>();
47+
48+
logger.LogInformation("Configuring");
49+
50+
return new Foo(logger);
51+
});
52+
}).OnInitialize((s, request) => {
53+
var serviceProvider = s.Services;
54+
var foo = serviceProvider.GetService<Foo>();
55+
56+
return Task.CompletedTask;
57+
})
3558
);
3659

3760
await server.WaitForExit;
3861
}
3962
}
63+
64+
internal class Foo
65+
{
66+
private readonly ILogger<Foo> _logger;
67+
68+
public Foo(ILogger<Foo> logger)
69+
{
70+
logger.LogInformation("inside ctor");
71+
_logger = logger;
72+
}
73+
74+
public void SayFoo()
75+
{
76+
_logger.LogInformation("Fooooo!");
77+
}
78+
}
4079
}

‎sample/SampleServer/SampleServer.csproj

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
<PropertyGroup>
44
<OutputType>Exe</OutputType>
55
<IsPackable>false</IsPackable>
6-
<TargetFramework>netcoreapp2.0</TargetFramework>
6+
<TargetFramework>netcoreapp2.1</TargetFramework>
77
<RuntimeIdentifier>win7-x64</RuntimeIdentifier>
88
</PropertyGroup>
99

1010
<ItemGroup>
1111
<ProjectReference Include="../../src/Server/Server.csproj" />
1212
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.0.0" />
13+
<PackageReference Include="Serilog.Extensions.Logging" Version="3.0.1" />
14+
<PackageReference Include="Serilog.Sinks.File" Version="4.1.0-dev-00850" />
1315
</ItemGroup>
1416

1517
</Project>

‎sample/SampleServer/TextDocumentHandler.cs

+7-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Threading;
33
using System.Threading.Tasks;
4+
using Microsoft.Extensions.Logging;
45
using OmniSharp.Extensions.Embedded.MediatR;
56
using OmniSharp.Extensions.LanguageServer;
67
using OmniSharp.Extensions.LanguageServer.Protocol;
@@ -15,7 +16,7 @@ namespace SampleServer
1516
{
1617
class TextDocumentHandler : ITextDocumentSyncHandler
1718
{
18-
private readonly OmniSharp.Extensions.LanguageServer.Protocol.Server.ILanguageServer _router;
19+
private readonly ILogger<TextDocumentHandler> _logger;
1920

2021
private readonly DocumentSelector _documentSelector = new DocumentSelector(
2122
new DocumentFilter() {
@@ -25,19 +26,17 @@ class TextDocumentHandler : ITextDocumentSyncHandler
2526

2627
private SynchronizationCapability _capability;
2728

28-
public TextDocumentHandler(OmniSharp.Extensions.LanguageServer.Protocol.Server.ILanguageServer router)
29+
public TextDocumentHandler(ILogger<TextDocumentHandler> logger, Foo foo)
2930
{
30-
_router = router;
31+
_logger = logger;
32+
foo.SayFoo();
3133
}
3234

3335
public TextDocumentSyncKind Change { get; } = TextDocumentSyncKind.Full;
3436

3537
public Task<Unit> Handle(DidChangeTextDocumentParams notification, CancellationToken token)
3638
{
37-
_router.Window.LogMessage(new LogMessageParams() {
38-
Type = MessageType.Log,
39-
Message = "Hello World!!!!"
40-
});
39+
_logger.LogInformation("Hello world!");
4140
return Unit.Task;
4241
}
4342

@@ -57,10 +56,7 @@ public void SetCapability(SynchronizationCapability capability)
5756
public async Task<Unit> Handle(DidOpenTextDocumentParams notification, CancellationToken token)
5857
{
5958
await Task.Yield();
60-
_router.Window.LogMessage(new LogMessageParams() {
61-
Type = MessageType.Log,
62-
Message = "Hello World!!!!"
63-
});
59+
_logger.LogInformation("Hello world!");
6460
return Unit.Value;
6561
}
6662

‎src/JsonRpc/RequestRouterBase.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public virtual async Task<ErrorResponse> RouteRequest(TDescriptor descriptor, Re
148148

149149
return new JsonRpc.Client.Response(request.Id, responseValue, request);
150150
}
151-
catch (TaskCanceledException e)
151+
catch (TaskCanceledException)
152152
{
153153
_logger.LogDebug("Request {Id} was cancelled", id);
154154
return new RequestCancelled();

‎src/Server/LanguageServer.cs

+24-18
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,11 @@ public static async Task<ILanguageServer> From(LanguageServerOptions options, Ca
8989
/// <returns></returns>
9090
public static ILanguageServer PreInit(LanguageServerOptions options)
9191
{
92-
var server = new LanguageServer(
92+
return new LanguageServer(
9393
options.Input,
9494
options.Output,
9595
options.Reciever,
9696
options.RequestProcessIdentifier,
97-
options.LoggerFactory,
9897
options.Serializer,
9998
options.Services,
10099
options.HandlerTypes.Select(x => x.Assembly)
@@ -106,21 +105,17 @@ public static ILanguageServer PreInit(LanguageServerOptions options)
106105
options.TextDocumentIdentifiers,
107106
options.TextDocumentIdentifierTypes,
108107
options.InitializeDelegates,
109-
options.InitializedDelegates
108+
options.InitializedDelegates,
109+
options.LoggingBuilderAction,
110+
options.AddDefaultLoggingProvider
110111
);
111-
112-
if (options.AddDefaultLoggingProvider)
113-
options.LoggerFactory.AddProvider(new LanguageServerLoggerProvider(server));
114-
115-
return server;
116112
}
117113

118114
internal LanguageServer(
119115
Stream input,
120116
Stream output,
121117
ILspReciever reciever,
122118
IRequestProcessIdentifier requestProcessIdentifier,
123-
ILoggerFactory loggerFactory,
124119
ISerializer serializer,
125120
IServiceCollection services,
126121
IEnumerable<Assembly> assemblies,
@@ -131,11 +126,22 @@ internal LanguageServer(
131126
IEnumerable<ITextDocumentIdentifier> textDocumentIdentifiers,
132127
IEnumerable<Type> textDocumentIdentifierTypes,
133128
IEnumerable<InitializeDelegate> initializeDelegates,
134-
IEnumerable<InitializedDelegate> initializedDelegates)
129+
IEnumerable<InitializedDelegate> initializedDelegates,
130+
Action<ILoggingBuilder> loggingBuilderAction,
131+
bool addDefaultLoggingProvider)
135132
{
136133
var outputHandler = new OutputHandler(output, serializer);
137134

138-
services.AddLogging();
135+
services.AddLogging(builder =>
136+
{
137+
loggingBuilderAction(builder);
138+
139+
if (addDefaultLoggingProvider)
140+
{
141+
builder.AddProvider(new LanguageServerLoggerProvider(this));
142+
}
143+
});
144+
139145
_reciever = reciever;
140146
_serializer = serializer;
141147
_supportedCapabilities = new SupportedCapabilities();
@@ -153,7 +159,7 @@ internal LanguageServer(
153159
services.AddSingleton(requestProcessIdentifier);
154160
services.AddSingleton<OmniSharp.Extensions.JsonRpc.IReciever>(reciever);
155161
services.AddSingleton<ILspReciever>(reciever);
156-
services.AddSingleton(loggerFactory);
162+
157163
foreach (var item in handlers)
158164
{
159165
services.AddSingleton(item);
@@ -209,6 +215,12 @@ internal LanguageServer(
209215

210216
_exitHandler = new ServerExitHandler(_shutdownHandler);
211217

218+
// We need to at least create Window here in case any handler does loggin in their constructor
219+
Document = new LanguageServerDocument(_responseRouter);
220+
Client = new LanguageServerClient(_responseRouter);
221+
Window = new LanguageServerWindow(_responseRouter);
222+
Workspace = new LanguageServerWorkspace(_responseRouter);
223+
212224
_disposable.Add(
213225
AddHandlers(this, _shutdownHandler, _exitHandler, new CancelRequestHandler<ILspHandlerDescriptor>(_requestRouter))
214226
);
@@ -226,12 +238,6 @@ internal LanguageServer(
226238
{
227239
_disposable.Add(_collection.Add(name, handlerFunc(_serviceProvider)));
228240
}
229-
230-
231-
Document = new LanguageServerDocument(_responseRouter);
232-
Client = new LanguageServerClient(_responseRouter);
233-
Window = new LanguageServerWindow(_responseRouter);
234-
Workspace = new LanguageServerWorkspace(_responseRouter);
235241
}
236242

237243
public ILanguageServerDocument Document { get; }

‎src/Server/LanguageServerOptions.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ public LanguageServerOptions()
2222

2323
public Stream Input { get; set; }
2424
public Stream Output { get; set; }
25-
public LogLevel MinimumLogLevel { get; set; } = LogLevel.Information;
26-
public ILoggerFactory LoggerFactory { get; set; } = new LoggerFactory();
2725
public ISerializer Serializer { get; set; } = Protocol.Serialization.Serializer.Instance;
2826
public IRequestProcessIdentifier RequestProcessIdentifier { get; set; } = new RequestProcessIdentifier();
2927
public ILspReciever Reciever { get; set; } = new LspReciever();
@@ -36,6 +34,7 @@ public LanguageServerOptions()
3634
internal List<Type> TextDocumentIdentifierTypes { get; set; } = new List<Type>();
3735
internal List<Assembly> HandlerAssemblies { get; set; } = new List<Assembly>();
3836
internal bool AddDefaultLoggingProvider { get; set; }
37+
internal Action<ILoggingBuilder> LoggingBuilderAction { get; set; } = new Action<ILoggingBuilder>(_ => { });
3938

4039
internal readonly List<InitializeDelegate> InitializeDelegates = new List<InitializeDelegate>();
4140
internal readonly List<InitializedDelegate> InitializedDelegates = new List<InitializedDelegate>();

‎src/Server/LanguageServerOptionsExtensions.cs

+6-12
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,6 @@ public static LanguageServerOptions WithOutput(this LanguageServerOptions option
2222
return options;
2323
}
2424

25-
public static LanguageServerOptions WithMinimumLogLevel(this LanguageServerOptions options, LogLevel logLevel)
26-
{
27-
options.MinimumLogLevel = logLevel;
28-
return options;
29-
}
30-
31-
public static LanguageServerOptions WithLoggerFactory(this LanguageServerOptions options, ILoggerFactory loggerFactory)
32-
{
33-
options.LoggerFactory = loggerFactory;
34-
return options;
35-
}
36-
3725
public static LanguageServerOptions WithRequestProcessIdentifier(this LanguageServerOptions options, IRequestProcessIdentifier requestProcessIdentifier)
3826
{
3927
options.RequestProcessIdentifier = requestProcessIdentifier;
@@ -109,5 +97,11 @@ public static LanguageServerOptions OnInitialized(this LanguageServerOptions opt
10997
options.InitializedDelegates.Add(@delegate);
11098
return options;
11199
}
100+
101+
public static LanguageServerOptions ConfigureLogging(this LanguageServerOptions options, Action<ILoggingBuilder> builderAction)
102+
{
103+
options.LoggingBuilderAction = builderAction;
104+
return options;
105+
}
112106
}
113107
}

‎test/Lsp.Tests/LanguageServerTests.cs

+7-7
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ public async Task Works_With_IWorkspaceSymbolsHandler()
3737
var serverStart = LanguageServer.From(x => x
3838
//.WithHandler(handler)
3939
.WithInput(process.ClientOutputStream)
40-
.WithOutput(process.ClientInputStream)
41-
.WithLoggerFactory(LoggerFactory)
42-
.AddDefaultLoggingProvider()
43-
.WithMinimumLogLevel(LogLevel.Trace),
40+
.WithOutput(process.ClientInputStream),
41+
//.WithLoggerFactory(LoggerFactory)
42+
//.AddDefaultLoggingProvider()
43+
//.WithMinimumLogLevel(LogLevel.Trace),
4444
cts.Token
4545
);
4646

@@ -63,9 +63,9 @@ public async Task GH141_CrashesWithEmptyInitializeParams()
6363
var server = LanguageServer.PreInit(x => x
6464
.WithInput(process.ClientOutputStream)
6565
.WithOutput(process.ClientInputStream)
66-
.WithLoggerFactory(LoggerFactory)
67-
.AddDefaultLoggingProvider()
68-
.WithMinimumLogLevel(LogLevel.Trace)
66+
//.WithLoggerFactory(LoggerFactory)
67+
//.AddDefaultLoggingProvider()
68+
//.WithMinimumLogLevel(LogLevel.Trace)
6969
.AddHandlers(TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"), "csharp"))
7070
) as IRequestHandler<InitializeParams, InitializeResult>;
7171

‎vscode-testextension/.vscode/tasks.json

+11-20
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,14 @@
88

99
// A task runner that calls a custom npm script that compiles the extension.
1010
{
11-
"version": "0.1.0",
12-
13-
// we want to run npm
14-
"command": "npm",
15-
16-
// the command is a shell script
17-
"isShellCommand": true,
18-
19-
// show the output window only if unrecognized errors occur.
20-
"showOutput": "silent",
21-
22-
// we run the custom script "compile" as defined in package.json
23-
"args": ["run", "compile", "--loglevel", "silent"],
24-
25-
// The tsc compiler is started in watching mode
26-
"isWatching": true,
27-
28-
// use the standard tsc in watch mode problem matcher to find compile problems in the output.
29-
"problemMatcher": "$tsc-watch"
30-
}
11+
"version": "2.0.0",
12+
"tasks": [
13+
{
14+
"label": "npm",
15+
"command": "npm",
16+
"args": ["run", "compile", "--loglevel", "silent"],
17+
"type": "shell",
18+
"problemMatcher": "$tsc-watch"
19+
}
20+
]
21+
}

‎vscode-testextension/src/extension.ts

+5-6
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,20 @@ import { Trace } from "vscode-jsonrpc";
2020

2121
export function activate(context: ExtensionContext) {
2222
// The server is implemented in node
23-
// let serverExe = 'dotnet';
23+
let serverExe = 'dotnet';
2424

2525
// let serverExe = 'D:\\Development\\Omnisharp\\csharp-language-server-protocol\\sample\\SampleServer\\bin\\Debug\\netcoreapp2.0\\win7-x64\\SampleServer.exe';
26-
let serverExe =
27-
"D:/Development/Omnisharp/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio.Driver/win7-x64/OmniSharp.exe";
26+
// let serverExe = "D:/Development/Omnisharp/omnisharp-roslyn/artifacts/publish/OmniSharp.Stdio.Driver/win7-x64/OmniSharp.exe";
2827
// The debug options for the server
2928
// let debugOptions = { execArgv: ['-lsp', '-d' };5
3029

3130
// If the extension is launched in debug mode then the debug server options are used
3231
// Otherwise the run options are used
3332
let serverOptions: ServerOptions = {
3433
// run: { command: serverExe, args: ['-lsp', '-d'] },
35-
run: { command: serverExe, args: ["-lsp"] },
34+
run: { command: serverExe, args: ["C:/src/gh/csharp-language-server-protocol/sample/SampleServer/bin/Debug/netcoreapp2.1/win7-x64/SampleServer.dll"] },
3635
// debug: { command: serverExe, args: ['-lsp', '-d'] }
37-
debug: { command: serverExe, args: ["-lsp"] }
36+
debug: { command: serverExe, args: ["C:/src/gh/csharp-language-server-protocol/sample/SampleServer/bin/Debug/netcoreapp2.1/win7-x64/SampleServer.dll"] }
3837
};
3938

4039
// Options to control the language client
@@ -65,7 +64,7 @@ export function activate(context: ExtensionContext) {
6564
serverOptions,
6665
clientOptions
6766
);
68-
// client.trace = Trace.Verbose;
67+
client.trace = Trace.Verbose;
6968
client.clientOptions.errorHandler;
7069
let disposable = client.start();
7170

0 commit comments

Comments
 (0)