-
Notifications
You must be signed in to change notification settings - Fork 105
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
{ | ||
|
@@ -106,8 +108,7 @@ public static ILanguageServer PreInit(LanguageServerOptions options) | |
options.TextDocumentIdentifierTypes, | ||
options.InitializeDelegates, | ||
options.InitializedDelegates, | ||
options.LoggingBuilderAction, | ||
options.AddDefaultLoggingProvider | ||
options.LoggingBuilderAction | ||
); | ||
} | ||
|
||
|
@@ -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; | ||
|
@@ -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) | ||
|
@@ -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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Best would be if we'd also here could notify There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I solved it by registering an own |
||
|
||
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; | ||
|
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; | ||
} | ||
} | ||
} |
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 |
---|---|---|
@@ -0,0 +1,9 @@ | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace OmniSharp.Extensions.LanguageServer.Server | ||
{ | ||
internal class LanguageServerLoggerSettings | ||
{ | ||
public LogLevel MinimumLogLevel { get; set; } | ||
} | ||
} |
There was a problem hiding this comment.
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 theAddLanguageServer
andSetMinimumLevel
?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
and the LSP side was only giving Information and Error but my file was giving me everything.