From 207f687b10dbc28ea94a26d89ebed572faa35388 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 5 May 2023 13:23:10 -0700 Subject: [PATCH 1/3] Fix the TracerProviderBuilder.AddInstrumentation factory pattern extension. --- .../CHANGELOG.md | 3 ++ ...njectionTracerProviderBuilderExtensions.cs | 2 +- .../MeterProviderBuilderExtensionsTests.cs | 34 +++++++++++++++++++ .../TracerProviderBuilderExtensionsTest.cs | 34 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index fdf49c7211f..3678b9a14e1 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Fixed the `TracerProviderBuilder.AddInstrumentation` factory extension. + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ## 1.5.0-alpha.2 Released 2023-Mar-31 diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs index 6a72dc7c1fa..3642e9960a0 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs @@ -106,7 +106,7 @@ public static TracerProviderBuilder AddInstrumentation( tracerProviderBuilder.ConfigureBuilder((sp, builder) => { - if (tracerProviderBuilder is ITracerProviderBuilder iTracerProviderBuilder + if (builder is ITracerProviderBuilder iTracerProviderBuilder && iTracerProviderBuilder.Provider != null) { builder.AddInstrumentation(() => instrumentationFactory(sp, iTracerProviderBuilder.Provider)); diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs index e3e3857dfd6..053310ea2ed 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs @@ -82,6 +82,39 @@ public void AddReaderUsingDependencyInjectionTest() Assert.True(reader.Head.Next?.Value is MyReader); } + [Fact] + public void AddInstrumentationTest() + { + List instrumentation = null; + + using (var provider = Sdk.CreateMeterProviderBuilder() + .AddInstrumentation() + .AddInstrumentation((sp, provider) => new MyInstrumentation() { Provider = provider }) + .AddInstrumentation(new MyInstrumentation()) + .Build() as MeterProviderSdk) + { + Assert.NotNull(provider); + + Assert.Equal(3, provider.Instrumentations.Count); + + Assert.Null(((MyInstrumentation)provider.Instrumentations[0]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[0]).Disposed); + + Assert.NotNull(((MyInstrumentation)provider.Instrumentations[1]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[1]).Disposed); + + Assert.Null(((MyInstrumentation)provider.Instrumentations[2]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[2]).Disposed); + + instrumentation = new List(provider.Instrumentations); + } + + Assert.NotNull(instrumentation); + Assert.True(((MyInstrumentation)instrumentation[0]).Disposed); + Assert.True(((MyInstrumentation)instrumentation[1]).Disposed); + Assert.True(((MyInstrumentation)instrumentation[2]).Disposed); + } + [Fact] public void SetAndConfigureResourceTest() { @@ -303,6 +336,7 @@ private static void RunBuilderServiceLifecycleTest( private sealed class MyInstrumentation : IDisposable { + internal MeterProvider Provider; internal bool Disposed; public void Dispose() diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs index 4e164ea7dae..cc678067a3c 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs @@ -172,6 +172,39 @@ public void AddProcessorUsingDependencyInjectionTest() Assert.True(processor.Head.Next?.Value is MyProcessor); } + [Fact] + public void AddInstrumentationTest() + { + List instrumentation = null; + + using (var provider = Sdk.CreateTracerProviderBuilder() + .AddInstrumentation() + .AddInstrumentation((sp, provider) => new MyInstrumentation() { Provider = provider }) + .AddInstrumentation(new MyInstrumentation()) + .Build() as TracerProviderSdk) + { + Assert.NotNull(provider); + + Assert.Equal(3, provider.Instrumentations.Count); + + Assert.Null(((MyInstrumentation)provider.Instrumentations[0]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[0]).Disposed); + + Assert.NotNull(((MyInstrumentation)provider.Instrumentations[1]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[1]).Disposed); + + Assert.Null(((MyInstrumentation)provider.Instrumentations[2]).Provider); + Assert.False(((MyInstrumentation)provider.Instrumentations[2]).Disposed); + + instrumentation = new List(provider.Instrumentations); + } + + Assert.NotNull(instrumentation); + Assert.True(((MyInstrumentation)instrumentation[0]).Disposed); + Assert.True(((MyInstrumentation)instrumentation[1]).Disposed); + Assert.True(((MyInstrumentation)instrumentation[2]).Disposed); + } + [Fact] public void SetAndConfigureResourceTest() { @@ -431,6 +464,7 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame private sealed class MyInstrumentation : IDisposable { + internal TracerProvider Provider; internal bool Disposed; public void Dispose() From 3576ca7ccd56733a9c7f6b987499dcaeca1033bf Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 5 May 2023 13:31:03 -0700 Subject: [PATCH 2/3] 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 3678b9a14e1..bfb05627be8 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * Fixed the `TracerProviderBuilder.AddInstrumentation` factory extension. - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#4468](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4468)) ## 1.5.0-alpha.2 From 7854f38fc5f537d8078fb9bc4c8a3ef358939749 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 8 May 2023 09:55:55 -0700 Subject: [PATCH 3/3] CHANGELOG tweak. --- src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index bfb05627be8..ac8b87f0d01 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -2,7 +2,10 @@ ## Unreleased -* Fixed the `TracerProviderBuilder.AddInstrumentation` factory extension. +* Fixed a bug which prevented the + `TracerProviderBuilder.AddInstrumentation(IServiceProvider, TracerProvider)` + factory extension from being called during construction of the SDK + `TracerProvider`. ([#4468](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4468)) ## 1.5.0-alpha.2