Skip to content

Adds ILoggingBuilder extension for LanguageServer logging. #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions sample/SampleServer/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ static void Main(string[] args)

static async Task MainAsync(string[] args)
{
// while (!System.Diagnostics.Debugger.IsAttached)
// {
//Debugger.Launch();
//while (!System.Diagnostics.Debugger.IsAttached)
//{
// await Task.Delay(100);
// }
//}

Log.Logger = new LoggerConfiguration()
.Enrich.FromLogContext()
Expand All @@ -35,8 +36,10 @@ static async Task MainAsync(string[] args)
options
.WithInput(Console.OpenStandardInput())
.WithOutput(Console.OpenStandardOutput())
.ConfigureLogging(x => x.AddSerilog())
.AddDefaultLoggingProvider()
.ConfigureLogging(x => x
.AddSerilog()
.AddLanguageServer(LogLevel.Error)
.SetMinimumLevel(LogLevel.Error))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to pass in the LogLevel for both the AddLanguageServer and SetMinimumLevel?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to. It's an option. One is global, the other one is just for LSP logging.

Copy link
Collaborator

@TylerLeonhardt TylerLeonhardt Oct 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mholo65 I gave this a go:

.AddSerilog()
.AddLanguageServer()
.SetMinimumLevel(LogLevel.Trace)

and the LSP side was only giving Information and Error but my file was giving me everything.

.WithHandler<TextDocumentHandler>()
.WithHandler<DidChangeWatchedFilesHandler>()
.WithHandler<FoldingRangeHandler>()
Expand Down
3 changes: 3 additions & 0 deletions sample/SampleServer/TextDocumentHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public TextDocumentHandler(ILogger<TextDocumentHandler> logger, Foo foo)

public Task<Unit> Handle(DidChangeTextDocumentParams notification, CancellationToken token)
{
_logger.LogCritical("Critical");
_logger.LogDebug("Debug");
_logger.LogTrace("Trace");
_logger.LogInformation("Hello world!");
return Unit.Task;
}
Expand Down
41 changes: 21 additions & 20 deletions src/Server/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
using OmniSharp.Extensions.LanguageServer.Server.Pipelines;
using ISerializer = OmniSharp.Extensions.LanguageServer.Protocol.Serialization.ISerializer;
using System.Reactive.Disposables;
using Microsoft.Extensions.Options;
using OmniSharp.Extensions.LanguageServer.Server.Logging;

namespace OmniSharp.Extensions.LanguageServer.Server
{
Expand Down Expand Up @@ -106,8 +108,7 @@ public static ILanguageServer PreInit(LanguageServerOptions options)
options.TextDocumentIdentifierTypes,
options.InitializeDelegates,
options.InitializedDelegates,
options.LoggingBuilderAction,
options.AddDefaultLoggingProvider
options.LoggingBuilderAction
);
}

Expand All @@ -127,20 +128,12 @@ internal LanguageServer(
IEnumerable<Type> textDocumentIdentifierTypes,
IEnumerable<InitializeDelegate> initializeDelegates,
IEnumerable<InitializedDelegate> initializedDelegates,
Action<ILoggingBuilder> loggingBuilderAction,
bool addDefaultLoggingProvider)
Action<ILoggingBuilder> loggingBuilderAction)
{
var outputHandler = new OutputHandler(output, serializer);

services.AddLogging(builder =>
{
loggingBuilderAction(builder);

if (addDefaultLoggingProvider)
{
builder.AddProvider(new LanguageServerLoggerProvider(this));
}
});
services.AddLogging(builder => loggingBuilderAction(builder));
services.AddSingleton<IOptionsMonitor<LoggerFilterOptions>, LanguageServerLoggerFilterOptions>();

_reciever = reciever;
_serializer = serializer;
Expand Down Expand Up @@ -248,11 +241,6 @@ internal LanguageServer(
public InitializeParams ClientSettings { get; private set; }
public InitializeResult ServerSettings { get; private set; }

/// <summary>
/// The minimum level for the server's default logger.
/// </summary>
public LogLevel MinimumLogLevel { get; set; }

public IServiceProvider Services => _serviceProvider;

public IDisposable AddHandler(string method, IJsonRpcHandler handler)
Expand Down Expand Up @@ -357,9 +345,22 @@ async Task<InitializeResult> IRequestHandler<InitializeParams, InitializeResult>
{
ClientSettings = request;

if (request.Trace == InitializeTrace.Verbose && MinimumLogLevel >= LogLevel.Information)
if (request.Trace == InitializeTrace.Verbose)
{
MinimumLogLevel = LogLevel.Trace;
var loggerSettings = _serviceProvider.GetService<LanguageServerLoggerSettings>();

if (loggerSettings?.MinimumLogLevel <= LogLevel.Information)
{
loggerSettings.MinimumLogLevel = LogLevel.Trace;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best would be if we'd also here could notify IOptionsMonitor<LoggerFilterOptions> that it has changed. Without a configuration file, we'd probably have to do something like this. https://stackoverflow.com/a/53077453

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably look at a configuration integration. My thought is something like interface ILspConfiguration : IConfiguration and then populate that some defaults, like the logging state, but also wrap up the configuration handlers so that they surface their data through it, along with the refresh tokens.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I solved it by registering an own IOptionsMonitor<LoggerFilterOptions>


var optionsMonitor = _serviceProvider.GetService<IOptionsMonitor<LoggerFilterOptions>>() as LanguageServerLoggerFilterOptions;

if (optionsMonitor?.CurrentValue.MinLevel <= LogLevel.Information)
{
optionsMonitor.CurrentValue.MinLevel = LogLevel.Trace;
optionsMonitor.Set(optionsMonitor.CurrentValue);
}
}

_clientVersion = request.Capabilities?.GetClientVersion() ?? ClientVersion.Lsp2;
Expand Down
1 change: 0 additions & 1 deletion src/Server/LanguageServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public LanguageServerOptions()
internal List<Type> HandlerTypes { get; set; } = new List<Type>();
internal List<Type> TextDocumentIdentifierTypes { get; set; } = new List<Type>();
internal List<Assembly> HandlerAssemblies { get; set; } = new List<Assembly>();
internal bool AddDefaultLoggingProvider { get; set; }
internal Action<ILoggingBuilder> LoggingBuilderAction { get; set; } = new Action<ILoggingBuilder>(_ => { });

internal readonly List<InitializeDelegate> InitializeDelegates = new List<InitializeDelegate>();
Expand Down
7 changes: 0 additions & 7 deletions src/Server/LanguageServerOptionsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ public static LanguageServerOptions WithServices(this LanguageServerOptions opti
return options;
}

public static LanguageServerOptions AddDefaultLoggingProvider(this LanguageServerOptions options)
{
options.AddDefaultLoggingProvider = true;
return options;
}


public static LanguageServerOptions OnInitialize(this LanguageServerOptions options, InitializeDelegate @delegate)
{
options.InitializeDelegates.Add(@delegate);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
using System;
using Microsoft.Extensions.Logging;
using OmniSharp.Extensions.JsonRpc;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Server;

namespace OmniSharp.Extensions.LanguageServer.Server
{
class LanguageServerLogger : ILogger
{
private readonly LanguageServer _responseRouter;
private readonly ILanguageServer _responseRouter;
private readonly Func<LogLevel> _logLevelGetter;

public LanguageServerLogger(LanguageServer responseRouter, Func<LogLevel> logLevelGetter)
public LanguageServerLogger(ILanguageServer responseRouter, Func<LogLevel> logLevelGetter)
{
_logLevelGetter = logLevelGetter;
_responseRouter = responseRouter;
Expand Down
17 changes: 17 additions & 0 deletions src/Server/Logging/LanguageServerLoggerExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace OmniSharp.Extensions.LanguageServer.Server
{
public static class LanguageServerLoggerExtensions
{
public static ILoggingBuilder AddLanguageServer(this ILoggingBuilder builder, LogLevel minLevel = LogLevel.Information)
{
builder.Services.AddSingleton<LanguageServerLoggerSettings>(_ => new LanguageServerLoggerSettings { MinimumLogLevel = minLevel });
builder.Services.AddSingleton<ILoggerProvider, LanguageServerLoggerProvider>();

return builder;
}
}
}
61 changes: 61 additions & 0 deletions src/Server/Logging/LanguageServerLoggerFilterOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Collections.Generic;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace OmniSharp.Extensions.LanguageServer.Server.Logging
{
internal class LanguageServerLoggerFilterOptions : IOptionsMonitor<LoggerFilterOptions>, IDisposable
{
private readonly List<IDisposable> _registrations = new List<IDisposable>();
private event Action<LoggerFilterOptions, string> _onChange;

public LanguageServerLoggerFilterOptions(IOptions<LoggerFilterOptions> options)
{
CurrentValue = options.Value;
}

public LoggerFilterOptions CurrentValue { get; private set; }

public LoggerFilterOptions Get(string _) => CurrentValue;

public IDisposable OnChange(Action<LoggerFilterOptions, string> listener)
{
var disposable = new ChangeTrackerDisposable(this, listener);
_onChange += disposable.OnChange;
return disposable;
}

public void Dispose()
{
foreach (var registration in _registrations)
{
registration.Dispose();
}

_registrations.Clear();
}

internal void Set(LoggerFilterOptions options)
{
CurrentValue = options;
_onChange?.Invoke(options, Options.DefaultName);
}

private class ChangeTrackerDisposable : IDisposable
{
private readonly Action<LoggerFilterOptions, string> _listener;
private readonly LanguageServerLoggerFilterOptions _monitor;

public ChangeTrackerDisposable(LanguageServerLoggerFilterOptions monitor, Action<LoggerFilterOptions, string> listener)
{
_listener = listener;
_monitor = monitor;
}

public void OnChange(LoggerFilterOptions options, string name) => _listener.Invoke(options, name);

public void Dispose() => _monitor._onChange -= OnChange;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ namespace OmniSharp.Extensions.LanguageServer.Server
{
class LanguageServerLoggerProvider : ILoggerProvider
{
private readonly LanguageServer _languageServer;
private readonly ILanguageServer _languageServer;
private readonly LanguageServerLoggerSettings _settings;

public LanguageServerLoggerProvider(LanguageServer languageServer)
public LanguageServerLoggerProvider(ILanguageServer languageServer, LanguageServerLoggerSettings settings)
{
_languageServer = languageServer;
_settings = settings;
}

public ILogger CreateLogger(string categoryName)
{
return new LanguageServerLogger(_languageServer, () => _languageServer.MinimumLogLevel);
return new LanguageServerLogger(_languageServer, () => _settings.MinimumLogLevel);
}

public void Dispose()
Expand Down
9 changes: 9 additions & 0 deletions src/Server/Logging/LanguageServerLoggerSettings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using Microsoft.Extensions.Logging;

namespace OmniSharp.Extensions.LanguageServer.Server
{
internal class LanguageServerLoggerSettings
{
public LogLevel MinimumLogLevel { get; set; }
}
}