diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index af084b8d65e..2c8643b55f3 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -34,6 +34,15 @@ Notes](../../RELEASENOTES.md). 10 million values. ([#7165](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7165)) +* Fixed a circular reference which could cause a `LoggerProvider` to fail to + resolve when one of its dependencies depends on `ILogger` or `ILoggerFactory`. + As part of this fix the `LoggerProvider` resolved from dependency injection + is now created lazily when the first logger is created rather than when + `ILoggerProvider` or `ILoggerFactory` is resolved. A consequence is that any + invalid configuration now surfaces when the first log record is written instead + of when the logging services are resolved. + ([#7308](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7308)) + ## 1.15.3 Released 2026-Apr-21 diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs index 7283fd357d8..fce1770df0a 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs @@ -15,9 +15,11 @@ namespace OpenTelemetry.Logs; [ProviderAlias("OpenTelemetry")] public class OpenTelemetryLoggerProvider : BaseProvider, ILoggerProvider, ISupportExternalScope { - internal readonly LoggerProvider Provider; + private readonly Lock syncObject = new(); private readonly bool ownsProvider; private readonly Hashtable loggers = []; + private Func? loggerProviderFactory; + private LoggerProvider? provider; private bool disposed; static OpenTelemetryLoggerProvider() @@ -40,7 +42,7 @@ public OpenTelemetryLoggerProvider(IOptionsMonitor o var optionsInstance = options.CurrentValue; #pragma warning restore CA1062 // Validate arguments of public methods - needed for netstandard2.1 - this.Provider = Sdk + this.provider = Sdk .CreateLoggerProviderBuilder() .ConfigureBuilder((sp, builder) => { @@ -61,15 +63,48 @@ public OpenTelemetryLoggerProvider(IOptionsMonitor o } internal OpenTelemetryLoggerProvider( - LoggerProvider loggerProvider, + Func loggerProviderFactory, OpenTelemetryLoggerOptions options, bool disposeProvider) { - this.Provider = loggerProvider; + Guard.ThrowIfNull(loggerProviderFactory); + + this.loggerProviderFactory = loggerProviderFactory; this.Options = options.Copy(); this.ownsProvider = disposeProvider; } + internal LoggerProvider Provider + { + get + { + // Volatile.Read/Write are used to make the lock-free fast path + // correct on weaker memory models, guaranteeing the provider + // is fully constructed before it is observed by another thread. + var provider = Volatile.Read(ref this.provider); + if (provider != null) + { + return provider; + } + + lock (this.syncObject) + { + provider = this.provider; + if (provider == null) + { + // If the factory throws (e.g. an invalid configuration) + // it is intentionally left in place so the next access + // retries instead of caching the failure. + provider = this.loggerProviderFactory!(); + Volatile.Write(ref this.provider, provider); + this.loggerProviderFactory = null; + } + + return provider; + } + } + } + internal OpenTelemetryLoggerOptions Options { get; } internal IExternalScopeProvider? ScopeProvider { get; private set; } @@ -130,7 +165,7 @@ protected override void Dispose(bool disposing) { if (this.ownsProvider) { - this.Provider.Dispose(); + this.provider?.Dispose(); } } diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index e61434ea7ec..ec626e0eac8 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -196,38 +196,41 @@ private static ILoggingBuilder AddOpenTelemetryInternal( { var state = sp.GetRequiredService(); - var provider = state.Provider; - if (provider == null) - { - /* - * Note: - * - * There is a possibility of a circular reference when - * accessing LoggerProvider from the IServiceProvider. - * - * If LoggerProvider is the first thing accessed, and it - * requires some service which accesses ILogger (for - * example, IHttpClientFactory), then the - * OpenTelemetryLoggerProvider will try to access a new - * (second) LoggerProvider while still in the process of - * building the first one: - * - * LoggerProvider -> IHttpClientFactory -> - * ILoggerFactory -> OpenTelemetryLoggerProvider -> - * LoggerProvider - * - * This check uses the provider reference captured on - * LoggerProviderBuilderSdk during construction of - * LoggerProviderSdk to detect if a provider has already - * been created to give to OpenTelemetryLoggerProvider - * and stop the loop. - */ - provider = sp.GetRequiredService(); - Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider."); - } - return new OpenTelemetryLoggerProvider( - provider, + () => + { + var provider = state.Provider; + if (provider != null) + { + return provider; + } + + /* + * Note: + * + * There is a possibility of a circular reference when + * accessing LoggerProvider from the IServiceProvider. + * + * If ILoggerFactory is the first thing accessed while + * LoggerProvider is still being built, then + * OpenTelemetryLoggerProvider.CreateLogger will need + * to use the provider that is already under + * construction instead of asking DI to create a + * second provider: + * + * LoggerProvider -> IHttpClientFactory -> + * ILoggerFactory -> OpenTelemetryLoggerProvider -> + * LoggerProvider + * + * This check uses the provider reference captured on + * LoggerProviderBuilderSdk during construction of + * LoggerProviderSdk to detect if a provider has + * already been created and stop the loop. + */ + provider = sp.GetRequiredService(); + Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider."); + return provider; + }, sp.GetRequiredService>().Value, disposeProvider: false); })); diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs index 065ee32a5b3..eeb380f40e4 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs @@ -59,15 +59,18 @@ public void OtlpExporter_Throws_OnGrpcWithDefaultFactory_ForLogging() using var sp = services.BuildServiceProvider(); - Assert.Throws(sp.GetRequiredService); + var loggerProvider = sp.GetRequiredService(); - Assert.Throws(() => LoggerFactory.Create(builder => + using var loggerFactory = LoggerFactory.Create(builder => { builder.AddOpenTelemetry(logging => { logging.AddOtlpExporter(o => o.Protocol = OtlpExportProtocol.Grpc); }); - })); + }); + + Assert.Throws(() => loggerProvider.CreateLogger("MyLogger")); + Assert.Throws(() => loggerFactory.CreateLogger("MyLogger")); } [Fact] diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index c38f941607b..4ae4230280e 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -1826,6 +1826,10 @@ private static void RunVerifyEnvironmentVariablesTakenFromIConfigurationTest( Assert.NotNull(factory); + var logger = factory.CreateLogger("TestLogger"); + + Assert.NotNull(logger); + Assert.True(configureDelegateCalled); Assert.True(configureExportProcessorOptionsCalled); Assert.True(configureBatchOptionsCalled); diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index 65de1e7b71a..f1b5e95ccfd 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -155,7 +155,7 @@ public void UseOpenTelemetryDependencyInjectionTest() Assert.NotNull(loggerProvider.Processor); - Assert.True(loggerProvider.Processor is TestLogProcessor); + Assert.IsType(loggerProvider.Processor); } [Fact] @@ -245,7 +245,7 @@ public void VerifyAddProcessorOverloadWithImplementationFactory() // assert Assert.NotNull(loggerProvider); Assert.NotNull(loggerProvider.Processor); - Assert.True(loggerProvider.Processor is TestLogProcessor); + Assert.IsType(loggerProvider.Processor); } [Fact] @@ -270,6 +270,11 @@ public void VerifyExceptionIsThrownWhenImplementationFactoryIsNull() [InlineData(false)] public void CircularReferenceTest(bool requestLoggerProviderDirectly) { + // Note: This test exercises the deferred LoggerProvider resolution path + // that breaks the circular reference fixed by + // https://github.com/open-telemetry/opentelemetry-dotnet/pull/7308. The + // re-entrant resolution it guards against only throws under the stricter + // circular dependency detection in .NET 11's dependency injection container. var services = new ServiceCollection(); services.AddLogging(logging => logging.AddOpenTelemetry()); @@ -287,13 +292,15 @@ public void CircularReferenceTest(bool requestLoggerProviderDirectly) { var factory = sp.GetRequiredService(); Assert.NotNull(factory); + + var logger = factory.CreateLogger("MyLogger"); + Assert.NotNull(logger); } var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; Assert.NotNull(loggerProvider); - - Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency); + Assert.IsType(loggerProvider.Processor); } [Theory]