Removing Propagator from Options (#1448)

* Removing Propagator from Options

updating changelog

fixing tests

fixing tests

* updating changelog

* moving to dispose

* Revert "moving to dispose"

This reverts commit 01d4140.

* fixing build

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

examples/AspNet/Global.asax.cs

@@ -36,10 +36,8 @@ public class WebApiApplication : HttpApplication
         protected void Application_Start()
         {
             var builder = Sdk.CreateTracerProviderBuilder()
-                 .AddAspNetInstrumentation(options => options.Propagator = new B3Propagator())
-                 .AddHttpClientInstrumentation(
-                        httpClientOptions => httpClientOptions.Propagator = new B3Propagator(),
-                        httpWebRequestOptions => httpWebRequestOptions.Propagator = new B3Propagator());
+                 .AddAspNetInstrumentation()
+                 .AddHttpClientInstrumentation();
 
             switch (ConfigurationManager.AppSettings["UseExporter"].ToLowerInvariant())
             {

src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs

@@ -17,7 +17,6 @@
 using System;
 using System.Diagnostics;
 using System.Web;
-using OpenTelemetry.Context.Propagation;
 
 namespace OpenTelemetry.Instrumentation.AspNet
 {
@@ -26,12 +25,6 @@ namespace OpenTelemetry.Instrumentation.AspNet
     /// </summary>
     public class AspNetInstrumentationOptions
     {
-        /// <summary>
-        /// Gets or sets <see cref="TextMapPropagator"/> for context propagation.
-        /// By default, <see cref="Propagators.DefaultTextMapPropagator" /> will be used.
-        /// </summary>
-        public TextMapPropagator Propagator { get; set; }
-
         /// <summary>
         /// Gets or sets a Filter function to filter instrumentation for requests on a per request basis.
         /// The Filter gets the HttpContext, and should return a boolean.

src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md

@@ -6,8 +6,11 @@
   to CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator
   and changed from interface to abstract class.
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
-* Propagators.DefaultTextMapPropagator will be used as the default Propagator
+* Propagators.DefaultTextMapPropagator will be used as the default Propagator.
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
+* Removed Propagator from Instrumentation Options. Instrumentation now always
+  respect the Propagator.DefaultTextMapPropagator.
+  ([#1448](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1448))
 
 ## 0.7.0-beta.1
 

src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs

@@ -69,7 +69,7 @@ public override void OnStartActivity(Activity activity, object payload)
 
             var request = context.Request;
             var requestValues = request.Unvalidated;
-            var textMapPropagator = this.options.Propagator ?? Propagators.DefaultTextMapPropagator;
+            var textMapPropagator = Propagators.DefaultTextMapPropagator;
 
             if (!(textMapPropagator is TraceContextPropagator))
             {
@@ -140,7 +140,7 @@ public override void OnStopActivity(Activity activity, object payload)
             Activity activityToEnrich = activity;
             Activity createdActivity = null;
 
-            var textMapPropagator = this.options.Propagator ?? Propagators.DefaultTextMapPropagator;
+            var textMapPropagator = Propagators.DefaultTextMapPropagator;
             bool isCustomPropagator = !(textMapPropagator is TraceContextPropagator);
 
             if (isCustomPropagator)

src/OpenTelemetry.Instrumentation.AspNet/README.md

@@ -71,12 +71,7 @@ public class WebApiApplication : HttpApplication
 ## Advanced configuration
 
 This instrumentation can be configured to change the default behavior by using
-`AspNetInstrumentationOptions`, which allows configuring `Propagator` and
-`Filter` as explained below.
-
-### Propagator
-
-TODO
+`AspNetInstrumentationOptions`, which allows configuring `Filter` as explained below.
 
 ### Filter
 

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs

@@ -17,7 +17,6 @@
 using System;
 using System.Diagnostics;
 using Microsoft.AspNetCore.Http;
-using OpenTelemetry.Context.Propagation;
 
 namespace OpenTelemetry.Instrumentation.AspNetCore
 {
@@ -26,12 +25,6 @@ namespace OpenTelemetry.Instrumentation.AspNetCore
     /// </summary>
     public class AspNetCoreInstrumentationOptions
     {
-        /// <summary>
-        /// Gets or sets <see cref="TextMapPropagator"/> for context propagation.
-        /// By default, <see cref="Propagators.DefaultTextMapPropagator" /> will be used.
-        /// </summary>
-        public TextMapPropagator Propagator { get; set; } = Propagators.DefaultTextMapPropagator;
-
         /// <summary>
         /// Gets or sets a Filter function to filter instrumentation for requests on a per request basis.
         /// The Filter gets the HttpContext, and should return a boolean.

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -16,6 +16,9 @@
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
 * Propagators.DefaultTextMapPropagator will be used as the default Propagator
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
+* Removed Propagator from Instrumentation Options. Instrumentation now always
+  respect the Propagator.DefaultTextMapPropagator.
+  ([#1448](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1448))
 
 ## 0.7.0-beta.1
 

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs

@@ -78,9 +78,10 @@ public override void OnStartActivity(Activity activity, object payload)
             }
 
             var request = context.Request;
-            if (!this.hostingSupportsW3C || !(this.options.Propagator is TraceContextPropagator))
+            var textMapPropagator = Propagators.DefaultTextMapPropagator;
+            if (!this.hostingSupportsW3C || !(textMapPropagator is TraceContextPropagator))
             {
-                var ctx = this.options.Propagator.Extract(default, request, HttpRequestHeaderValuesGetter);
+                var ctx = textMapPropagator.Extract(default, request, HttpRequestHeaderValuesGetter);
 
                 if (ctx.ActivityContext.IsValid()
                     && ctx.ActivityContext != new ActivityContext(activity.TraceId, activity.ParentSpanId, activity.ActivityTraceFlags, activity.TraceStateString, true))

src/OpenTelemetry.Instrumentation.AspNetCore/README.md

@@ -54,14 +54,9 @@ public void ConfigureServices(IServiceCollection services)
 ## Advanced configuration
 
 This instrumentation can be configured to change the default behavior by using
-`AspNetCoreInstrumentationOptions`, which allows configuring
-[`Propagator`](#propagator) and adding [`Filter`](#filter),
+`AspNetCoreInstrumentationOptions`, which allows adding [`Filter`](#filter),
 [`Enrich`](#enrich) as explained below.
 
-### Propagator
-
-TODO
-
 ### Filter
 
 This instrumentation by default collects all the incoming http requests. It

src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md

@@ -12,6 +12,9 @@
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
 * Propagators.DefaultTextMapPropagator will be used as the default Propagator
   ([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
+* Removed Propagator from Instrumentation Options. Instrumentation now always
+  respect the Propagator.DefaultTextMapPropagator.
+  ([#1448](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1448))
 
 ## 0.7.0-beta.1
 

src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs

@@ -18,7 +18,6 @@
 using System.Diagnostics;
 using System.Net.Http;
 using System.Runtime.CompilerServices;
-using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Http.Implementation;
 using OpenTelemetry.Trace;
 
@@ -34,12 +33,6 @@ public class HttpClientInstrumentationOptions
         /// </summary>
         public bool SetHttpFlavor { get; set; }
 
-        /// <summary>
-        /// Gets or sets <see cref="TextMapPropagator"/> for context propagation.
-        /// By default, <see cref="Propagators.DefaultTextMapPropagator" /> will be used.
-        /// </summary>
-        public TextMapPropagator Propagator { get; set; } = Propagators.DefaultTextMapPropagator;
-
         /// <summary>
         /// Gets or sets a Filter function to filter instrumentation for requests on a per request basis.
         /// The Filter gets the HttpRequestMessage, and should return a boolean.

src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs

@@ -18,7 +18,6 @@
 using System;
 using System.Diagnostics;
 using System.Net;
-using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Http.Implementation;
 using OpenTelemetry.Trace;
 
@@ -34,15 +33,6 @@ public class HttpWebRequestInstrumentationOptions
         /// </summary>
         public bool SetHttpFlavor { get; set; }
 
-        /// <summary>
-        /// Gets or sets <see cref="TextMapPropagator"/> for context propagation. Default value: <see cref="CompositeTextMapPropagator"/> with <see cref="TraceContextPropagator"/> &amp; <see cref="BaggagePropagator"/>.
-        /// </summary>
-        public TextMapPropagator Propagator { get; set; } = new CompositeTextMapPropagator(new TextMapPropagator[]
-        {
-            new TraceContextPropagator(),
-            new BaggagePropagator(),
-        });
-
         /// <summary>
         /// Gets or sets a Filter function to filter instrumentation for requests on a per request basis.
         /// The Filter gets the HttpWebRequest, and should return a boolean.

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs

@@ -83,7 +83,7 @@ public override void OnStartActivity(Activity activity, object payload)
                 return;
             }
 
-            if (this.options.Propagator.Extract(default, request, HttpRequestMessageHeaderValuesGetter) != default)
+            if (Propagators.DefaultTextMapPropagator.Extract(default, request, HttpRequestMessageHeaderValuesGetter) != default)
             {
                 // this request is already instrumented, we should back off
                 activity.IsAllDataRequested = false;
@@ -115,9 +115,11 @@ public override void OnStartActivity(Activity activity, object payload)
                 }
             }
 
-            if (!(this.httpClientSupportsW3C && this.options.Propagator is TraceContextPropagator))
+            var textMapPropagator = Propagators.DefaultTextMapPropagator;
+
+            if (!(this.httpClientSupportsW3C && textMapPropagator is TraceContextPropagator))
             {
-                this.options.Propagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpRequestMessageHeaderValueSetter);
+                textMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpRequestMessageHeaderValueSetter);
             }
         }
 

src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs

@@ -196,11 +196,11 @@ private static void AddExceptionTags(Exception exception, Activity activity)
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static void InstrumentRequest(HttpWebRequest request, ActivityContext activityContext)
-            => Options.Propagator.Inject(new PropagationContext(activityContext, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
+            => Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activityContext, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static bool IsRequestInstrumented(HttpWebRequest request)
-            => Options.Propagator.Extract(default, request, HttpWebRequestHeaderValuesGetter) != default;
+            => Propagators.DefaultTextMapPropagator.Extract(default, request, HttpWebRequestHeaderValuesGetter) != default;
 
         private static void ProcessRequest(HttpWebRequest request)
         {

test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs

@@ -144,6 +144,7 @@ public void AspNetRequestsAreCollectedSuccessfully(
             }
 
             var activityProcessor = new Mock<BaseProcessor<Activity>>();
+            Sdk.SetDefaultTextMapPropagator(propagator.Object);
             using (openTelemetry = Sdk.CreateTracerProviderBuilder()
                 .AddAspNetInstrumentation(
                 (options) =>
@@ -163,11 +164,6 @@ public void AspNetRequestsAreCollectedSuccessfully(
                         return httpContext.Request.Path != filter;
                     };
 
-                    if (!carrierFormat.Equals("TraceContext"))
-                    {
-                        options.Propagator = propagator.Object;
-                    }
-
                     options.Enrich = ActivityEnrichment;
                 })
             .SetResource(expectedResource)

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs

@@ -58,21 +58,6 @@ public void AddAspNetCoreInstrumentation_BadArgs()
             Assert.Throws<ArgumentNullException>(() => builder.AddAspNetCoreInstrumentation());
         }
 
-        [Fact]
-        public void DefaultPropagatorIsFromPropagators()
-        {
-            var options = new AspNetCoreInstrumentationOptions();
-            Assert.Same(Propagators.DefaultTextMapPropagator, options.Propagator);
-        }
-
-        [Fact]
-        public void PropagatorSetDoesNotAffectGlobalPropagators()
-        {
-            var options = new AspNetCoreInstrumentationOptions();
-            options.Propagator = new TraceContextPropagator();
-            Assert.NotSame(Propagators.DefaultTextMapPropagator, options.Propagator);
-        }
-
         [Fact]
         public async Task StatusIsUnsetOn200Response()
         {
@@ -225,8 +210,9 @@ public async Task CustomPropagator()
                 .WithWebHostBuilder(builder =>
                     builder.ConfigureTestServices(services =>
                     {
+                        Sdk.SetDefaultTextMapPropagator(propagator.Object);
                         this.openTelemetrySdk = Sdk.CreateTracerProviderBuilder()
-                            .AddAspNetCoreInstrumentation((opt) => opt.Propagator = propagator.Object)
+                            .AddAspNetCoreInstrumentation()
                             .AddProcessor(activityProcessor.Object)
                             .Build();
                     })))
@@ -250,6 +236,11 @@ public async Task CustomPropagator()
 
             Assert.Equal(expectedTraceId, activity.Context.TraceId);
             Assert.Equal(expectedSpanId, activity.ParentSpanId);
+            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
+            {
+                new TraceContextPropagator(),
+                new BaggagePropagator(),
+            }));
         }
 
         [Fact]

test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs

@@ -74,9 +74,6 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync(bool shouldEnrich
             parent.TraceStateString = "k1=v1,k2=v2";
             parent.ActivityTraceFlags = ActivityTraceFlags.Recorded;
 
-            // Ensure that the header value func does not throw if the header key can't be found
-            var mockPropagator = new Mock<TextMapPropagator>();
-
             // var isInjectedHeaderValueGetterThrows = false;
             // mockTextFormat
             //     .Setup(x => x.IsInjected(It.IsAny<HttpRequestMessage>(), It.IsAny<Func<HttpRequestMessage, string, IEnumerable<string>>>()))
@@ -97,7 +94,6 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync(bool shouldEnrich
             using (Sdk.CreateTracerProviderBuilder()
                         .AddHttpClientInstrumentation(o =>
                         {
-                            o.Propagator = mockPropagator.Object;
                             if (shouldEnrich)
                             {
                                 o.Enrich = ActivityEnrichment;
@@ -155,10 +151,11 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync_CustomFormat(bool
             parent.TraceStateString = "k1=v1,k2=v2";
             parent.ActivityTraceFlags = ActivityTraceFlags.Recorded;
 
+            Sdk.SetDefaultTextMapPropagator(propagator.Object);
+
             using (Sdk.CreateTracerProviderBuilder()
                    .AddHttpClientInstrumentation((opt) =>
                    {
-                       opt.Propagator = propagator.Object;
                        if (shouldEnrich)
                        {
                            opt.Enrich = ActivityEnrichment;
@@ -187,6 +184,11 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync_CustomFormat(bool
 
             Assert.Equal($"00/{activity.Context.TraceId}/{activity.Context.SpanId}/01", traceparents.Single());
             Assert.Equal("k1=v1,k2=v2", tracestates.Single());
+            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
+            {
+                new TraceContextPropagator(),
+                new BaggagePropagator(),
+            }));
         }
 
         [Fact]

test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs

@@ -107,10 +107,10 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsyncWhenActivityIs
                     contentFromPropagator = context.ActivityContext;
                 });
 
-            // Sdk.SetDefaultTextMapPropagator(propagator.Object);
+            Sdk.SetDefaultTextMapPropagator(propagator.Object);
             using var shutdownSignal = Sdk.CreateTracerProviderBuilder()
                 .AddProcessor(activityProcessor.Object)
-                .AddHttpWebRequestInstrumentation(options => options.Propagator = propagator.Object)
+                .AddHttpWebRequestInstrumentation()
                 .Build();
 
             var request = (HttpWebRequest)WebRequest.Create(this.url);
@@ -135,6 +135,11 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsyncWhenActivityIs
             Assert.NotEqual(default, contentFromPropagator.SpanId);
 
             parent.Stop();
+            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
+            {
+                new TraceContextPropagator(),
+                new BaggagePropagator(),
+            }));
         }
 
         [Fact]
@@ -149,9 +154,10 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsync_CustomFormat(
                 });
 
             var activityProcessor = new Mock<BaseProcessor<Activity>>();
+            Sdk.SetDefaultTextMapPropagator(propagator.Object);
             using var shutdownSignal = Sdk.CreateTracerProviderBuilder()
                 .AddProcessor(activityProcessor.Object)
-                .AddHttpWebRequestInstrumentation(options => options.Propagator = propagator.Object)
+                .AddHttpWebRequestInstrumentation()
                 .Build();
 
             var request = (HttpWebRequest)WebRequest.Create(this.url);
@@ -182,6 +188,11 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsync_CustomFormat(
             Assert.Equal("k1=v1,k2=v2", tracestate);
 
             parent.Stop();
+            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
+            {
+                new TraceContextPropagator(),
+                new BaggagePropagator(),
+            }));
         }
 
         [Fact]