From 107dd264c2796b5da0b42eaa6da4055efd03f22c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 10:31:27 -0700 Subject: [PATCH] [di] Expose a detached TracerProviderBuilder extension on IServiceCollection which may modify services. (#4508) --- .../.publicApi/net462/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../CHANGELOG.md | 6 + ...ctionTracingServiceCollectionExtensions.cs | 52 +++++++-- .../TracerProviderServiceCollectionBuilder.cs | 107 ++++++++++++++++++ .../Builder/TracerProviderBuilderBase.cs | 71 ++---------- .../ServiceCollectionExtensionsTests.cs | 19 +++- .../TracerProviderBuilderExtensionsTest.cs | 27 ++++- 8 files changed, 208 insertions(+), 76 deletions(-) create mode 100644 src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/TracerProviderServiceCollectionBuilder.cs diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt index e69de29bb2d..6f215dd80e7 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +static OpenTelemetry.Trace.OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.ConfigureOpenTelemetryTracerProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! \ No newline at end of file diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index e69de29bb2d..6f215dd80e7 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +static OpenTelemetry.Trace.OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.ConfigureOpenTelemetryTracerProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! \ No newline at end of file diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index ac8b87f0d01..7c931eadc82 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -8,6 +8,12 @@ `TracerProvider`. ([#4468](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4468)) +* Added an `IServiceCollection.ConfigureOpenTelemetryTracerProvider` overload + which may be used to configure `TracerProviderBuilder`s while the + `IServiceCollection` is modifiable (before the `IServiceProvider` has been + created). + ([#4508](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4508)) + ## 1.5.0-alpha.2 Released 2023-Mar-31 diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs index af1bd6d0d9f..f20e9f0d253 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs @@ -26,9 +26,7 @@ public static class OpenTelemetryDependencyInjectionTracingServiceCollectionExte { /// /// Registers an action used to configure the OpenTelemetry used to create the for the being - /// configured. + /// cref="TracerProviderBuilder"/>. /// /// /// Notes: @@ -36,34 +34,68 @@ public static class OpenTelemetryDependencyInjectionTracingServiceCollectionExte /// 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 traces 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 ConfigureOpenTelemetryTracerProvider( this IServiceCollection services, - Action configure) + Action configure) { - RegisterBuildAction(services, configure); + Guard.ThrowIfNull(services); + Guard.ThrowIfNull(configure); + + configure(new TracerProviderServiceCollectionBuilder(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 traces 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. + /// + /// + /// . + /// Callback action to configure the . + /// The so that additional calls + /// can be chained. + public static IServiceCollection ConfigureOpenTelemetryTracerProvider( + this IServiceCollection services, + Action configure) { Guard.ThrowIfNull(services); Guard.ThrowIfNull(configure); services.AddSingleton( new ConfigureTracerProviderBuilderCallbackWrapper(configure)); + + return services; } private sealed class ConfigureTracerProviderBuilderCallbackWrapper : IConfigureTracerProviderBuilder diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/TracerProviderServiceCollectionBuilder.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/TracerProviderServiceCollectionBuilder.cs new file mode 100644 index 00000000000..cb2e0d7f0f1 --- /dev/null +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/TracerProviderServiceCollectionBuilder.cs @@ -0,0 +1,107 @@ +// +// 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.Trace; + +internal sealed class TracerProviderServiceCollectionBuilder : TracerProviderBuilder, ITracerProviderBuilder +{ + public TracerProviderServiceCollectionBuilder(IServiceCollection services) + { + services.ConfigureOpenTelemetryTracerProvider((sp, builder) => this.Services = null); + + this.Services = services; + } + + public IServiceCollection? Services { get; set; } + + public TracerProvider? Provider => null; + + /// + public override TracerProviderBuilder AddInstrumentation(Func instrumentationFactory) + { + Guard.ThrowIfNull(instrumentationFactory); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddInstrumentation(instrumentationFactory); + }); + + return this; + } + + /// + public override TracerProviderBuilder AddSource(params string[] names) + { + Guard.ThrowIfNull(names); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddSource(names); + }); + + return this; + } + + /// + public override TracerProviderBuilder AddLegacySource(string operationName) + { + Guard.ThrowIfNullOrWhitespace(operationName); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddLegacySource(operationName); + }); + + return this; + } + + /// + public TracerProviderBuilder ConfigureServices(Action configure) + => this.ConfigureServicesInternal(configure); + + /// + public TracerProviderBuilder ConfigureBuilder(Action configure) + => this.ConfigureBuilderInternal(configure); + + /// + TracerProviderBuilder IDeferredTracerProviderBuilder.Configure(Action configure) + => this.ConfigureBuilderInternal(configure); + + private TracerProviderBuilder ConfigureBuilderInternal(Action configure) + { + var services = this.Services + ?? throw new NotSupportedException("Builder cannot be configured during TracerProvider construction."); + + services.ConfigureOpenTelemetryTracerProvider(configure); + + return this; + } + + private TracerProviderBuilder ConfigureServicesInternal(Action configure) + { + Guard.ThrowIfNull(configure); + + var services = this.Services + ?? throw new NotSupportedException("Services cannot be configured during TracerProvider construction."); + + configure(services); + + return this; + } +} diff --git a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs index a1edc48d960..0a4c2d6d0f8 100644 --- a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs +++ b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs @@ -28,7 +28,7 @@ namespace OpenTelemetry.Trace; public class TracerProviderBuilderBase : TracerProviderBuilder, ITracerProviderBuilder { private readonly bool allowBuild; - private IServiceCollection? services; + private readonly TracerProviderServiceCollectionBuilder innerBuilder; /// /// Initializes a new instance of the class. @@ -43,9 +43,7 @@ public TracerProviderBuilderBase() .TryAddSingleton( sp => throw new NotSupportedException("Self-contained TracerProvider cannot be accessed using the application IServiceProvider call Build instead.")); - services.ConfigureOpenTelemetryTracerProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new TracerProviderServiceCollectionBuilder(services); this.allowBuild = true; } @@ -58,9 +56,7 @@ internal TracerProviderBuilderBase(IServiceCollection services) .AddOpenTelemetryTracerProviderBuilderServices() .TryAddSingleton(sp => new TracerProviderSdk(sp, ownsServiceProvider: false)); - services.ConfigureOpenTelemetryTracerProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new TracerProviderServiceCollectionBuilder(services); this.allowBuild = false; } @@ -71,12 +67,7 @@ internal TracerProviderBuilderBase(IServiceCollection services) /// public override TracerProviderBuilder AddInstrumentation(Func instrumentationFactory) { - Guard.ThrowIfNull(instrumentationFactory); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddInstrumentation(instrumentationFactory); - }); + this.innerBuilder.AddInstrumentation(instrumentationFactory); return this; } @@ -84,12 +75,7 @@ public override TracerProviderBuilder AddInstrumentation(Func< /// public override TracerProviderBuilder AddSource(params string[] names) { - Guard.ThrowIfNull(names); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddSource(names); - }); + this.innerBuilder.AddSource(names); return this; } @@ -97,23 +83,18 @@ public override TracerProviderBuilder AddSource(params string[] names) /// public override TracerProviderBuilder AddLegacySource(string operationName) { - Guard.ThrowIfNullOrWhitespace(operationName); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddLegacySource(operationName); - }); + this.innerBuilder.AddLegacySource(operationName); return this; } /// TracerProviderBuilder ITracerProviderBuilder.ConfigureServices(Action configure) - => this.ConfigureServicesInternal(configure); + => this.innerBuilder.ConfigureServices(configure); /// TracerProviderBuilder IDeferredTracerProviderBuilder.Configure(Action configure) - => this.ConfigureBuilderInternal(configure); + => this.innerBuilder.ConfigureBuilder(configure); internal TracerProvider InvokeBuild() => this.Build(); @@ -134,7 +115,7 @@ protected TracerProviderBuilder AddInstrumentation( Guard.ThrowIfNullOrWhitespace(instrumentationVersion); Guard.ThrowIfNull(instrumentationFactory); - return this.ConfigureBuilderInternal((sp, builder) => + return this.innerBuilder.ConfigureBuilder((sp, builder) => { if (builder is TracerProviderBuilderSdk tracerProviderBuilderState) { @@ -157,14 +138,14 @@ protected TracerProvider Build() throw new NotSupportedException("A TracerProviderBuilder bound to external service cannot be built directly. Access the TracerProvider using the application IServiceProvider instead."); } - var services = this.services; + var services = this.innerBuilder.Services; if (services == null) { throw new NotSupportedException("TracerProviderBuilder build method cannot be called multiple times."); } - this.services = null; + this.innerBuilder.Services = null; #if DEBUG bool validateScopes = true; @@ -175,34 +156,4 @@ protected TracerProvider Build() return new TracerProviderSdk(serviceProvider, ownsServiceProvider: true); } - - private TracerProviderBuilder ConfigureBuilderInternal(Action configure) - { - var services = this.services; - - if (services == null) - { - throw new NotSupportedException("Builder cannot be configured during TracerProvider construction."); - } - - services.ConfigureOpenTelemetryTracerProvider(configure); - - return this; - } - - private TracerProviderBuilder ConfigureServicesInternal(Action configure) - { - Guard.ThrowIfNull(configure); - - var services = this.services; - - if (services == null) - { - throw new NotSupportedException("Services cannot be configured during TracerProvider construction."); - } - - configure(services); - - return this; - } } diff --git a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs index 16fee70d1d4..9a96356b951 100644 --- a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs @@ -30,26 +30,33 @@ public class ServiceCollectionExtensionsTests [InlineData(3)] public void ConfigureOpenTelemetryTracerProvider(int numberOfCalls) { - var invocations = 0; + var beforeServiceProviderInvocations = 0; + var afterServiceProviderInvocations = 0; var services = new ServiceCollection(); for (int i = 0; i < numberOfCalls; i++) { - services.ConfigureOpenTelemetryTracerProvider((sp, builder) => invocations++); + services.ConfigureOpenTelemetryTracerProvider(builder => beforeServiceProviderInvocations++); + services.ConfigureOpenTelemetryTracerProvider((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()); } [Theory] @@ -107,4 +114,8 @@ public void ConfigureOpenTelemetryLoggerProvider(int numberOfCalls) Assert.Equal(invocations, registrations.Count()); Assert.Equal(numberOfCalls, registrations.Count()); } + + private sealed class CustomType + { + } } diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs index 85e5b88552f..3075bdf1c9c 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs @@ -311,6 +311,7 @@ public void TracerProviderNestedResolutionUsingBuilderTest(bool callNestedConfig { bool innerConfigureBuilderTestExecuted = false; bool innerConfigureOpenTelemetryLoggerProviderTestExecuted = false; + bool innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = false; using var provider = Sdk.CreateTracerProviderBuilder() .ConfigureServices(services => @@ -318,7 +319,17 @@ public void TracerProviderNestedResolutionUsingBuilderTest(bool callNestedConfig if (callNestedConfigure) { services.ConfigureOpenTelemetryTracerProvider( - (sp, builder) => innerConfigureOpenTelemetryLoggerProviderTestExecuted = true); + builder => + { + innerConfigureOpenTelemetryLoggerProviderTestExecuted = true; + builder.AddInstrumentation(); + }); + services.ConfigureOpenTelemetryTracerProvider( + (sp, builder) => + { + innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = true; + Assert.Throws(() => builder.AddInstrumentation()); + }); } }) .ConfigureBuilder((sp, builder) => @@ -326,10 +337,22 @@ public void TracerProviderNestedResolutionUsingBuilderTest(bool callNestedConfig innerConfigureBuilderTestExecuted = true; Assert.Throws(() => sp.GetService()); }) - .Build(); + .Build() as TracerProviderSdk; + + 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()); }