From f39a7b197ba0fc93e3b7be62ce84cb0a91cebd7a Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 10:33:16 -0700 Subject: [PATCH 1/6] Expose a detached MeterProviderBuilder extension on IServiceCollection which may modify services. --- .../MeterProviderServiceCollectionBuilder.cs | 94 +++++++++++++++++++ ...ctionMetricsServiceCollectionExtensions.cs | 50 ++++++++-- .../Builder/MeterProviderBuilderBase.cs | 62 ++---------- .../ServiceCollectionExtensionsTests.cs | 19 +++- .../MeterProviderBuilderExtensionsTests.cs | 27 +++++- 5 files changed, 184 insertions(+), 68 deletions(-) create mode 100644 src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/MeterProviderServiceCollectionBuilder.cs new file mode 100644 index 0000000000..a2ddc2d59a --- /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 77955dc9ef..fc1e10b4e0 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,68 @@ 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. + /// + /// + /// . + /// 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/Metrics/Builder/MeterProviderBuilderBase.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs index d77953fc4b..b9e0502e40 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,18 @@ 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); /// MeterProviderBuilder IDeferredMeterProviderBuilder.Configure(Action configure) - => this.ConfigureBuilderInternal(configure); + => this.innerBuilder.ConfigureBuilder(configure); internal MeterProvider InvokeBuild() => this.Build(); @@ -113,14 +99,14 @@ 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; 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 +117,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 16fee70d1d..81ac6c3beb 100644 --- a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs @@ -58,26 +58,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] @@ -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/Metrics/MeterProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs index 03cea95023..6e52ed000e 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()); } From 58917f608825393c292fd7a435b61e5c037bf796 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 10:37:17 -0700 Subject: [PATCH 2/6] Ceremony and cleanup. --- .../.publicApi/net462/PublicAPI.Unshipped.txt | 1 + .../.publicApi/netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../CHANGELOG.md | 6 ++++++ .../ServiceCollectionExtensionsTests.cs | 4 ---- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api.ProviderBuilderExtensions/.publicApi/net462/PublicAPI.Unshipped.txt index 6f215dd80e..eb84743d0d 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 6f215dd80e..eb84743d0d 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 7c931eadc8..f90c1f8b92 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -14,6 +14,12 @@ 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). + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ## 1.5.0-alpha.2 Released 2023-Mar-31 diff --git a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs index 09d2103b18..82f141935f 100644 --- a/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Api.ProviderBuilderExtensions.Tests/ServiceCollectionExtensionsTests.cs @@ -121,8 +121,4 @@ public void ConfigureOpenTelemetryLoggerProvider(int numberOfCalls) Assert.Equal(invocations, registrations.Count()); Assert.Equal(numberOfCalls, registrations.Count()); } - - private sealed class CustomType - { - } } From 4b972a450c061ed275f3d28c48ba8ccf0758934d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 10:39:48 -0700 Subject: [PATCH 3/6] XML comment fix. --- ...etryDependencyInjectionMetricsServiceCollectionExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs index fc1e10b4e0..7a1049b2b9 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs @@ -34,7 +34,7 @@ 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. From ad4378c6100ed856b74ee82c81d8f500dafdb914 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 10:41:25 -0700 Subject: [PATCH 4/6] CHANGELOG patch. --- src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index f90c1f8b92..7b20a8091a 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -18,7 +18,7 @@ which may be used to configure `MeterProviderBuilder`s while the `IServiceCollection` is modifiable (before the `IServiceProvider` has been created). - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#4517](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4517)) ## 1.5.0-alpha.2 From a4fa9bcfeefac6e548d8c14ac849d937112b9689 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 25 May 2023 15:03:55 -0700 Subject: [PATCH 5/6] Code review. --- ...yDependencyInjectionMetricsServiceCollectionExtensions.cs | 5 ++++- ...yDependencyInjectionTracingServiceCollectionExtensions.cs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs index 7a1049b2b9..e62776c6ca 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Metrics/OpenTelemetryDependencyInjectionMetricsServiceCollectionExtensions.cs @@ -77,7 +77,10 @@ public static class OpenTelemetryDependencyInjectionMetricsServiceCollectionExte /// 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.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs index f20e9f0d25..7d46cb5213 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracingServiceCollectionExtensions.cs @@ -77,7 +77,10 @@ public static class OpenTelemetryDependencyInjectionTracingServiceCollectionExte /// 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. /// /// /// . From bf43a29a260c2118148e05b7a5b95cb9871457ce Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 31 May 2023 10:35:34 -0700 Subject: [PATCH 6/6] Apply fix for broken chains. --- .../Builder/MeterProviderBuilderBase.cs | 20 ++++---- .../Metrics/MeterProviderSdkTest.cs | 48 +++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs index b9e0502e40..4994eae728 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderBase.cs @@ -79,11 +79,19 @@ public override MeterProviderBuilder AddMeter(params string[] names) /// MeterProviderBuilder IMeterProviderBuilder.ConfigureServices(Action configure) - => this.innerBuilder.ConfigureServices(configure); + { + this.innerBuilder.ConfigureServices(configure); + + return this; + } /// MeterProviderBuilder IDeferredMeterProviderBuilder.Configure(Action configure) - => this.innerBuilder.ConfigureBuilder(configure); + { + this.innerBuilder.ConfigureBuilder(configure); + + return this; + } internal MeterProvider InvokeBuild() => this.Build(); @@ -99,12 +107,8 @@ 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.innerBuilder.Services; - - if (services == null) - { - throw new NotSupportedException("MeterProviderBuilder build method cannot be called multiple times."); - } + var services = this.innerBuilder.Services + ?? throw new NotSupportedException("MeterProviderBuilder build method cannot be called multiple times."); this.innerBuilder.Services = null; diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTest.cs new file mode 100644 index 0000000000..9a980e12ea --- /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); + } +}