From 4376564d8e66d646a341fa15bfa82fdef99b6654 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 8 May 2023 10:18:05 -0700 Subject: [PATCH] [di-tracing] Fix the TracerProviderBuilder.AddInstrumentation factory pattern extension (#4468) --- .../CHANGELOG.md | 6 ++++ ...njectionTracerProviderBuilderExtensions.cs | 2 +- .../MeterProviderBuilderExtensionsTests.cs | 34 +++++++++++++++++++ .../TracerProviderBuilderExtensionsTest.cs | 34 +++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md index fdf49c7211f..ac8b87f0d01 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* 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 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()