From a888b5bcaa1b9d95aa2e0c769471bde05b928811 Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 18 May 2026 17:39:36 +0100 Subject: [PATCH 1/7] [OpenTelemetry] Fix circular reference Fix circular reference when `LoggerProvider` requires `ILogger`. --- .../ILogger/OpenTelemetryLoggerProvider.cs | 39 +++++++++-- .../ILogger/OpenTelemetryLoggingExtensions.cs | 65 ++++++++++--------- .../OpenTelemetryLoggingExtensionsTests.cs | 10 +-- 3 files changed, 74 insertions(+), 40 deletions(-) diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs index 7283fd357d8..0d01741ee13 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,42 @@ 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 + { + var provider = this.provider; + if (provider != null) + { + return provider; + } + + lock (this.syncObject) + { + provider = this.provider; + if (provider == null) + { + provider = this.loggerProviderFactory!(); + this.provider = provider; + this.loggerProviderFactory = null; + } + + return provider; + } + } + } + internal OpenTelemetryLoggerOptions Options { get; } internal IExternalScopeProvider? ScopeProvider { get; private set; } @@ -130,7 +159,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.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index bdd33e10ba2..96ef2d3dc23 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -156,7 +156,7 @@ public void UseOpenTelemetryDependencyInjectionTest() Assert.NotNull(loggerProvider.Processor); - Assert.True(loggerProvider.Processor is TestLogProcessor); + Assert.IsType(loggerProvider.Processor); } [Fact] @@ -246,7 +246,7 @@ public void VerifyAddProcessorOverloadWithImplementationFactory() // assert Assert.NotNull(loggerProvider); Assert.NotNull(loggerProvider.Processor); - Assert.True(loggerProvider.Processor is TestLogProcessor); + Assert.IsType(loggerProvider.Processor); } [Fact] @@ -288,13 +288,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] From 239bb04c96583f0d69044c18a81dd35c0abdb0ad Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 18 May 2026 17:47:05 +0100 Subject: [PATCH 2/7] [OpenTelemetry] Update CHANGELOG Add CHANGELOG entry. --- src/OpenTelemetry/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index b2dd3221336..d019da3d458 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -17,6 +17,9 @@ Notes](../../RELEASENOTES.md). * Update `OpenTelemetrySdkEventSource` to support the W3C randomness flag. ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301)) +* Fix circular reference when `LoggerProvider` requires `ILogger`. + ([#TODO](https://github.com/open-telemetry/opentelemetry-dotnet/pull/TODO)) + ## 1.15.3 Released 2026-Apr-21 From e7b8af6a87d19f84a4f3de19347d0489a29ddd3e Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Mon, 18 May 2026 17:49:18 +0100 Subject: [PATCH 3/7] [OpenTelemetry] Update CHANGELOG Add PR number. --- src/OpenTelemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index d019da3d458..ca85280c7d4 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -18,7 +18,7 @@ Notes](../../RELEASENOTES.md). ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301)) * Fix circular reference when `LoggerProvider` requires `ILogger`. - ([#TODO](https://github.com/open-telemetry/opentelemetry-dotnet/pull/TODO)) + ([#7308](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7308)) ## 1.15.3 From 52d44cbb4b987aa6618887f04f7c046bfc97dfe8 Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 18 May 2026 18:02:48 +0100 Subject: [PATCH 4/7] [OTLP] Fix test Resolve an ILogger to force resolution of dependencies. --- .../OtlpLogExporterTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 1f3b2d70822..bab93f5ec38 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -1727,6 +1727,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); From 6368429901ad201da9750fe1c5af595e0afbc7de Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 18 May 2026 18:28:40 +0100 Subject: [PATCH 5/7] [OTLP] Fix test Fix test that stopped throwing `NotSupportedException` to resolve the `ILoggerProvider` and `LoggerFactory` due to deferred creation. --- .../OtlpExporterHelperExtensionsTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs index 065ee32a5b3..cd6143bcedb 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs @@ -59,15 +59,13 @@ public void OtlpExporter_Throws_OnGrpcWithDefaultFactory_ForLogging() using var sp = services.BuildServiceProvider(); - Assert.Throws(sp.GetRequiredService); - Assert.Throws(() => LoggerFactory.Create(builder => { builder.AddOpenTelemetry(logging => { logging.AddOtlpExporter(o => o.Protocol = OtlpExportProtocol.Grpc); }); - })); + }).CreateLogger("MyLogger")); } [Fact] From a89ceba55dc1db035ba6d2818aec38c7174117cf Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 19 May 2026 10:59:34 +0100 Subject: [PATCH 6/7] [OTLP] Address feedback Update test to assert that creating a logger from the logger provider throws `NotSupportedException`. --- .../OtlpExporterHelperExtensionsTests.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs index cd6143bcedb..eeb380f40e4 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterHelperExtensionsTests.cs @@ -59,13 +59,18 @@ public void OtlpExporter_Throws_OnGrpcWithDefaultFactory_ForLogging() using var sp = services.BuildServiceProvider(); - Assert.Throws(() => LoggerFactory.Create(builder => + var loggerProvider = sp.GetRequiredService(); + + using var loggerFactory = LoggerFactory.Create(builder => { builder.AddOpenTelemetry(logging => { logging.AddOtlpExporter(o => o.Protocol = OtlpExportProtocol.Grpc); }); - }).CreateLogger("MyLogger")); + }); + + Assert.Throws(() => loggerProvider.CreateLogger("MyLogger")); + Assert.Throws(() => loggerFactory.CreateLogger("MyLogger")); } [Fact] From 756742a8918f480677f5d80382a33ead5c3a7c66 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 9 Jun 2026 10:37:28 +0100 Subject: [PATCH 7/7] [OpenTelemetry] Address feedback - Use `Volatile.Read/Write` to access provider. - Do not cache logging provider resolution failures. - Update CHANGELOG entry. - Add explanatory comments. --- src/OpenTelemetry/CHANGELOG.md | 8 +++++++- .../Logs/ILogger/OpenTelemetryLoggerProvider.cs | 10 ++++++++-- .../Logs/OpenTelemetryLoggingExtensionsTests.cs | 5 +++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index a62bc5818e6..503f70126da 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -22,7 +22,13 @@ Notes](../../RELEASENOTES.md). * Update `OpenTelemetrySdkEventSource` to support the W3C randomness flag. ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301)) -* Fix circular reference when `LoggerProvider` requires `ILogger`. +* 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 diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs index 0d01741ee13..fce1770df0a 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerProvider.cs @@ -78,7 +78,10 @@ internal LoggerProvider Provider { get { - var provider = this.provider; + // 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; @@ -89,8 +92,11 @@ internal LoggerProvider Provider 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!(); - this.provider = provider; + Volatile.Write(ref this.provider, provider); this.loggerProviderFactory = null; } diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index 96ef2d3dc23..41c180e4492 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -271,6 +271,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());