From 4e4e8012bbeee033787160c535162c45d57c7817 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 31 May 2023 10:10:05 -0700 Subject: [PATCH 1/3] Fixed TracerProviderBuilderBase changing the builder type. --- .../Builder/TracerProviderBuilderBase.cs | 24 ++++++----- .../Trace/TracerProviderSdkTest.cs | 40 +++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs index 0a4c2d6d0f8..1b2315ef73a 100644 --- a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs +++ b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderBase.cs @@ -90,11 +90,19 @@ public override TracerProviderBuilder AddLegacySource(string operationName) /// TracerProviderBuilder ITracerProviderBuilder.ConfigureServices(Action configure) - => this.innerBuilder.ConfigureServices(configure); + { + this.innerBuilder.ConfigureServices(configure); + + return this; + } /// TracerProviderBuilder IDeferredTracerProviderBuilder.Configure(Action configure) - => this.innerBuilder.ConfigureBuilder(configure); + { + this.innerBuilder.ConfigureBuilder(configure); + + return this; + } internal TracerProvider InvokeBuild() => this.Build(); @@ -115,7 +123,7 @@ protected TracerProviderBuilder AddInstrumentation( Guard.ThrowIfNullOrWhitespace(instrumentationVersion); Guard.ThrowIfNull(instrumentationFactory); - return this.innerBuilder.ConfigureBuilder((sp, builder) => + this.innerBuilder.ConfigureBuilder((sp, builder) => { if (builder is TracerProviderBuilderSdk tracerProviderBuilderState) { @@ -125,6 +133,8 @@ protected TracerProviderBuilder AddInstrumentation( instrumentationFactory); } }); + + return this; } /// @@ -138,12 +148,8 @@ 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.innerBuilder.Services; - - if (services == null) - { - throw new NotSupportedException("TracerProviderBuilder build method cannot be called multiple times."); - } + var services = this.innerBuilder.Services + ?? throw new NotSupportedException("TracerProviderBuilder build method cannot be called multiple times."); this.innerBuilder.Services = null; diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs index abdee03731f..7da31fffcc1 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs @@ -1218,11 +1218,51 @@ public void SdkSamplesAndProcessesLegacySourceWhenAddLegacySourceIsCalledWithWil Assert.Contains(nonLegacyActivity.OperationName, onStopProcessedActivities); // Processor.OnEnd is called since we added a legacy OperationName } + [Fact] + public void BuilderTypeDoesNotChangeTest() + { + var originalBuilder = new TestTracerProviderBuilder(); + + // Tests the protected version of AddInstrumentation on TracerProviderBuilderBase + var currentBuilder = originalBuilder.AddInstrumentation(); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + var deferredBuilder = currentBuilder as IDeferredTracerProviderBuilder; + 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.AddSource("MySource"); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + currentBuilder = currentBuilder.AddLegacySource("MyLegacySource"); + Assert.True(ReferenceEquals(originalBuilder, currentBuilder)); + + using var provider = currentBuilder.Build(); + + Assert.NotNull(provider); + } + public void Dispose() { GC.SuppressFinalize(this); } + private sealed class TestTracerProviderBuilder : TracerProviderBuilderBase + { + public TracerProviderBuilder AddInstrumentation() + { + return this.AddInstrumentation("SomeInstrumentation", "1.0.0", () => new object()); + } + } + private class TestInstrumentation : IDisposable { public bool IsDisposed; From 3d92dc581fbbf1f1b0dd13067e6fb136801c0227 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 31 May 2023 10:12:01 -0700 Subject: [PATCH 2/3] CHANGELOG stub. --- src/OpenTelemetry/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 9f2ad6a2a06..36a055b098d 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Fixed a bug introduced by + [#4508](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4508) in + 1.5.0-rc.1 which caused the "Build" extension to return `null` when performing + chained/fluent calls. + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ## 1.5.0-rc.1 Released 2023-May-25 From f54d5c5f2cf8ef9df7e89bf6f1c5acae98db83e3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 31 May 2023 10:17:24 -0700 Subject: [PATCH 3/3] CHANGELOG patch. --- 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 36a055b098d..a76b47a1c56 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -6,7 +6,7 @@ [#4508](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4508) in 1.5.0-rc.1 which caused the "Build" extension to return `null` when performing chained/fluent calls. - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#4529](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4529)) ## 1.5.0-rc.1