From ba8fd47238425f8657e9d62553dd86f93e65bc4e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 31 May 2023 12:52:55 -0700 Subject: [PATCH] [di] Expose a detached MeterProviderBuilder extension on IServiceCollection which may modify services (#4517) --- .../.publicApi/net462/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../CHANGELOG.md | 6 ++ .../MeterProviderServiceCollectionBuilder.cs | 94 +++++++++++++++++++ ...ctionMetricsServiceCollectionExtensions.cs | 53 +++++++++-- ...ctionTracingServiceCollectionExtensions.cs | 5 +- .../Builder/MeterProviderBuilderBase.cs | 76 ++++----------- .../ServiceCollectionExtensionsTests.cs | 19 ++-- .../MeterProviderBuilderExtensionsTests.cs | 27 +++++- .../Metrics/MeterProviderSdkTest.cs | 48 ++++++++++ 10 files changed, 252 insertions(+), 78 deletions(-) create mode 100644 src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs create mode 100644 test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt index 6f215dd80e7..eb84743d0d5 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ +static OpenTelemetry.Metrics.OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.ConfigureOpenTelemetryMeterProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! 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 6f215dd80e7..eb84743d0d5 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ +static OpenTelemetry.Metrics.OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.ConfigureOpenTelemetryMeterProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! 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 156ef19b8c8..e7f29d6ded3 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -18,6 +18,12 @@ 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/Metrics/MeterProviderServiceCollectionBuilder.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs new file mode 100644 index 00000000000..a2ddc2d59a7 --- /dev/null +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs @@ -0,0 +1,94 @@ +// +// 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.Metrics; + +internal sealed class MeterProviderServiceCollectionBuilder : MeterProviderBuilder, IMeterProviderBuilder +{ + public MeterProviderServiceCollectionBuilder(IServiceCollection services) + { + services.ConfigureOpenTelemetryMeterProvider((sp, builder) => this.Services = null); + + this.Services = services; + } + + public IServiceCollection? Services { get; set; } + + public MeterProvider? Provider => null; + + /// + public override MeterProviderBuilder AddInstrumentation(Func instrumentationFactory) + { + Guard.ThrowIfNull(instrumentationFactory); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddInstrumentation(instrumentationFactory); + }); + + return this; + } + + /// + public override MeterProviderBuilder AddMeter(params string[] names) + { + Guard.ThrowIfNull(names); + + this.ConfigureBuilderInternal((sp, builder) => + { + builder.AddMeter(names); + }); + + return this; + } + + /// + public MeterProviderBuilder ConfigureServices(Action configure) + => this.ConfigureServicesInternal(configure); + + /// + public MeterProviderBuilder ConfigureBuilder(Action configure) + => this.ConfigureBuilderInternal(configure); + + /// + MeterProviderBuilder IDeferredMeterProviderBuilder.Configure(Action configure) + => this.ConfigureBuilderInternal(configure); + + private MeterProviderBuilder ConfigureBuilderInternal(Action configure) + { + var services = this.Services + ?? throw new NotSupportedException("Builder cannot be configured during MeterProvider construction."); + + services.ConfigureOpenTelemetryMeterProvider(configure); + + return this; + } + + private MeterProviderBuilder ConfigureServicesInternal(Action configure) + { + Guard.ThrowIfNull(configure); + + var services = this.Services + ?? throw new NotSupportedException("Services cannot be configured during MeterProvider construction."); + + configure(services); + + return this; + } +} diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs index 77955dc9ef0..e62776c6cad 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs @@ -26,9 +26,7 @@ public static class OpenTelemetryDependencyInjectionMetricsServiceCollectionExte { /// /// Registers an action used to configure the OpenTelemetry used to create the for the being - /// configured. + /// cref="MeterProviderBuilder"/>. /// /// /// Notes: @@ -36,34 +34,71 @@ public static class OpenTelemetryDependencyInjectionMetricsServiceCollectionExte /// 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 + /// A will NOT be created automatically /// using this method. To begin collecting metrics 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 ConfigureOpenTelemetryMeterProvider( this IServiceCollection services, - Action configure) + Action configure) { - RegisterBuildAction(services, configure); + Guard.ThrowIfNull(services); + Guard.ThrowIfNull(configure); + + configure(new MeterProviderServiceCollectionBuilder(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 metrics 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 ConfigureOpenTelemetryMeterProvider( + this IServiceCollection services, + Action configure) { Guard.ThrowIfNull(services); Guard.ThrowIfNull(configure); services.AddSingleton( new ConfigureMeterProviderBuilderCallbackWrapper(configure)); + + return services; } private sealed class ConfigureMeterProviderBuilderCallbackWrapper : IConfigureMeterProviderBuilder diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs index f20e9f0d253..7d46cb52130 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs @@ -77,7 +77,10 @@ public static IServiceCollection ConfigureOpenTelemetryTracerProvider( /// once the has been created. Many helper extensions /// register services and may throw if invoked inside the configuration - /// delegate. + /// delegate. If you don't need access to the + /// call instead which is safe to be used with + /// helper extensions. /// /// /// . diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs index d77953fc4b1..4994eae7286 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs @@ -28,7 +28,7 @@ namespace OpenTelemetry.Metrics; public class MeterProviderBuilderBase : MeterProviderBuilder, IMeterProviderBuilder { private readonly bool allowBuild; - private IServiceCollection? services; + private readonly MeterProviderServiceCollectionBuilder innerBuilder; public MeterProviderBuilderBase() { @@ -40,9 +40,7 @@ public MeterProviderBuilderBase() .TryAddSingleton( sp => throw new NotSupportedException("Self-contained MeterProvider cannot be accessed using the application IServiceProvider call Build instead.")); - services.ConfigureOpenTelemetryMeterProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new MeterProviderServiceCollectionBuilder(services); this.allowBuild = true; } @@ -55,9 +53,7 @@ internal MeterProviderBuilderBase(IServiceCollection services) .AddOpenTelemetryMeterProviderBuilderServices() .TryAddSingleton(sp => new MeterProviderSdk(sp, ownsServiceProvider: false)); - services.ConfigureOpenTelemetryMeterProvider((sp, builder) => this.services = null); - - this.services = services; + this.innerBuilder = new MeterProviderServiceCollectionBuilder(services); this.allowBuild = false; } @@ -68,12 +64,7 @@ internal MeterProviderBuilderBase(IServiceCollection services) /// public override MeterProviderBuilder AddInstrumentation(Func instrumentationFactory) { - Guard.ThrowIfNull(instrumentationFactory); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddInstrumentation(instrumentationFactory); - }); + this.innerBuilder.AddInstrumentation(instrumentationFactory); return this; } @@ -81,23 +72,26 @@ public override MeterProviderBuilder AddInstrumentation(Func public override MeterProviderBuilder AddMeter(params string[] names) { - Guard.ThrowIfNull(names); - - this.ConfigureBuilderInternal((sp, builder) => - { - builder.AddMeter(names); - }); + this.innerBuilder.AddMeter(names); return this; } /// MeterProviderBuilder IMeterProviderBuilder.ConfigureServices(Action configure) - => this.ConfigureServicesInternal(configure); + { + this.innerBuilder.ConfigureServices(configure); + + return this; + } /// MeterProviderBuilder IDeferredMeterProviderBuilder.Configure(Action configure) - => this.ConfigureBuilderInternal(configure); + { + this.innerBuilder.ConfigureBuilder(configure); + + return this; + } internal MeterProvider InvokeBuild() => this.Build(); @@ -113,14 +107,10 @@ protected MeterProvider Build() throw new NotSupportedException("A MeterProviderBuilder bound to external service cannot be built directly. Access the MeterProvider using the application IServiceProvider instead."); } - var services = this.services; + var services = this.innerBuilder.Services + ?? throw new NotSupportedException("MeterProviderBuilder build method cannot be called multiple times."); - if (services == null) - { - throw new NotSupportedException("MeterProviderBuilder build method cannot be called multiple times."); - } - - this.services = null; + this.innerBuilder.Services = null; #if DEBUG bool validateScopes = true; @@ -131,34 +121,4 @@ protected MeterProvider Build() return new MeterProviderSdk(serviceProvider, ownsServiceProvider: true); } - - private MeterProviderBuilder ConfigureBuilderInternal(Action configure) - { - var services = this.services; - - if (services == null) - { - throw new NotSupportedException("Builder cannot be configured during MeterProvider construction."); - } - - services.ConfigureOpenTelemetryMeterProvider(configure); - - return this; - } - - private MeterProviderBuilder ConfigureServicesInternal(Action configure) - { - Guard.ThrowIfNull(configure); - - var services = this.services; - - if (services == null) - { - throw new NotSupportedException("Services cannot be configured during MeterProvider construction."); - } - - configure(services); - - return this; - } } diff --git a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs index 9a96356b951..82f141935f8 100644 --- a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs @@ -65,26 +65,33 @@ public void ConfigureOpenTelemetryTracerProvider(int numberOfCalls) [InlineData(3)] public void ConfigureOpenTelemetryMeterProvider(int numberOfCalls) { - var invocations = 0; + var beforeServiceProviderInvocations = 0; + var afterServiceProviderInvocations = 0; var services = new ServiceCollection(); for (int i = 0; i < numberOfCalls; i++) { - services.ConfigureOpenTelemetryMeterProvider((sp, builder) => invocations++); + services.ConfigureOpenTelemetryMeterProvider(builder => beforeServiceProviderInvocations++); + services.ConfigureOpenTelemetryMeterProvider((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] @@ -114,8 +121,4 @@ 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/Metrics/MeterProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs index 03cea950239..6e52ed000ef 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs @@ -221,6 +221,7 @@ public void MeterProviderNestedResolutionUsingBuilderTest(bool callNestedConfigu { bool innerConfigureBuilderTestExecuted = false; bool innerConfigureOpenTelemetryLoggerProviderTestExecuted = false; + bool innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = false; using var provider = Sdk.CreateMeterProviderBuilder() .ConfigureServices(services => @@ -228,7 +229,17 @@ public void MeterProviderNestedResolutionUsingBuilderTest(bool callNestedConfigu if (callNestedConfigure) { services.ConfigureOpenTelemetryMeterProvider( - (sp, builder) => innerConfigureOpenTelemetryLoggerProviderTestExecuted = true); + builder => + { + innerConfigureOpenTelemetryLoggerProviderTestExecuted = true; + builder.AddInstrumentation(); + }); + services.ConfigureOpenTelemetryMeterProvider( + (sp, builder) => + { + innerConfigureOpenTelemetryLoggerProviderTestWithServiceProviderExecuted = true; + Assert.Throws(() => builder.AddInstrumentation()); + }); } }) .ConfigureBuilder((sp, builder) => @@ -236,10 +247,22 @@ public void MeterProviderNestedResolutionUsingBuilderTest(bool callNestedConfigu innerConfigureBuilderTestExecuted = true; Assert.Throws(() => sp.GetService()); }) - .Build(); + .Build() as MeterProviderSdk; + + 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/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs new file mode 100644 index 00000000000..9a980e12ea4 --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs @@ -0,0 +1,48 @@ +// +// 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 Xunit; + +namespace OpenTelemetry.Metrics.Tests; + +public class MeterProviderSdkTest +{ + [Fact] + public void BuilderTypeDoesNotChangeTest() + { + var originalBuilder = Sdk.CreateMeterProviderBuilder(); + var currentBuilder = originalBuilder; + + var deferredBuilder = currentBuilder as IDeferredMeterProviderBuilder; + 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)); + + currentBuilder = currentBuilder.AddMeter("MySource"); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + using var provider = currentBuilder.Build(); + + Assert.NotNull(provider); + } +}