From d523140443e6017e741a92abb7d952e946628a61 Mon Sep 17 00:00:00 2001 From: Petr Houska Date: Sun, 6 Jul 2025 12:04:22 +0200 Subject: [PATCH] Skip null loggers when creating loggers in Logger factory. --- .../src/LoggerFactory.cs | 14 ++++- .../tests/Common/LoggerFactoryTest.cs | 60 ++++++++++++++++--- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs index 3ff5e4714613ab..47153ed10d401c 100644 --- a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs +++ b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs @@ -8,6 +8,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; namespace Microsoft.Extensions.Logging @@ -211,12 +212,19 @@ private void AddProviderRegistration(ILoggerProvider provider, bool dispose) private LoggerInformation[] CreateLoggers(string categoryName) { - var loggers = new LoggerInformation[_providerRegistrations.Count]; + var loggers = new List(_providerRegistrations.Count); for (int i = 0; i < _providerRegistrations.Count; i++) { - loggers[i] = new LoggerInformation(_providerRegistrations[i].Provider, categoryName); + var loggerInformation = new LoggerInformation(_providerRegistrations[i].Provider, categoryName); + + // We do not need to check for NullLogger.Instance as no provider would reasonably return it (the handling is at + // outer loggers level, not inner level loggers in Logger/LoggerProvider). + if (loggerInformation.Logger != NullLogger.Instance) + { + loggers.Add(loggerInformation); + } } - return loggers; + return loggers.ToArray(); } private (MessageLogger[] MessageLoggers, ScopeLogger[]? ScopeLoggers) ApplyFilters(LoggerInformation[] loggers) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs index 48b9eb2174f6cf..188dd00d88317a 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Collections.Generic; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using Xunit; @@ -20,7 +21,7 @@ public void AddProvider_ThrowsAfterDisposed() var factory = new LoggerFactory(); factory.Dispose(); - Assert.Throws(() => ((ILoggerFactory) factory).AddProvider(CreateProvider())); + Assert.Throws(() => ((ILoggerFactory)factory).AddProvider(CreateProvider())); } [Fact] @@ -214,7 +215,7 @@ public void TestActivityIds(ActivityTrackingOptions options) public void TestInvalidActivityTrackingOptions() { Assert.Throws(() => - LoggerFactory.Create(builder => { builder.Configure(o => o.ActivityTrackingOptions = (ActivityTrackingOptions) 0xFF00);}) + LoggerFactory.Create(builder => { builder.Configure(o => o.ActivityTrackingOptions = (ActivityTrackingOptions)0xFF00); }) ); } @@ -434,7 +435,7 @@ public void BaggageFormattedOutput() } activity.Stop(); - string [] loggerOutput = new string[] + string[] loggerOutput = new string[] { $"Inside Scope Info!", $"[TraceId, {activity.GetTraceId()}]", @@ -451,7 +452,7 @@ public void BaggageFormattedOutput() public void CallsSetScopeProvider_OnSupportedProviders() { var loggerProvider = new ExternalScopeLoggerProvider(); - var loggerFactory = new LoggerFactory(new [] { loggerProvider }); + var loggerFactory = new LoggerFactory(new[] { loggerProvider }); var logger = loggerFactory.CreateLogger("Logger"); @@ -480,7 +481,7 @@ public void CallsSetScopeProvider_OnSupportedProviders() public void BeginScope_ReturnsExternalSourceTokenDirectly() { var loggerProvider = new ExternalScopeLoggerProvider(); - var loggerFactory = new LoggerFactory(new [] { loggerProvider }); + var loggerFactory = new LoggerFactory(new[] { loggerProvider }); var logger = loggerFactory.CreateLogger("Logger"); @@ -503,7 +504,7 @@ public void BeginScope_ReturnsCompositeToken_ForMultipleLoggers() { var loggerProvider = new ExternalScopeLoggerProvider(); var loggerProvider2 = new InternalScopeLoggerProvider(); - var loggerFactory = new LoggerFactory(new ILoggerProvider[] { loggerProvider, loggerProvider2}); + var loggerFactory = new LoggerFactory(new ILoggerProvider[] { loggerProvider, loggerProvider2 }); var logger = loggerFactory.CreateLogger("Logger"); @@ -543,12 +544,57 @@ public void CreateDisposeDisposesInnerServiceProvider() var provider = new Mock(); provider.Setup(p => p.Dispose()).Callback(() => disposed = true); - var factory = LoggerFactory.Create(builder => builder.Services.AddSingleton(_=> provider.Object)); + var factory = LoggerFactory.Create(builder => builder.Services.AddSingleton(_ => provider.Object)); factory.Dispose(); Assert.True(disposed); } + // Moq heavily utilizes RefEmit, which does not work on most aot workloads + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))] + public void TestCreateLoggers_NullLoggerIsIgnoredWhenReturnedByProvider() + { + // We test this via checking if Scope optimisaion (ie not return scope wrapper but the + // returned scope directly only one logger) is applied. + var nullProvider = new Mock(); + nullProvider.Setup(p => p.CreateLogger(It.IsAny())).Returns(NullLogger.Instance); + + var validProvider = new CustomScopeLoggerProvider(); + + var factory = LoggerFactory.Create(builder => + { + builder.AddProvider(nullProvider.Object); + builder.AddProvider(validProvider); + }); + var logger = factory.CreateLogger("TestLogger"); + + var scope = logger.BeginScope("TestScope"); + Assert.IsType(scope); + + logger.LogInformation("Test message"); + Assert.Equal(1, validProvider.LogText.Count); + } + + private class CustomScopeLoggerProvider : ILoggerProvider, ILogger + { + public List LogText { get; set; } = new List(); + + public void Dispose() { } + + public ILogger CreateLogger(string categoryName) => this; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) => LogText.Add(formatter(state, exception)); + + public bool IsEnabled(LogLevel logLevel) => true; + + public IDisposable BeginScope(TState state) => new CustomScope(); + + internal class CustomScope : IDisposable + { + public void Dispose() { } + } + } + private class InternalScopeLoggerProvider : ILoggerProvider, ILogger { private IExternalScopeProvider _scopeProvider = new LoggerExternalScopeProvider();