Block Transparent Trace Context from Http Requests based on Filter

[stevenmolencsat]

Component

OpenTelemetry.Instrumentation.AspNetCore

Is your feature request related to a problem?

We operate a mixed environment that involves vendor interaction with our systems. We have found the Open Telemetry incoming HTTP instrumentation easily broken when our vendor(s) sets the traceparent header. There's no way to ignore those headers and still use open telemetry with the aspnetcore instrumentation.

The current "Filter" option only is useful if we don't want telemetry or traces at all from a source. This feature would allow us to not be affected by up-stream systems that aren't part of our own open telemetry eco-system, but might be leaking these open telemetry headers, yet still allow us to trace these transactions with ourselves as the starting parent.

What is the expected behavior?

If an origin is in a configurable list, then the instrumentation should ignore any traceparent that might come from the http headers.

Which alternative solutions or features have you considered?

Writing middleware that does essentially the same thing and removes the traceparent header before the instrumentation takes effect.

Additional context

After evaluating the source code, here's a possible solution. First add the list of URIs that you don't want to be part of your trace lineage to the options and then 1 or 2:

1. Within the OpenTelemetry.Instrumentation.AspNetCore.Implementation.HttpInListener in OnStartActivity, when it goes to create the TextMapPropagator, don't consider the http headers.
2. Before it gets to that step where it creates the TextMapPropagator, simply removes the traceparent header from the request headers before logical evaluation.

[stevenmolencsat]

I'd be willing to do the work, just need opinions on if a PR would be acceptable so as whether or not to throw time at this.

[cijothomas]

Writing middleware that does essentially the same thing and removes the traceparent header before the instrumentation takes effect.

This won't work as asp.net core produces its activity at the hosting layer, i.e before middlewares are reached.

[cijothomas]

The solution to this is likely much more involved as asp.net core only respects its own ContextPropagator from .NET runtime, but the instrumentation library respects our own OTel Context propagator. So you'll need to add the ignore-traceparents-origin list in both places.

We need to sunset OTel Propagators in favor of the one from runtime, and then add this capability to the runtime's ContextPropagator. This was explored a bit in #2013 and also discussed in open-telemetry/opentelemetry-dotnet#5667 , but no work has been done to make this happen.

[cijothomas]

The solution to this is likely much more involved as asp.net core only respects its own ContextPropagator from .NET runtime, but the instrumentation library respects our own OTel Context propagator. So you'll need to add the ignore-traceparents-origin list in both places.

We need to sunset OTel Propagators in favor of the one from runtime, and then add this capability to the runtime's ContextPropagator. This was explored a bit in #2013 and also discussed in open-telemetry/opentelemetry-dotnet#5667 , but no work has been done to make this happen.

Once the Propagator issue is unified to have one, then this issue can be addressed by authoring custom Propagator that can be taught to ignore traceparent from certain incoming hosts, and that should be sufficient, given Asp.Net leverages Propagator to see if there is a parent that must be parented to vs creating own root span.

Would you like to explore this further? (i.e explore deprecating OTel contextpropagator with runtime's)

[stevenmolencsat]

Yeah. I wrote a mini-poc locally and there was literally no middleware that could get to the header before .net core had picked it up :s We ended up doing a hack and introducing an "alwaysonsampler" so that it just ignored the trace-flag saying things were previously recorded and went ahead and sampled anyways. it left things in a semi-gross state though with a "missing span" for the parent.

We definitely would like to explore that option but it sounds like there is further infrastructure work before we can get to making a custom propogator @cijothomas or did I read that wrong?

[stevenmolencsat]

I think I see what you are getting at. In this package, we need to use the DistributedContextPropogator instead of the TextMapPropogator, and then also write a CustomPropogator that does the work to ignore certain origins?

Where's the mechanic to add Propogators to the pipeline?

[cijothomas]

we need to use the DistributedContextPropogator instead of the TextMapPropogator,

Both. As ASP.NET Core natively respects DistributedContextPropogator, but the instrumentation library here respects TextMapPropogator, so you need to control both. (pretty bad situation, that needs to be smoothened)

Unfortunately, there aren't much docs around usage of either (there are open issues about documenting this, but no progress has been made). Please do give this a try, and we can help if you get stuck.

[PFAR]

Check out https://github.com/martinjt/otel-ispublicendpoint/blob/27bc2037f9ebab0b59b4a5704cf1027af4937590/src/DisableAllPropagation.cs

Use builder.services.DisableInboundTracePropagation() and in DisableAllContextPropagator:ExtractTraceIdAndState add your own logic to extract traceparent from the carrier object and determine whether it is compatible with aspnetcore instrumentation and set traceId and traceState, and in other cases set them to null to create a new root.

The repo also contains a ProtectedPropagators.cs with example how you can annotate only a subset of externally exposed endpoints with ignore-traceparents-origin functionality