Skip to content

Commit

Permalink
Remove MS.Ext.Logging and Abstractions from most of our repo (#10199)
Browse files Browse the repository at this point in the history
We don't get much from using MS.Ext.Logging for our logging, and we pay
for it with ~two~ three (?) DLL loads in VS. This fixes that by
implementing the logging bits outselves, and removing the abstraction
for options monitoring which we weren't really using.

I also removed the notion of "scopes" from the logging because we never
actually implemented them, so this also closes
#8232. We don't actually do any
structured logging so we're really not missing much.

The only place that does still use MS.Ext.Logging is our one DLL that is
referenced by Roslyn in VS Code, since they pass us an ILogger, so we
don't really have a choice about that bit :)

I still need to do a little more testing of the options bit, ~because
I'm seeing some oddities, but I think the issue is really one with the
unified settings experience and/or our options updating code.~ Okay
yeah, this is happening in VS main too. Logged
#10200 but will make sure this
doesn't get any worse :)
  • Loading branch information
davidwengier authored Apr 2, 2024
2 parents 96ebc95 + 83a5725 commit 8c34fdd
Show file tree
Hide file tree
Showing 173 changed files with 631 additions and 647 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public async Task SetupAsync()
htmlCodeActionProviders: languageServer.GetRequiredService<IEnumerable<IHtmlCodeActionProvider>>(),
clientConnection: languageServer.GetRequiredService<IClientConnection>(),
languageServerFeatureOptions: languageServer.GetRequiredService<LanguageServerFeatureOptions>(),
loggerFactory: languageServer.GetRequiredService<IRazorLoggerFactory>(),
loggerFactory: languageServer.GetRequiredService<ILoggerFactory>(),
telemetryReporter: null);

var projectRoot = Path.Combine(RepoRoot, "src", "Razor", "test", "testapps", "ComponentApp");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using Microsoft.AspNetCore.Razor.LanguageServer;
using Microsoft.AspNetCore.Razor.LanguageServer.Completion;
using Microsoft.AspNetCore.Razor.LanguageServer.Completion.Delegation;
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
Expand Down Expand Up @@ -43,11 +44,12 @@ public async Task SetupAsync()
var documentMappingService = lspServices.GetRequiredService<IRazorDocumentMappingService>();
var clientConnection = lspServices.GetRequiredService<IClientConnection>();
var completionListCache = lspServices.GetRequiredService<CompletionListCache>();
var loggerFactory = lspServices.GetRequiredService<IRazorLoggerFactory>();
var loggerFactory = lspServices.GetRequiredService<ILoggerFactory>();

var delegatedCompletionListProvider = new TestDelegatedCompletionListProvider(responseRewriters, documentMappingService, clientConnection, completionListCache);
var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider);
var optionsMonitor = new BenchmarkOptionsMonitor<RazorLSPOptions>(RazorLSPOptions.Default);
var configurationService = new DefaultRazorConfigurationService(clientConnection, loggerFactory);
var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);

CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, telemetryReporter: null, optionsMonitor, loggerFactory);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ private IRazorDocumentMappingService BuildRazorDocumentMappingService()
return razorDocumentMappingService.Object;
}

private IRazorLoggerFactory BuildLoggerFactory() => Mock.Of<IRazorLoggerFactory>(
r => r.CreateLogger(
private ILoggerFactory BuildLoggerFactory() => Mock.Of<ILoggerFactory>(
r => r.GetOrCreateLogger(
It.IsAny<string>()) == new NoopLogger(),
MockBehavior.Strict);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Nerdbank.Streams;

Expand Down Expand Up @@ -109,9 +108,9 @@ public Task<TResponse> SendRequestAsync<TParams, TResponse>(string method, TPara
}
}

internal class NoopLoggerFactory() : AbstractRazorLoggerFactory([new NoopLoggerProvider()]);
internal class NoopLoggerFactory() : AbstractLoggerFactory([new NoopLoggerProvider()]);

internal class NoopLoggerProvider : IRazorLoggerProvider
internal class NoopLoggerProvider : ILoggerProvider
{
public ILogger CreateLogger(string categoryName)
{
Expand All @@ -125,17 +124,12 @@ public void Dispose()

internal class NoopLogger : ILogger, ILspLogger
{
public IDisposable BeginScope<TState>(TState state)
{
throw new NotImplementedException();
}

public bool IsEnabled(LogLevel logLevel)
{
return true;
}

public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
public void Log<TState>(LogLevel logLevel, TState state, Exception exception, Func<TState, Exception, string> formatter)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public TestRazorSemanticTokensInfoService(
LanguageServerFeatureOptions languageServerFeatureOptions,
IRazorDocumentMappingService documentMappingService,
RazorSemanticTokensLegendService razorSemanticTokensLegendService,
IRazorLoggerFactory loggerFactory)
ILoggerFactory loggerFactory)
: base(documentMappingService, razorSemanticTokensLegendService, csharpSemanticTokensProvider: null!, languageServerFeatureOptions, loggerFactory)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public async Task InitializeRazorSemanticAsync()
{
EnsureServicesInitialized();

var loggerFactory = RazorLanguageServer.GetRequiredService<IRazorLoggerFactory>();
var loggerFactory = RazorLanguageServer.GetRequiredService<ILoggerFactory>();

var projectRoot = Path.Combine(RepoRoot, "src", "Razor", "test", "testapps", "ComponentApp");
ProjectFilePath = Path.Combine(projectRoot, "ComponentApp.csproj");
Expand Down Expand Up @@ -159,7 +159,7 @@ public TestCustomizableRazorSemanticTokensInfoService(
LanguageServerFeatureOptions languageServerFeatureOptions,
IRazorDocumentMappingService documentMappingService,
RazorSemanticTokensLegendService razorSemanticTokensLegendService,
IRazorLoggerFactory loggerFactory)
ILoggerFactory loggerFactory)
: base(documentMappingService, razorSemanticTokensLegendService, csharpSemanticTokensProvider: null!, languageServerFeatureOptions, loggerFactory)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
<PackageReference Include="BenchmarkDotNet" Version="$(BenchmarkDotNetPackageVersion)" />
<PackageReference Include="Moq" Version="$(MoqPackageVersion)" />
<PackageReference Include="BenchmarkDotNet.Diagnostics.Windows" Version="$(BenchmarkDotNetPackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsPackageVersion)" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="$(MicrosoftExtensionsPackageVersion)" />
<PackageReference Include="Microsoft.DiaSymReader" Version="$(MicrosoftDiaSymReaderVersion)" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Text;
using Microsoft.Extensions.Logging;
using Moq;

namespace Microsoft.AspNetCore.Razor.Microbenchmarks;
Expand Down Expand Up @@ -63,9 +62,9 @@ protected ProjectSnapshotManagerBenchmarkBase(int documentCount = 100)

Documents = documents.ToImmutable();

var loggerFactoryMock = new Mock<IRazorLoggerFactory>(MockBehavior.Strict);
var loggerFactoryMock = new Mock<ILoggerFactory>(MockBehavior.Strict);
loggerFactoryMock
.Setup(x => x.CreateLogger(It.IsAny<string>()))
.Setup(x => x.GetOrCreateLogger(It.IsAny<string>()))
.Returns(Mock.Of<ILogger>(MockBehavior.Strict));

ErrorReporter = new TestErrorReporter();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer;
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;

namespace Microsoft.AspNetCore.Razor.Microbenchmarks.Serialization;

internal class BenchmarkConfigurationSyncService : IConfigurationSyncService
{
public Task<RazorLSPOptions?> GetLatestOptionsAsync(CancellationToken cancellationToken)
{
return Task.FromResult<RazorLSPOptions?>(null);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Text;
using BenchmarkDotNet.Attributes;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.LanguageServer;
using Microsoft.AspNetCore.Razor.LanguageServer.Completion;
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
using Microsoft.CodeAnalysis.Razor.Completion;
Expand All @@ -24,7 +25,8 @@ public class CompletionListSerializationBenchmark
public CompletionListSerializationBenchmark()
{
var completionService = new LspTagHelperCompletionService();
var optionsMonitor = new BenchmarkOptionsMonitor<RazorLSPOptions>(RazorLSPOptions.Default);
var configurationService = new BenchmarkConfigurationSyncService();
var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);
var tagHelperCompletionProvider = new TagHelperCompletionProvider(completionService, optionsMonitor);

_serializer = JsonSerializer.Create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
using Microsoft.AspNetCore.Razor.Telemetry;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using StreamJsonRpc;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.AspNetCore.Razor.LanguageServer.Formatting;
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer.AutoInsert;
Expand Down Expand Up @@ -41,10 +38,10 @@ internal sealed class AutoClosingTagOnAutoInsertProvider : IOnAutoInsertProvider
);
private static readonly ImmutableHashSet<string> s_voidElementsCaseSensitive = s_voidElements.WithComparer(StringComparer.Ordinal);

private readonly IOptionsMonitor<RazorLSPOptions> _optionsMonitor;
private readonly RazorLSPOptionsMonitor _optionsMonitor;
private readonly ILogger _logger;

public AutoClosingTagOnAutoInsertProvider(IOptionsMonitor<RazorLSPOptions> optionsMonitor, IRazorLoggerFactory loggerFactory)
public AutoClosingTagOnAutoInsertProvider(RazorLSPOptionsMonitor optionsMonitor, ILoggerFactory loggerFactory)
{
if (optionsMonitor is null)
{
Expand All @@ -57,7 +54,7 @@ public AutoClosingTagOnAutoInsertProvider(IOptionsMonitor<RazorLSPOptions> optio
}

_optionsMonitor = optionsMonitor;
_logger = loggerFactory.CreateLogger<IOnAutoInsertProvider>();
_logger = loggerFactory.GetOrCreateLogger<IOnAutoInsertProvider>();
}

public string TriggerCharacter => ">";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,18 @@
using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.AspNetCore.Razor.LanguageServer.Formatting;
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer.AutoInsert;

internal sealed class CloseTextTagOnAutoInsertProvider : IOnAutoInsertProvider
{
private readonly IOptionsMonitor<RazorLSPOptions> _optionsMonitor;
private readonly RazorLSPOptionsMonitor _optionsMonitor;
private readonly ILogger _logger;

public CloseTextTagOnAutoInsertProvider(IOptionsMonitor<RazorLSPOptions> optionsMonitor, IRazorLoggerFactory loggerFactory)
public CloseTextTagOnAutoInsertProvider(RazorLSPOptionsMonitor optionsMonitor, ILoggerFactory loggerFactory)
{
if (optionsMonitor is null)
{
Expand All @@ -35,7 +32,7 @@ public CloseTextTagOnAutoInsertProvider(IOptionsMonitor<RazorLSPOptions> options
}

_optionsMonitor = optionsMonitor;
_logger = loggerFactory.CreateLogger<IOnAutoInsertProvider>();
_logger = loggerFactory.GetOrCreateLogger<IOnAutoInsertProvider>();
}

public string TriggerCharacter => ">";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer.AutoInsert;
Expand All @@ -26,15 +24,15 @@ internal class OnAutoInsertEndpoint(
IRazorDocumentMappingService documentMappingService,
IClientConnection clientConnection,
IEnumerable<IOnAutoInsertProvider> onAutoInsertProvider,
IOptionsMonitor<RazorLSPOptions> optionsMonitor,
IRazorLoggerFactory loggerFactory)
: AbstractRazorDelegatingEndpoint<VSInternalDocumentOnAutoInsertParams, VSInternalDocumentOnAutoInsertResponseItem?>(languageServerFeatureOptions, documentMappingService, clientConnection, loggerFactory.CreateLogger<OnAutoInsertEndpoint>()), ICapabilitiesProvider
RazorLSPOptionsMonitor optionsMonitor,
ILoggerFactory loggerFactory)
: AbstractRazorDelegatingEndpoint<VSInternalDocumentOnAutoInsertParams, VSInternalDocumentOnAutoInsertResponseItem?>(languageServerFeatureOptions, documentMappingService, clientConnection, loggerFactory.GetOrCreateLogger<OnAutoInsertEndpoint>()), ICapabilitiesProvider
{
private static readonly HashSet<string> s_htmlAllowedTriggerCharacters = new(StringComparer.Ordinal) { "=", };
private static readonly HashSet<string> s_cSharpAllowedTriggerCharacters = new(StringComparer.Ordinal) { "'", "/", "\n" };

private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions ?? throw new ArgumentNullException(nameof(languageServerFeatureOptions));
private readonly IOptionsMonitor<RazorLSPOptions> _optionsMonitor = optionsMonitor ?? throw new ArgumentNullException(nameof(optionsMonitor));
private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor ?? throw new ArgumentNullException(nameof(optionsMonitor));
private readonly IReadOnlyList<IOnAutoInsertProvider> _onAutoInsertProviders = onAutoInsertProvider?.ToList() ?? throw new ArgumentNullException(nameof(onAutoInsertProvider));

protected override string CustomMessageTarget => CustomMessageNames.RazorOnAutoInsertEndpointName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer.AutoInsert;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.CodeAnalysis.Razor.Protocol.CodeActions;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Newtonsoft.Json.Linq;
using StreamJsonRpc;
Expand All @@ -36,7 +35,7 @@ internal sealed class CodeActionEndpoint(
IEnumerable<IHtmlCodeActionProvider> htmlCodeActionProviders,
IClientConnection clientConnection,
LanguageServerFeatureOptions languageServerFeatureOptions,
IRazorLoggerFactory loggerFactory,
ILoggerFactory loggerFactory,
ITelemetryReporter? telemetryReporter)
: IRazorRequestHandler<VSCodeActionParams, SumType<Command, CodeAction>[]?>, ICapabilitiesProvider
{
Expand All @@ -48,7 +47,7 @@ internal sealed class CodeActionEndpoint(
private readonly IEnumerable<IHtmlCodeActionProvider> _htmlCodeActionProviders = htmlCodeActionProviders ?? throw new ArgumentNullException(nameof(htmlCodeActionProviders));
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions ?? throw new ArgumentNullException(nameof(languageServerFeatureOptions));
private readonly IClientConnection _clientConnection = clientConnection ?? throw new ArgumentNullException(nameof(clientConnection));
private readonly ILogger _logger = loggerFactory.CreateLogger<CodeActionEndpoint>();
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<CodeActionEndpoint>();
private readonly ITelemetryReporter? _telemetryReporter = telemetryReporter;

internal bool _supportsCodeActionResolve = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Newtonsoft.Json.Linq;

Expand All @@ -31,7 +30,7 @@ public CodeActionResolveEndpoint(
IEnumerable<IRazorCodeActionResolver> razorCodeActionResolvers,
IEnumerable<CSharpCodeActionResolver> csharpCodeActionResolvers,
IEnumerable<HtmlCodeActionResolver> htmlCodeActionResolvers,
IRazorLoggerFactory loggerFactory)
ILoggerFactory loggerFactory)
{
if (razorCodeActionResolvers is null)
{
Expand All @@ -53,7 +52,7 @@ public CodeActionResolveEndpoint(
throw new ArgumentNullException(nameof(loggerFactory));
}

_logger = loggerFactory.CreateLogger<CodeActionResolveEndpoint>();
_logger = loggerFactory.GetOrCreateLogger<CodeActionResolveEndpoint>();

_razorCodeActionResolvers = CreateResolverMap(razorCodeActionResolvers);
_csharpCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(csharpCodeActionResolvers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

Expand All @@ -25,14 +24,14 @@ internal sealed class ExtractToCodeBehindCodeActionProvider : IRazorCodeActionPr
private static readonly Task<IReadOnlyList<RazorVSInternalCodeAction>?> s_emptyResult = Task.FromResult<IReadOnlyList<RazorVSInternalCodeAction>?>(null);
private readonly ILogger _logger;

public ExtractToCodeBehindCodeActionProvider(IRazorLoggerFactory loggerFactory)
public ExtractToCodeBehindCodeActionProvider(ILoggerFactory loggerFactory)
{
if (loggerFactory is null)
{
throw new ArgumentNullException(nameof(loggerFactory));
}

_logger = loggerFactory.CreateLogger<ExtractToCodeBehindCodeActionProvider>();
_logger = loggerFactory.GetOrCreateLogger<ExtractToCodeBehindCodeActionProvider>();
}

public Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(RazorCodeActionContext context, CancellationToken cancellationToken)
Expand Down
Loading

0 comments on commit 8c34fdd

Please sign in to comment.