Add an options to flow the incoming distributed tracing headers

[Kahbazi]

Currently YARP would remove the distributed tracing headers from an incoming requests, and the new updated headers is being added to it by SocketsHttpHandler. These new headers counts YARP as a SPAN, and if we look at the distributed tracing in a tool like Zipkin, the hierarchy would be [client -> YARP -> Destination]. Although This would happen only when all three applications are logging to a distributed tracing system.

There might be some scenarios when YARP is not configured to be in part of distributed tracing system or users don't want to count YARP as a SPAN in distributed tracing system. In this case I think the hierarchy between client and destination would be lost because of the missing link (YARP). In this case an option for clusters to just flow the incoming distributed tracing headers is helpful.

[MihaZupan]

This is exactly what the PassThroughPropagator was designed for.

DistributedContextPropagator.Current = DistributedContextPropagator.CreatePassThroughPropagator();

// Or
services
    .AddReverseProxy()
    .ConfigureHttpClient((context, handler) =>
    {
        handler.ActivityHeadersPropagator = DistributedContextPropagator.CreatePassThroughPropagator();
    });

May be worth a mention in #1342

[Kahbazi]

Cool, I didn't know about this option.
Now should this be an option in HttpClientConfig? It might be useful when having YARP as an exe or docker, so this could be set via config instead of code.

[Kahbazi]

This is what could be added. I will send a PR if you agree with the design.

public enum DistributedContextPropagator
{
    Default = 0, // CreateDefaultPropagator()
    PassThrough = 1, // CreatePassThroughPropagator()
    NoOutput = 2, // CreateNoOutputPropagator()
    Disable = 3 // null , (I'm not sure if there's any behavior different between setting null and CreateNoOutputPropagator)
}

public class HttpClientConfig
{
+    public DistributedContextPropagator? DistributedContextPropagator  { get; init; }
}

[Tratcher]

This would only be for 6.0+, right?

[Kahbazi]

This would only be for 6.0+, right?

Yes, I believe so.

[MihaZupan]

It would be 6.0+ only, but nothing stops the user from implementing what they need on top of something like the sample DiagnosticsHandlerFactory

I'm not sure if there's any behavior different between setting null and CreateNoOutputPropagator

NoOutputPropagator means HttpClient will still raise diagnostic events, but won't add headers.
null means no events and no headers - the handler won't be included at all.

In the context of Yarp, I think the following options would make sense:

1. remove headers
2. keep the original headers (passthrough, 5.0 default)
3. maintain the trace and create a new span (6.0 default)

Only the first two should ever be useful to set from the config - for the third you would want code instrumentation.

[karelz]

Triage: We think default will work for most people. If we have more upvotes, we can add a config.
Leaving it open for a while to collect more feedback.

[samsp-msft]

I'm a bit confused as to what needs to be done here: it sounds like adding the passthrough propagator solves the issue. If so, we should add to the samples/docs.

[karelz]

Triage: We have it doc'ed a bit, so no need to do more until we have more upvotes.