Skip null loggers when creating loggers in Logger factory.

Author: petrroll

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<LoggerInformation>(_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<T>.Instance as no provider would reasonably return it (the <T> 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)

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<ObjectDisposedException>(() => ((ILoggerFactory) factory).AddProvider(CreateProvider()));
+            Assert.Throws<ObjectDisposedException>(() => ((ILoggerFactory)factory).AddProvider(CreateProvider()));
         }
 
         [Fact]
@@ -214,7 +215,7 @@ public void TestActivityIds(ActivityTrackingOptions options)
         public void TestInvalidActivityTrackingOptions()
         {
             Assert.Throws<ArgumentException>(() =>
-                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<ILoggerProvider>();
             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<ILoggerProvider>();
+            nullProvider.Setup(p => p.CreateLogger(It.IsAny<string>())).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<CustomScopeLoggerProvider.CustomScope>(scope);
+
+            logger.LogInformation("Test message");
+            Assert.Equal(1, validProvider.LogText.Count);
+        }
+
+        private class CustomScopeLoggerProvider : ILoggerProvider, ILogger
+        {
+            public List<string> LogText { get; set; } = new List<string>();
+
+            public void Dispose() { }
+
+            public ILogger CreateLogger(string categoryName) => this;
+
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) => LogText.Add(formatter(state, exception));
+
+            public bool IsEnabled(LogLevel logLevel) => true;
+
+            public IDisposable BeginScope<TState>(TState state) => new CustomScope();
+
+            internal class CustomScope : IDisposable
+            {
+                public void Dispose() { }
+            }
+        }
+
         private class InternalScopeLoggerProvider : ILoggerProvider, ILogger
         {
             private IExternalScopeProvider _scopeProvider = new LoggerExternalScopeProvider();