diff --git a/examples/AspNet/Global.asax.cs b/examples/AspNet/Global.asax.cs index c1fc938f692..2dc5ae10537 100644 --- a/examples/AspNet/Global.asax.cs +++ b/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()) { diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs index c0c97345e72..ea0e9f0616b 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs +++ b/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 /// public class AspNetInstrumentationOptions { - /// - /// Gets or sets for context propagation. - /// By default, will be used. - /// - public TextMapPropagator Propagator { get; set; } - /// /// 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. diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index dd2c46a1853..7fcd29b50b2 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index ac7d3636704..25fc3b97734 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/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) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/README.md b/src/OpenTelemetry.Instrumentation.AspNet/README.md index 53e330a4529..1ad20d0af43 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/README.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs index da3b9a908f0..528bae8674c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs +++ b/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 /// public class AspNetCoreInstrumentationOptions { - /// - /// Gets or sets for context propagation. - /// By default, will be used. - /// - public TextMapPropagator Propagator { get; set; } = Propagators.DefaultTextMapPropagator; - /// /// 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. diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 0de5b6a4869..5d3d4a29ffe 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 9015af938bb..03cde029a96 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/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)) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index 2f6955cdc2d..5e822fcd80d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 0e4d8e4a630..d04bfe3d819 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs index eaf8fd1dcf3..3d836d8bad5 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs +++ b/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 /// public bool SetHttpFlavor { get; set; } - /// - /// Gets or sets for context propagation. - /// By default, will be used. - /// - public TextMapPropagator Propagator { get; set; } = Propagators.DefaultTextMapPropagator; - /// /// 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. diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs index c96a901bedf..c9d3497e89c 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs +++ b/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 /// public bool SetHttpFlavor { get; set; } - /// - /// Gets or sets for context propagation. Default value: with & . - /// - public TextMapPropagator Propagator { get; set; } = new CompositeTextMapPropagator(new TextMapPropagator[] - { - new TraceContextPropagator(), - new BaggagePropagator(), - }); - /// /// 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. diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index ec810f3a65e..9b877fa601a 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/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); } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 7603cf37a26..cef7f855859 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/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) { diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 40326f39fe0..bf4c110a454 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -144,6 +144,7 @@ public void AspNetRequestsAreCollectedSuccessfully( } var activityProcessor = new Mock>(); + 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) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index b343af26f11..77ffb9f2271 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -58,21 +58,6 @@ public void AddAspNetCoreInstrumentation_BadArgs() Assert.Throws(() => 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] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs index de76619afe9..b0a5f284e9f 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs +++ b/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(); - // var isInjectedHeaderValueGetterThrows = false; // mockTextFormat // .Setup(x => x.IsInjected(It.IsAny(), It.IsAny>>())) @@ -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] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs index 3e14632d67e..f1bfcfbd172 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs +++ b/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>(); + 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]