Merge branch 'main' into vibankwa/redact-http-attributes

OpenTelemetry.sln

@@ -301,6 +301,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\SingletonOptionsManager.cs = src\Shared\Options\SingletonOptionsManager.cs
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shims", "Shims", "{A0CB9A10-F22D-4E66-A449-74B3D0361A9C}"

src/OpenTelemetry/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* Fixed an issue in Logging where unwanted objects (processors, exporters, etc.)
+  could be created inside delegates automatically executed by the Options API
+  during configuration reload.
+  ([#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,13 @@ 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 and IOptionsSnapshot
+        // features for OpenTelemetryLoggerOptions as a workaround to prevent
+        // unwanted objects (processors, exporters, etc.) being created by
+        // configuration delegates being re-run during reload of IConfiguration
+        // or from options created while under scopes.
+        services.DisableOptionsReloading<OpenTelemetryLoggerOptions>();
+
         /* Note: This ensures IConfiguration is available when using
          * IServiceCollections NOT attached to a host. For example when
          * performing:
@@ -192,7 +199,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 +256,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

@@ -81,6 +81,7 @@ public DelegatingOptionsFactory(
     public TOptions Create(string name)
     {
         TOptions options = this.optionsFactoryFunc(this.configuration, name);
+
         foreach (IConfigureOptions<TOptions> setup in _setups)
         {
             if (setup is IConfigureNamedOptions<TOptions> namedSetup)

src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs

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

src/Shared/Options/SingletonOptionsManager.cs

@@ -0,0 +1,47 @@
+// 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 SingletonOptionsManager<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> : IOptionsMonitor<TOptions>, IOptionsSnapshot<TOptions>
+#else
+internal sealed class SingletonOptionsManager<TOptions> : IOptionsMonitor<TOptions>, IOptionsSnapshot<TOptions>
+#endif
+    where TOptions : class
+{
+    private readonly TOptions instance;
+
+    public SingletonOptionsManager(IOptions<TOptions> options)
+    {
+        this.instance = options.Value;
+    }
+
+    public TOptions CurrentValue => this.instance;
+
+    public TOptions Value => this.instance;
+
+    public TOptions Get(string? name) => this.instance;
+
+    public IDisposable? OnChange(Action<TOptions, string?> listener)
+        => NoopChangeNotification.Instance;
+
+    private sealed class NoopChangeNotification : IDisposable
+    {
+        private NoopChangeNotification()
+        {
+        }
+
+        public static NoopChangeNotification Instance { get; } = new();
+
+        public void Dispose()
+        {
+        }
+    }
+}

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,100 @@ public void CircularReferenceTest(bool requestLoggerProviderDirectly)
         Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency);
     }
 
-    private class TestLogProcessor : BaseProcessor<LogRecord>
+    [Theory]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    [InlineData(false, false)]
+    public void OptionReloadingTest(bool useOptionsMonitor, bool useOptionsSnapshot)
+    {
+        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 =>
+            {
+                delegateInvocationCount++;
+
+                options.AddProcessor(new TestLogProcessor());
+            }));
+
+        using var sp = services.BuildServiceProvider();
+
+        if (useOptionsMonitor)
+        {
+            var optionsMonitor = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>();
+
+            Assert.NotNull(optionsMonitor.CurrentValue);
+        }
+
+        if (useOptionsSnapshot)
+        {
+            using var scope = sp.CreateScope();
+
+            var optionsSnapshot = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<OpenTelemetryLoggerOptions>>();
+
+            Assert.NotNull(optionsSnapshot.Value);
+        }
+
+        var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
+
+        Assert.Equal(1, delegateInvocationCount);
+
+        root.Reload();
+
+        Assert.Equal(1, delegateInvocationCount);
+    }
+
+    [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));
+
+        using var scope = sp.CreateScope();
+
+        var optionsSnapshot = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<OpenTelemetryLoggerOptions>>().Value;
+        Assert.True(ReferenceEquals(options, optionsSnapshot));
+    }
+
+    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;