Add caching to ForwardingLoggerFactory

Author: jviau

src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Concurrent;
 using System.Diagnostics;
 using Microsoft.Azure.WebJobs.Script;
 
@@ -16,8 +17,12 @@ namespace Microsoft.Extensions.Logging
     [DebuggerDisplay(@"InnerFactory = \{ {_inner} \}, ScriptHostState = {_manager.State}")]
     public sealed class ForwardingLoggerFactory : ILoggerFactory
     {
+        private readonly ConcurrentDictionary<string, ForwardingLogger> _loggers = new(StringComparer.Ordinal);
         private readonly ILoggerFactory _inner;
         private readonly IScriptHostManager _manager;
+        private readonly object _sync = new();
+
+        private bool _disposed;
 
         public ForwardingLoggerFactory(ILoggerFactory inner, IScriptHostManager manager)
         {
@@ -34,12 +39,30 @@ public void AddProvider(ILoggerProvider provider)
 
         /// <inheritdoc />
         public ILogger CreateLogger(string categoryName)
-            => new ForwardingLogger(categoryName, _inner.CreateLogger(categoryName), _manager);
+        {
+            ObjectDisposedException.ThrowIf(_disposed, this);
+
+            if (!_loggers.TryGetValue(categoryName, out ForwardingLogger? logger))
+            {
+                lock (_sync)
+                {
+                    if (!_loggers.TryGetValue(categoryName, out logger))
+                    {
+                        ILogger innerLogger = _inner.CreateLogger(categoryName);
+                        logger = new ForwardingLogger(categoryName, innerLogger, _manager);
+                        _loggers[categoryName] = logger;
+                    }
+                }
+            }
+
+            return logger;
+        }
 
         /// <inheritdoc />
         public void Dispose()
         {
-            // no op.
+            // this is just to block further logger creation.
+            _disposed = true;
         }
     }
 }

test/WebJobs.Script.Tests/Diagnostics/ForwardingLoggerFactoryTests.cs

@@ -63,9 +63,26 @@ public void CreateLogger_ReturnsLoggerInstance()
         }
 
         [Fact]
-        public void Dispose_DoesNotThrow()
+        public void CreateLogger_Caching_ReturnsSameInstance()
         {
-            TestHelpers.Act(_factory.Dispose).Should().NotThrow();
+            string categoryName = "TestCategory";
+            _mockInnerFactory.Setup(f => f.CreateLogger(categoryName)).Returns(_mockInnerLogger.Object);
+
+            ILogger result1 = _factory.CreateLogger(categoryName);
+            ILogger result2 = _factory.CreateLogger(categoryName);
+
+            result1.Should().NotBeNull();
+            result1.Should().BeOfType<ForwardingLogger>();
+            result2.Should().BeSameAs(result1);
+            _mockInnerFactory.Verify(f => f.CreateLogger(categoryName), Times.Once);
+        }
+
+        [Fact]
+        public void Dispose_BlocksCreation()
+        {
+            _factory.Dispose();
+            TestHelpers.Act(() => _factory.CreateLogger("TestCategory"))
+                .Should().Throw<ObjectDisposedException>();
         }
     }
 }