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.

Sign up for GitHub

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

Adds ILoggingBuilder extension for LanguageServer logging. #187

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0da7b96
Adds ILoggingBuilder extension for LanguageServer logging.
bjorkstromm Oct 14, 2019
0721cbf
Make loggerfactory react on loglevel changes.
bjorkstromm Oct 19, 2019
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
View file
Open in desktop
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.Critical)
.SetMinimumLevel(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.

I'd like to avoid having this, but truth is that MEL adds a default filter which works on the factory level, so we won't get our logs to LSP logger if they are < Information (which is default),

Copy link
Collaborator

Choose a reason for hiding this comment

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

As long as it's configured by the consumer (read: Host process) I think that's fine.

.WithHandler<TextDocumentHandler>()
.WithHandler<DidChangeWatchedFilesHandler>()
.WithHandler<FoldingRangeHandler>()
Expand Down
3 changes: 3 additions & 0 deletions sample/SampleServer/TextDocumentHandler.cs
View file
Open in desktop
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
31 changes: 11 additions & 20 deletions src/Server/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using OmniSharp.Extensions.LanguageServer.Server.Pipelines;
using ISerializer = OmniSharp.Extensions.LanguageServer.Protocol.Serialization.ISerializer;
using System.Reactive.Disposables;
using Microsoft.Extensions.Options;

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

Expand All @@ -127,20 +127,11 @@ 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));

_reciever = reciever;
_serializer = serializer;
Expand Down Expand Up @@ -248,11 +239,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 +343,14 @@ 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.

There's a better way - I had to do something similar for KubeClient:

https://github.com/tintoy/dotnet-kube-client/tree/develop/src/KubeClient.Extensions.Configuration

Copy link
Collaborator

Choose a reason for hiding this comment

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

Specifically:

https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient.Extensions.Configuration/ConfigMapConfigurationProvider.cs#L155

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then:

https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient.Extensions.Configuration/ConfigMapConfigurationProvider.cs#L191

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>

}

_clientVersion = request.Capabilities?.GetClientVersion() ?? ClientVersion.Lsp2;
Expand Down
1 change: 0 additions & 1 deletion src/Server/LanguageServerOptions.cs
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions src/Server/LanguageServerLogger.cs → src/Server/Logging/LanguageServerLogger.cs
View file
Open in desktop
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
View file
Open in desktop
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;
}
}
}
8 changes: 5 additions & 3 deletions src/Server/LanguageServerLoggerProvider.cs → ...r/Logging/LanguageServerLoggerProvider.cs
View file
Open in desktop
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
View file
Open in desktop
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; }
}
}