Revert "Removing Propagator from Options (#1448)"

This reverts commit 7c9bee3.

examples/AspNet/Global.asax.cs

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

src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs

@@ -17,6 +17,7 @@
 using System;
 using System.Diagnostics;
 using System.Web;
+using OpenTelemetry.Context.Propagation;
 
 namespace OpenTelemetry.Instrumentation.AspNet
 {
@@ -25,6 +26,12 @@ 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,11 +6,8 @@
   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 = Propagators.DefaultTextMapPropagator;
+            var textMapPropagator = this.options.Propagator ?? 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 = Propagators.DefaultTextMapPropagator;
+            var textMapPropagator = this.options.Propagator ?? Propagators.DefaultTextMapPropagator;
             bool isCustomPropagator = !(textMapPropagator is TraceContextPropagator);
 
             if (isCustomPropagator)

src/OpenTelemetry.Instrumentation.AspNet/README.md

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

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs

@@ -17,6 +17,7 @@
 using System;
 using System.Diagnostics;
 using Microsoft.AspNetCore.Http;
+using OpenTelemetry.Context.Propagation;
 
 namespace OpenTelemetry.Instrumentation.AspNetCore
 {
@@ -25,6 +26,12 @@ 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,9 +16,6 @@
   ([#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,10 +78,9 @@ public override void OnStartActivity(Activity activity, object payload)
             }
 
             var request = context.Request;
-            var textMapPropagator = Propagators.DefaultTextMapPropagator;
-            if (!this.hostingSupportsW3C || !(textMapPropagator is TraceContextPropagator))
+            if (!this.hostingSupportsW3C || !(this.options.Propagator is TraceContextPropagator))
             {
-                var ctx = textMapPropagator.Extract(default, request, HttpRequestHeaderValuesGetter);
+                var ctx = this.options.Propagator.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,9 +54,14 @@ public void ConfigureServices(IServiceCollection services)
 ## Advanced configuration
 
 This instrumentation can be configured to change the default behavior by using
-`AspNetCoreInstrumentationOptions`, which allows adding [`Filter`](#filter),
+`AspNetCoreInstrumentationOptions`, which allows configuring
+[`Propagator`](#propagator) and 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,9 +12,6 @@
   ([#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,6 +18,7 @@
 using System.Diagnostics;
 using System.Net.Http;
 using System.Runtime.CompilerServices;
+using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Http.Implementation;
 using OpenTelemetry.Trace;
 
@@ -33,6 +34,12 @@ 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,6 +18,7 @@
 using System;
 using System.Diagnostics;
 using System.Net;
+using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Http.Implementation;
 using OpenTelemetry.Trace;
 
@@ -33,6 +34,15 @@ 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 (Propagators.DefaultTextMapPropagator.Extract(default, request, HttpRequestMessageHeaderValuesGetter) != default)
+            if (this.options.Propagator.Extract(default, request, HttpRequestMessageHeaderValuesGetter) != default)
             {
                 // this request is already instrumented, we should back off
                 activity.IsAllDataRequested = false;
@@ -115,11 +115,9 @@ public override void OnStartActivity(Activity activity, object payload)
                 }
             }
 
-            var textMapPropagator = Propagators.DefaultTextMapPropagator;
-
-            if (!(this.httpClientSupportsW3C && textMapPropagator is TraceContextPropagator))
+            if (!(this.httpClientSupportsW3C && this.options.Propagator is TraceContextPropagator))
             {
-                textMapPropagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpRequestMessageHeaderValueSetter);
+                this.options.Propagator.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)
-            => Propagators.DefaultTextMapPropagator.Inject(new PropagationContext(activityContext, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
+            => Options.Propagator.Inject(new PropagationContext(activityContext, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static bool IsRequestInstrumented(HttpWebRequest request)
-            => Propagators.DefaultTextMapPropagator.Extract(default, request, HttpWebRequestHeaderValuesGetter) != default;
+            => Options.Propagator.Extract(default, request, HttpWebRequestHeaderValuesGetter) != default;
 
         private static void ProcessRequest(HttpWebRequest request)
         {

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

@@ -144,7 +144,6 @@ public void AspNetRequestsAreCollectedSuccessfully(
             }
 
             var activityProcessor = new Mock<BaseProcessor<Activity>>();
-            Sdk.SetDefaultTextMapPropagator(propagator.Object);
             using (openTelemetry = Sdk.CreateTracerProviderBuilder()
                 .AddAspNetInstrumentation(
                 (options) =>
@@ -164,6 +163,11 @@ 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,6 +58,21 @@ 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()
         {
@@ -210,9 +225,8 @@ public async Task CustomPropagator()
                 .WithWebHostBuilder(builder =>
                     builder.ConfigureTestServices(services =>
                     {
-                        Sdk.SetDefaultTextMapPropagator(propagator.Object);
                         this.openTelemetrySdk = Sdk.CreateTracerProviderBuilder()
-                            .AddAspNetCoreInstrumentation()
+                            .AddAspNetCoreInstrumentation((opt) => opt.Propagator = propagator.Object)
                             .AddProcessor(activityProcessor.Object)
                             .Build();
                     })))
@@ -236,11 +250,6 @@ 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,6 +74,9 @@ 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>>>()))
@@ -94,6 +97,7 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync(bool shouldEnrich
             using (Sdk.CreateTracerProviderBuilder()
                         .AddHttpClientInstrumentation(o =>
                         {
+                            o.Propagator = mockPropagator.Object;
                             if (shouldEnrich)
                             {
                                 o.Enrich = ActivityEnrichment;
@@ -151,11 +155,10 @@ 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;
@@ -184,11 +187,6 @@ 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()
+                .AddHttpWebRequestInstrumentation(options => options.Propagator = propagator.Object)
                 .Build();
 
             var request = (HttpWebRequest)WebRequest.Create(this.url);
@@ -135,11 +135,6 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsyncWhenActivityIs
             Assert.NotEqual(default, contentFromPropagator.SpanId);
 
             parent.Stop();
-            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
-            {
-                new TraceContextPropagator(),
-                new BaggagePropagator(),
-            }));
         }
 
         [Fact]
@@ -154,10 +149,9 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsync_CustomFormat(
                 });
 
             var activityProcessor = new Mock<BaseProcessor<Activity>>();
-            Sdk.SetDefaultTextMapPropagator(propagator.Object);
             using var shutdownSignal = Sdk.CreateTracerProviderBuilder()
                 .AddProcessor(activityProcessor.Object)
-                .AddHttpWebRequestInstrumentation()
+                .AddHttpWebRequestInstrumentation(options => options.Propagator = propagator.Object)
                 .Build();
 
             var request = (HttpWebRequest)WebRequest.Create(this.url);
@@ -188,11 +182,6 @@ 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]