dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

System.Net.Http.DiagnosticsHandler.InjectHeaders does not add baggage to the existing header in the request header #106861

Open efanovroman opened 3 weeks ago

efanovroman commented 3 weeks ago

Description

  1. DiagnosticsHandler the class has a method InjectHeaders, which is executed when transferring activity fields to the request header
{
    _propagator.Inject(currentActivity, request, static (carrier, key, value) =>
    {
        if (carrier is HttpRequestMessage request &&
            key is not null &&
            HeaderDescriptor.TryGet(key, out HeaderDescriptor descriptor) &&
            !request.Headers.TryGetHeaderValue(descriptor, out _))
        {
            request.Headers.TryAddWithoutValidation(descriptor, value);
        }
    );
}
  1. I have a custom SamplePropagator : DistributedContextPropagator which has overridden the InjectBaggage method
    {
    using var e = baggage.GetEnumerator();
    if (e.MoveNext())
    {
        var baggageList = new StringBuilder();
        do
        {
            KeyValuePair<string, string> item = e.Current;
            baggageList.Append(WebUtility.UrlEncode(item.Key))
            .Append('=')
            .Append(WebUtility.UrlEncode(item.Value))
            .Append(CommaWithSpace);
        } while (e.MoveNext());
        setter(carrier, HeaderNames.Baggage, baggageList.ToString(0, baggageList.Length - 2));
    }
    }

Thus the activity baggage is transferred to the request headers baggage

  1. If at the time of calling DiagnosticsHandler.InjectHeaders there is already baggage in the request header (in my case I use the Sentry package Sentry.AspNetCore, which adds various values ​​to the baggage header before this) then the DiagnosticsHandler code on line 474 request.Headers.TryAddWithoutValidation(descriptor, value); is not executed because the condition !request.Headers.TryGetHeaderValue(descriptor, out _) on line 372 is not satisfied.

  2. However, the HttpRequestHeaders.TryAddWithoutValidation method handles the case of adding a new header to an existing value

    HttpRequestMessage t = new HttpRequestMessage();
    t.Headers.TryAddWithoutValidation(HeaderNames.Baggage, "sentry=123");
    t.Headers.TryAddWithoutValidation(HeaderNames.Baggage, "id=123");

result: "[baggage, System.String[]]";baggage;"[sentry=123, id=123]"

Problems I am losing baggage from the current activity because it is not added to the existing header in the request

Configuration .NET Core 8, on windows 10 Propagator SamplePropagator.txt DiagnosticSample DiagnosticSample.txt RequestIdController RequestIdController.txt Startup


{
    services.AddHttpClient("myApi");
    services
        .AddControllers()
        .AddNewtonsoftJson(options =>
        {
            options.SerializerSettings.Converters.Add(new Newtonsoft.Json.Converters.StringEnumConverter());
        })
        .AddDefaultMvcServices();
    services.TryAddEnumerable(ServiceDescriptor.Singleton<IDiagnosticObserver, DiagnosticSample>());
    SamplePropagator.Init();
}
dotnet-policy-service[bot] commented 3 weeks ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.