[di-tracing] Fix the TracerProviderBuilder.AddInstrumentation factory pattern extension (#4468)

Author: CodeBlanch

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

src/OpenTelemetry.Api.ProviderBuilderExtensions/Trace/OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions.cs

@@ -106,7 +106,7 @@ public static TracerProviderBuilder AddInstrumentation<T>(
 
         tracerProviderBuilder.ConfigureBuilder((sp, builder) =>
         {
-            if (tracerProviderBuilder is ITracerProviderBuilder iTracerProviderBuilder
+            if (builder is ITracerProviderBuilder iTracerProviderBuilder
                 && iTracerProviderBuilder.Provider != null)
             {
                 builder.AddInstrumentation(() => instrumentationFactory(sp, iTracerProviderBuilder.Provider));

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<object> instrumentation = null;
+
+            using (var provider = Sdk.CreateMeterProviderBuilder()
+                .AddInstrumentation<MyInstrumentation>()
+                .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<object>(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()

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<object> instrumentation = null;
+
+            using (var provider = Sdk.CreateTracerProviderBuilder()
+                .AddInstrumentation<MyInstrumentation>()
+                .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<object>(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()