[logs] Mitigate unwanted object creation during configuration reload

OpenTelemetry.sln

@@ -300,6 +300,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Options", "Options", "{4949
 	ProjectSection(SolutionItems) = preProject
 		src\Shared\Options\DelegatingOptionsFactory.cs = src\Shared\Options\DelegatingOptionsFactory.cs
 		src\Shared\Options\DelegatingOptionsFactoryServiceCollectionExtensions.cs = src\Shared\Options\DelegatingOptionsFactoryServiceCollectionExtensions.cs
+		src\Shared\Options\SingletonOptionsMonitor.cs = src\Shared\Options\SingletonOptionsMonitor.cs
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shims", "Shims", "{A0CB9A10-F22D-4E66-A449-74B3D0361A9C}"

src/OpenTelemetry/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+* New instances of `OpenTelemetryLoggerOptions` will no longer be created during
+  configuration reload(s). This was done to prevent the accumulation of extra
+  background threads created by additional `BatchLogRecordExportProcessor`s
+  added during reload which are never used.
+  ([#5514](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5514))
+
 ## 1.8.0
 
 Released 2024-Apr-02

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs

@@ -117,6 +117,7 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui
         Guard.ThrowIfNull(resourceBuilder);
 
         this.ResourceBuilder = resourceBuilder;
+
         return this;
     }
 

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs

@@ -173,6 +173,11 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
         // Note: This will bind logger options element (e.g., "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions
         RegisterLoggerProviderOptions(services);
 
+        // Note: We disable built-in IOptionsMonitor features for
+        // OpenTelemetryLoggerOptions to prevent leaks of batch processors added
+        // by configuration delegates during reload of IConfiguration.
+        services.DisableOptionsMonitor<OpenTelemetryLoggerOptions>();
+
         /* Note: This ensures IConfiguration is available when using
          * IServiceCollections NOT attached to a host. For example when
          * performing:
@@ -192,7 +197,7 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
         var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder(
             (sp, logging) =>
             {
-                var options = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue;
+                var options = sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value;
 
                 if (options.ResourceBuilder != null)
                 {
@@ -249,7 +254,7 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
 
                     return new OpenTelemetryLoggerProvider(
                         provider,
-                        sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
+                        sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value,
                         disposeProvider: false);
                 }));
 

src/Shared/Options/DelegatingOptionsFactory.cs

@@ -28,9 +28,9 @@ namespace Microsoft.Extensions.Options;
 /// </summary>
 /// <typeparam name="TOptions">The type of options being requested.</typeparam>
 #if NET6_0_OR_GREATER
-internal sealed class DelegatingOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
+internal class DelegatingOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
 #else
-internal sealed class DelegatingOptionsFactory<TOptions> :
+internal class DelegatingOptionsFactory<TOptions> :
 #endif
     IOptionsFactory<TOptions>
     where TOptions : class
@@ -78,9 +78,17 @@ public DelegatingOptionsFactory(
     /// <returns>The created <typeparamref name="TOptions"/> instance with the given <paramref name="name"/>.</returns>
     /// <exception cref="OptionsValidationException">One or more <see cref="IValidateOptions{TOptions}"/> return failed <see cref="ValidateOptionsResult"/> when validating the <typeparamref name="TOptions"/> instance been created.</exception>
     /// <exception cref="MissingMethodException">The <typeparamref name="TOptions"/> does not have a public parameterless constructor or <typeparamref name="TOptions"/> is <see langword="abstract"/>.</exception>
-    public TOptions Create(string name)
+    public virtual TOptions Create(string name)
     {
         TOptions options = this.optionsFactoryFunc(this.configuration, name);
+
+        RunConfigurationsAndValidations(name, options);
+
+        return options;
+    }
+
+    protected void RunConfigurationsAndValidations(string name, TOptions options)
+    {
         foreach (IConfigureOptions<TOptions> setup in _setups)
         {
             if (setup is IConfigureNamedOptions<TOptions> namedSetup)
@@ -113,7 +121,5 @@ public TOptions Create(string name)
                 throw new OptionsValidationException(name, typeof(TOptions), failures);
             }
         }
-
-        return options;
     }
 }

src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs

@@ -62,6 +62,21 @@ public static IServiceCollection RegisterOptionsFactory<T>(
                 sp.GetServices<IValidateOptions<T>>());
         });
 
+        return services!;
+    }
+
+#if NET6_0_OR_GREATER
+    public static IServiceCollection DisableOptionsMonitor<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>(
+#else
+    public static IServiceCollection DisableOptionsMonitor<T>(
+#endif
+        this IServiceCollection services)
+        where T : class
+    {
+        Debug.Assert(services != null, "services was null");
+
+        services!.TryAddSingleton<IOptionsMonitor<T>, SingletonOptionsMonitor<T>>();
+
         return services!;
     }
 }

src/Shared/Options/SingletonOptionsMonitor.cs

@@ -0,0 +1,31 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#nullable enable
+
+#if NET6_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
+
+namespace Microsoft.Extensions.Options;
+
+#if NET6_0_OR_GREATER
+internal sealed class SingletonOptionsMonitor<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> : IOptionsMonitor<TOptions>
+#else
+internal sealed class SingletonOptionsMonitor<TOptions> : IOptionsMonitor<TOptions>
+#endif
+    where TOptions : class
+{
+    private readonly TOptions instance;
+
+    public SingletonOptionsMonitor(IOptions<TOptions> options)
+    {
+        this.instance = options.Value;
+    }
+
+    public TOptions CurrentValue => this.instance;
+
+    public TOptions Get(string? name) => this.instance;
+
+    public IDisposable? OnChange(Action<TOptions, string?> listener) => null;
+}

test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs

@@ -7,6 +7,7 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Xunit;
 
 namespace OpenTelemetry.Logs.Tests;
@@ -297,11 +298,114 @@ public void CircularReferenceTest(bool requestLoggerProviderDirectly)
         Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency);
     }
 
-    private class TestLogProcessor : BaseProcessor<LogRecord>
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void OptionReloadingTest(bool useOptionsMonitor)
     {
+        var defaultInstance = new OpenTelemetryLoggerOptions();
+
+        OpenTelemetryLoggerOptions? lastOptions = null;
+        var processors = new List<TestLogProcessor>();
+        var delegateInvocationCount = 0;
+
+        var root = new ConfigurationBuilder().Build();
+
+        var services = new ServiceCollection();
+
+        services.AddSingleton<IConfiguration>(root);
+
+        services.AddLogging(logging => logging
+            .AddConfiguration(root.GetSection("logging"))
+            .AddOpenTelemetry(options =>
+            {
+                Assert.Equal(defaultInstance.IncludeFormattedMessage, options.IncludeFormattedMessage);
+                Assert.Equal(defaultInstance.IncludeScopes, options.IncludeScopes);
+                Assert.Equal(defaultInstance.ParseStateValues, options.ParseStateValues);
+                Assert.Equal(defaultInstance.IncludeAttributes, options.IncludeAttributes);
+                Assert.Equal(defaultInstance.IncludeTraceState, options.IncludeTraceState);
+
+                if (lastOptions != null)
+                {
+                    Assert.True(ReferenceEquals(options, lastOptions));
+                }
+
+                lastOptions = options;
+
+                delegateInvocationCount++;
+                var processor = new TestLogProcessor();
+                processors.Add(processor);
+                options.AddProcessor(processor);
+
+                options.IncludeFormattedMessage = !defaultInstance.IncludeFormattedMessage;
+                options.IncludeScopes = !defaultInstance.IncludeScopes;
+                options.ParseStateValues = !defaultInstance.ParseStateValues;
+                options.IncludeAttributes = !defaultInstance.IncludeAttributes;
+                options.IncludeTraceState = !defaultInstance.IncludeTraceState;
+            }));
+
+        using var sp = services.BuildServiceProvider();
+
+        if (useOptionsMonitor)
+        {
+            var optionsMonitor = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>();
+
+            // Note: Change notification is disabled for OpenTelemetryLoggerOptions.
+            Assert.Null(optionsMonitor.OnChange((o, n) => Assert.Fail()));
+        }
+
+        var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
+
+        Assert.Equal(1, delegateInvocationCount);
+        Assert.Single(processors);
+        Assert.DoesNotContain(processors, p => p.Disposed);
+
+        root.Reload();
+
+        Assert.Equal(1, delegateInvocationCount);
+        Assert.Single(processors);
+        Assert.DoesNotContain(processors, p => p.Disposed);
+    }
+
+    [Fact]
+    public void MixedOptionsUsageTest()
+    {
+        var root = new ConfigurationBuilder().Build();
+
+        var services = new ServiceCollection();
+
+        services.AddSingleton<IConfiguration>(root);
+
+        services.AddLogging(logging => logging
+            .AddConfiguration(root.GetSection("logging"))
+            .AddOpenTelemetry(options =>
+            {
+                options.AddProcessor(new TestLogProcessor());
+            }));
+
+        using var sp = services.BuildServiceProvider();
+
+        var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
+
+        var optionsMonitor = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue;
+        var options = sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value;
+
+        Assert.True(ReferenceEquals(options, optionsMonitor));
+    }
+
+    private sealed class TestLogProcessor : BaseProcessor<LogRecord>
+    {
+        public bool Disposed;
+
+        protected override void Dispose(bool disposing)
+        {
+            this.Disposed = true;
+
+            base.Dispose(disposing);
+        }
     }
 
-    private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
+    private sealed class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
     {
         private readonly ILogger logger;