diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index e7f29d6ded3..8726d2daf01 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Added an `IServiceCollection.ConfigureOpenTelemetryMeterProvider` overload + which may be used to configure `MeterProviderBuilder`s while the + `IServiceCollection` is modifiable (before the `IServiceProvider` has been + created). + ([#4517](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4517)) + ## 1.5.0-rc.1 Released 2023-May-25 @@ -18,12 +24,6 @@ Released 2023-May-25 created). ([#4508](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4508)) -* Added an `IServiceCollection.ConfigureOpenTelemetryMeterProvider` overload - which may be used to configure `MeterProviderBuilder`s while the - `IServiceCollection` is modifiable (before the `IServiceProvider` has been - created). - ([#4517](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4517)) - ## 1.5.0-alpha.2 Released 2023-Mar-31 diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/LoggerProviderServiceCollectionBuilder.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/LoggerProviderServiceCollectionBuilder.cs new file mode 100644 index 00000000000..e91d1dd75e1 --- /dev/null +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/LoggerProviderServiceCollectionBuilder.cs @@ -0,0 +1,81 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.Extensions.DependencyInjection; +using OpenTelemetry.Internal; + +namespace OpenTelemetry.Logs; + +internal sealed class LoggerProviderServiceCollectionBuilder : LoggerProviderBuilder, ILoggerProviderBuilder +{ + public LoggerProviderServiceCollectionBuilder(IServiceCollection services) + { + services.ConfigureOpenTelemetryLoggerProvider((sp, builder) => this.Services = null); + + this.Services = services; + } + + public IServiceCollection? Services { get; set; } + + public LoggerProvider? Provider => null; + + /// + public override LoggerProviderBuilder AddInstrumentation(Func instrumentationFactory) + { + Guard.ThrowIfNull(instrumentationFactory); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddInstrumentation(instrumentationFactory); + }); + + return this; + } + + /// + public LoggerProviderBuilder ConfigureServices(Action configure) + => this.ConfigureServicesInternal(configure); + + /// + public LoggerProviderBuilder ConfigureBuilder(Action configure) + => this.ConfigureBuilderInternal(configure); + + /// + LoggerProviderBuilder IDeferredLoggerProviderBuilder.Configure(Action configure) + => this.ConfigureBuilderInternal(configure); + + private LoggerProviderBuilder ConfigureBuilderInternal(Action configure) + { + var services = this.Services + ?? throw new NotSupportedException("Builder cannot be configured during LoggerProvider construction."); + + services.ConfigureOpenTelemetryLoggerProvider(configure); + + return this; + } + + private LoggerProviderBuilder ConfigureServicesInternal(Action configure) + { + Guard.ThrowIfNull(configure); + + var services = this.Services + ?? throw new NotSupportedException("Services cannot be configured during LoggerProvider construction."); + + configure(services); + + return this; + } +} diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggingServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggingServiceCollectionExtensions.cs index d0542ddcc32..159b6365096 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggingServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Logs/OpenTelemetryDependencyInjectionLoggingServiceCollectionExtensions.cs @@ -26,9 +26,7 @@ internal static class OpenTelemetryDependencyInjectionLoggingServiceCollectionEx { /// /// Registers an action used to configure the OpenTelemetry used to create the for the being - /// configured. + /// cref="LoggerProviderBuilder"/>. /// /// /// Notes: @@ -36,34 +34,71 @@ internal static class OpenTelemetryDependencyInjectionLoggingServiceCollectionEx /// This is safe to be called multiple times and by library authors. /// Each registered configuration action will be applied /// sequentially. - /// A will not be created automatically - /// using this method. To begin collecting metrics use the + /// A will NOT be created automatically + /// using this method. To begin collecting logs use the /// IServiceCollection.AddOpenTelemetry extension in the /// OpenTelemetry.Extensions.Hosting package. /// /// - /// The to add - /// services to. + /// . /// Callback action to configure the . /// The so that additional calls /// can be chained. public static IServiceCollection ConfigureOpenTelemetryLoggerProvider( this IServiceCollection services, - Action configure) + Action configure) { - RegisterBuildAction(services, configure); + Guard.ThrowIfNull(services); + Guard.ThrowIfNull(configure); + + configure(new LoggerProviderServiceCollectionBuilder(services)); return services; } - private static void RegisterBuildAction(IServiceCollection services, Action configure) + /// + /// Registers an action used to configure the OpenTelemetry once the + /// is available. + /// + /// + /// Notes: + /// + /// This is safe to be called multiple times and by library authors. + /// Each registered configuration action will be applied + /// sequentially. + /// A will NOT be created automatically + /// using this method. To begin collecting logs use the + /// IServiceCollection.AddOpenTelemetry extension in the + /// OpenTelemetry.Extensions.Hosting package. + /// The supplied configuration delegate is called once the is available. Services may NOT be added to a + /// once the has been created. Many helper extensions + /// register services and may throw if invoked inside the configuration + /// delegate. If you don't need access to the + /// call instead which is safe to be used with + /// helper extensions. + /// + /// + /// . + /// Callback action to configure the . + /// The so that additional calls + /// can be chained. + public static IServiceCollection ConfigureOpenTelemetryLoggerProvider( + this IServiceCollection services, + Action configure) { Guard.ThrowIfNull(services); Guard.ThrowIfNull(configure); services.AddSingleton( new ConfigureLoggerProviderBuilderCallbackWrapper(configure)); + + return services; } private sealed class ConfigureLoggerProviderBuilderCallbackWrapper : IConfigureLoggerProviderBuilder diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 8ea31498630..b0cc40eadec 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -171,7 +171,7 @@ internal OpenTelemetryBuilder WithLogging(Action configur // Note: This enables ILogger integration this.Services.AddLogging().AddOpenTelemetry(); - var builder = new LoggerProviderServiceCollectionBuilder(this.Services); + var builder = new LoggerProviderBuilderBase(this.Services); configure(builder); diff --git a/src/OpenTelemetry/Logs/Builder/LoggerProviderServiceCollectionBuilder.cs b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs similarity index 60% rename from src/OpenTelemetry/Logs/Builder/LoggerProviderServiceCollectionBuilder.cs rename to src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs index f90a387714a..87bf3144708 100644 --- a/src/OpenTelemetry/Logs/Builder/LoggerProviderServiceCollectionBuilder.cs +++ b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,20 +23,17 @@ namespace OpenTelemetry.Logs; /// -/// Contains methods for registering actions into an which will be used to build a once the is -/// available. +/// Contains methods for building instances. /// -internal sealed class LoggerProviderServiceCollectionBuilder : LoggerProviderBuilder, ILoggerProviderBuilder +internal sealed class LoggerProviderBuilderBase : LoggerProviderBuilder, ILoggerProviderBuilder { private readonly bool allowBuild; - private IServiceCollection? services; + private readonly LoggerProviderServiceCollectionBuilder innerBuilder; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public LoggerProviderServiceCollectionBuilder() + public LoggerProviderBuilderBase() { var services = new ServiceCollection(); @@ -46,14 +43,12 @@ public LoggerProviderServiceCollectionBuilder() .TryAddSingleton( sp => throw new NotSupportedException("Self-contained LoggerProvider cannot be accessed using the application IServiceProvider call Build instead.")); - services.ConfigureOpenTelemetryLoggerProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new LoggerProviderServiceCollectionBuilder(services); this.allowBuild = true; } - internal LoggerProviderServiceCollectionBuilder(IServiceCollection services) + internal LoggerProviderBuilderBase(IServiceCollection services) { Guard.ThrowIfNull(services); @@ -61,9 +56,7 @@ internal LoggerProviderServiceCollectionBuilder(IServiceCollection services) .AddOpenTelemetryLoggerProviderBuilderServices() .TryAddSingleton(sp => new LoggerProviderSdk(sp, ownsServiceProvider: false)); - services.ConfigureOpenTelemetryLoggerProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new LoggerProviderServiceCollectionBuilder(services); this.allowBuild = false; } @@ -74,23 +67,26 @@ internal LoggerProviderServiceCollectionBuilder(IServiceCollection services) /// public override LoggerProviderBuilder AddInstrumentation(Func instrumentationFactory) { - Guard.ThrowIfNull(instrumentationFactory); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddInstrumentation(instrumentationFactory); - }); + this.innerBuilder.AddInstrumentation(instrumentationFactory); return this; } /// LoggerProviderBuilder ILoggerProviderBuilder.ConfigureServices(Action configure) - => this.ConfigureServicesInternal(configure); + { + this.innerBuilder.ConfigureServices(configure); + + return this; + } /// LoggerProviderBuilder IDeferredLoggerProviderBuilder.Configure(Action configure) - => this.ConfigureBuilderInternal(configure); + { + this.innerBuilder.ConfigureBuilder(configure); + + return this; + } internal LoggerProvider Build() { @@ -99,10 +95,10 @@ internal LoggerProvider Build() throw new NotSupportedException("A LoggerProviderBuilder bound to external service cannot be built directly. Access the LoggerProvider using the application IServiceProvider instead."); } - var services = this.services + var services = this.innerBuilder.Services ?? throw new NotSupportedException("LoggerProviderBuilder build method cannot be called multiple times."); - this.services = null; + this.innerBuilder.Services = null; #if DEBUG bool validateScopes = true; @@ -113,26 +109,4 @@ internal LoggerProvider Build() return new LoggerProviderSdk(serviceProvider, ownsServiceProvider: true); } - - private LoggerProviderBuilder ConfigureBuilderInternal(Action configure) - { - var services = this.services - ?? throw new NotSupportedException("Builder cannot be configured during LoggerProvider construction."); - - services.ConfigureOpenTelemetryLoggerProvider(configure); - - return this; - } - - private LoggerProviderBuilder ConfigureServicesInternal(Action configure) - { - Guard.ThrowIfNull(configure); - - var services = this.services - ?? throw new NotSupportedException("Services cannot be configured during LoggerProvider construction."); - - configure(services); - - return this; - } } diff --git a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderExtensions.cs b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderExtensions.cs index 78364924e03..749979eb286 100644 --- a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderExtensions.cs @@ -151,9 +151,9 @@ public static LoggerProviderBuilder AddProcessor( /// . public static LoggerProvider Build(this LoggerProviderBuilder loggerProviderBuilder) { - if (loggerProviderBuilder is LoggerProviderServiceCollectionBuilder loggerProviderServiceCollectionBuilder) + if (loggerProviderBuilder is LoggerProviderBuilderBase loggerProviderBuilderBase) { - return loggerProviderServiceCollectionBuilder.Build(); + return loggerProviderBuilderBase.Build(); } return new LoggerProvider(); diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 6f63fce672a..bfd6e620258 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -51,7 +51,7 @@ public static ILoggingBuilder AddOpenTelemetry( // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions LoggerProviderOptions.RegisterProviderOptions(builder.Services); - new LoggerProviderServiceCollectionBuilder(builder.Services).ConfigureBuilder( + new LoggerProviderBuilderBase(builder.Services).ConfigureBuilder( (sp, logging) => { var options = sp.GetRequiredService>().CurrentValue; diff --git a/src/OpenTelemetry/Sdk.cs b/src/OpenTelemetry/Sdk.cs index 0258018b6e1..d52ac7b9f5d 100644 --- a/src/OpenTelemetry/Sdk.cs +++ b/src/OpenTelemetry/Sdk.cs @@ -96,7 +96,7 @@ public static TracerProviderBuilder CreateTracerProviderBuilder() /// to build a . internal static LoggerProviderBuilder CreateLoggerProviderBuilder() { - return new LoggerProviderServiceCollectionBuilder(); + return new LoggerProviderBuilderBase(); } } } diff --git a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs index 82f141935f8..860b31ff411 100644 --- a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs @@ -100,25 +100,32 @@ public void ConfigureOpenTelemetryMeterProvider(int numberOfCalls) [InlineData(3)] public void ConfigureOpenTelemetryLoggerProvider(int numberOfCalls) { - var invocations = 0; + var beforeServiceProviderInvocations = 0; + var afterServiceProviderInvocations = 0; var services = new ServiceCollection(); for (int i = 0; i < numberOfCalls; i++) { - services.ConfigureOpenTelemetryLoggerProvider((sp, builder) => invocations++); + services.ConfigureOpenTelemetryLoggerProvider(builder => beforeServiceProviderInvocations++); + services.ConfigureOpenTelemetryLoggerProvider((sp, builder) => afterServiceProviderInvocations++); } using var serviceProvider = services.BuildServiceProvider(); var registrations = serviceProvider.GetServices(); + Assert.Equal(numberOfCalls, beforeServiceProviderInvocations); + Assert.Equal(0, afterServiceProviderInvocations); + foreach (var registration in registrations) { registration.ConfigureBuilder(serviceProvider, null!); } - Assert.Equal(invocations, registrations.Count()); - Assert.Equal(numberOfCalls, registrations.Count()); + Assert.Equal(numberOfCalls, beforeServiceProviderInvocations); + Assert.Equal(numberOfCalls, afterServiceProviderInvocations); + + Assert.Equal(numberOfCalls * 2, registrations.Count()); } } diff --git a/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs index 0783f8b90b8..022f0c5bc01 100644 --- a/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs @@ -63,6 +63,7 @@ public void LoggerProviderBuilderNestedResolutionUsingBuilderTest(bool callNeste { bool innerConfigureBuilderTestExecuted = false; bool innerConfigureOpenTelemetryLoggerProviderTestExecuted = false; + bool innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = false; using var provider = Sdk.CreateLoggerProviderBuilder() .ConfigureServices(services => @@ -70,7 +71,17 @@ public void LoggerProviderBuilderNestedResolutionUsingBuilderTest(bool callNeste if (callNestedConfigure) { services.ConfigureOpenTelemetryLoggerProvider( - (sp, builder) => innerConfigureOpenTelemetryLoggerProviderTestExecuted = true); + builder => + { + innerConfigureOpenTelemetryLoggerProviderTestExecuted = true; + builder.AddInstrumentation(); + }); + services.ConfigureOpenTelemetryLoggerProvider( + (sp, builder) => + { + innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = true; + Assert.Throws(() => builder.AddInstrumentation()); + }); } }) .ConfigureBuilder((sp, builder) => @@ -78,10 +89,22 @@ public void LoggerProviderBuilderNestedResolutionUsingBuilderTest(bool callNeste innerConfigureBuilderTestExecuted = true; Assert.Throws(() => sp.GetService()); }) - .Build(); + .Build() as LoggerProviderSdk; + + Assert.NotNull(provider); Assert.True(innerConfigureBuilderTestExecuted); Assert.Equal(callNestedConfigure, innerConfigureOpenTelemetryLoggerProviderTestExecuted); + Assert.Equal(callNestedConfigure, innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted); + + if (callNestedConfigure) + { + Assert.Single(provider.Instrumentations); + } + else + { + Assert.Empty(provider.Instrumentations); + } Assert.Throws(() => provider.GetServiceProvider()?.GetService()); } diff --git a/test/OpenTelemetry.Tests/Logs/LoggerProviderSdkTests.cs b/test/OpenTelemetry.Tests/Logs/LoggerProviderSdkTests.cs index 3926c1a9edf..dc8ace58797 100644 --- a/test/OpenTelemetry.Tests/Logs/LoggerProviderSdkTests.cs +++ b/test/OpenTelemetry.Tests/Logs/LoggerProviderSdkTests.cs @@ -146,6 +146,29 @@ public void AddProcessorTest() Assert.True(provider.Processor is CompositeProcessor); } + [Fact] + public void BuilderTypeDoesNotChangeTest() + { + var originalBuilder = Sdk.CreateLoggerProviderBuilder(); + var currentBuilder = originalBuilder; + + var deferredBuilder = currentBuilder as IDeferredLoggerProviderBuilder; + Assert.NotNull(deferredBuilder); + + currentBuilder = deferredBuilder.Configure((sp, innerBuilder) => { }); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + currentBuilder = currentBuilder.ConfigureServices(s => { }); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + currentBuilder = currentBuilder.AddInstrumentation(() => new object()); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + using var provider = currentBuilder.Build(); + + Assert.NotNull(provider); + } + private sealed class NoopProcessor : BaseProcessor { }