Skip to content

Support W3C Baggage proposed standard in HttpHandlerDiagnosticListener #45496

Open

Description

@jbogard

The W3C Correlation-Context proposed standard was renamed to "baggage". As part of that rename, the header was renamed from Correlation-Context to baggage.

The details of the spec are in public working draft, which remain the same as Correlation-Context except with the header name change.

Since this is a potentially breaking change (server may be expecting the legacy name Correlation-Context, either this can be a clean break (remove/rename Correlation-Context to baggage), or it can be done in phases:

  1. Populate both headers with same value
  2. Remove legacy header name/value

Related: dotnet/aspnetcore#28319

Activity

  1. Dotnet-GitSync-Bot commented on Dec 2, 2020

    @Dotnet-GitSync-Bot
    Collaborator

    I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

  2. changed the title [-]Support W3C Baggage proposed standard[/-] [+]Support W3C Baggage proposed standard in HttpHandlerDiagnosticListener[/+] on Dec 2, 2020
  3. jbogard commented on Dec 2, 2020

    @jbogard
    ContributorAuthor

    @tommcdon I think this is probably yours, I didn't get the label magic to work. It's System.Diagnostics.DiagnosticSource.

  4. added this to the Future milestone on Dec 3, 2020
  5. added
    enhancementProduct code improvement that does NOT require public API changes/additions
    and removed
    untriagedNew issue has not been triaged by the area owner
    on Dec 3, 2020
  6. karelz commented on Dec 3, 2020

    @karelz
    Member

    Triage: We should do it once the spec is final. Is that the case already @jbogard?
    cc @MihaZupan

  7. jbogard commented on Dec 3, 2020

    @jbogard
    ContributorAuthor

    No, it is not. It moved into the first public working draft status back on October 20th, 2020. However, the Correlation-Context header behavior for Baggage was implemented before this standard was in public working draft status.

    It's in an odd place - the name of the standard/header was renamed, and now in public working draft, but it was the "old" name, never standardized, that was implemented for .NET Core 3.0.

  8. karelz commented on Dec 4, 2020

    @karelz
    Member

    Oh, does it mean that we implemented a draft, that won't be part of the final spec?
    We can either act right away, or wait for the spec to mature a bit more ... @MihaZupan what would you recommend?
    Either way, we will have to deal with handling potential breaking change for customers -- are there mitigations we can put in place? (e.g. generating both headers)

  9. 2 remaining items

  10. dominikjeske commented on Oct 3, 2022

    @dominikjeske

    Lack of this functionality makes inconsistent behavior because Activity.Baggage is send via 'Correlation-Context' header and in ASP.NET Core this header is ignored (because change in this PR dotnet/aspnetcore@281679e#diff-f4b6e9bc19da99141d4427d88673ed12fdf3474082f571b027a583cb6959ca99 by @jbogard ) so baggage is lost.
    In other hand when we use OpenTelemetry Baggage.SetBaggage (which sets 'baggage' header in instrumentation code) then it is read by ASP.NET core into Activity.Baggage - so on server side we have same data in Activity.Baggage and Baggage.Current and when we send another request then we will have Correlation-Context (from Activity.Baggage) and baggage (from Baggage.Current when OpenTelemetry instrumentation enabled).
    Generally baggage implementation and usage is now very confusing.

  11. tarekgh commented on Oct 3, 2022

    @tarekgh
    Member

    @dominikjeske can't you use DistributedContextPropagator.Inject to inject the baggage with the desired header name?

  12. dominikjeske commented on Oct 3, 2022

    @dominikjeske

    @tarekgh It is not a problem for me to send something with correct header - I can use OpenTelemetry to include Baggage and read it or I can do it manually with delegating handlers and custom middleware but this is not a point. I described two problems with default behavior which I find confusing:

    1. One header used to send and other used to read
    2. Header duplication when OpenTelemetry is used

    ps. Is there a way do disable propagation of Activity.Baggage?

  13. mxwlf commented on Feb 2, 2024

    @mxwlf

    As of .NET 8, it is still the behavior to send the Correlation-Context header instead of baggage, even when Activity.DefaultIdFormat is set to W3C.
    It is a good thing that ASP.NET supports both headers but this leaves other systems with potentially broken traceability. Have you come to a decision offline? Maybe supporting configuration to send both or another Format option for the legacy Correlation-Context header?

  14. dominikjeske commented on Mar 28, 2024

    @dominikjeske

    Unfortunately my tests on .NET 8 confirms that situation is still broken. Maybe I will put my results here to show how much broken it is especially when using with Open Telemetry and I think many people are using OTel now. My test environment

    PowerShell A -> WebApi B -> WebApi C.

    1. B is filling Activity.Current?.Baggage -> C has Activity.Current?.Baggage propagated by 'Correlation-Context' header and Baggage.Current is not set
    2. B is filling OTel Baggage.Current -> C has Activity.Current?.Baggage propagated by 'Correlation-Context' header and Baggage.Current is set by 'baggage' header
    3. B is filling both Activity.Current?.Baggage and Baggage.Current -> C has propagated Activity.Current?.Baggage and Baggage.Current but only from values of B Baggage.Current (data from Activity is missing)
    4. A is filling baggage -> B receive that and both both Activity.Current?.Baggage and Baggage.Current -> C receive data in header 'baggage' and duplicated in 'Correlation-Context' because first header is from OTel and second from Activity.

    Problem is that ASP.NET Core supports both headers and http client support one and none of this is configurable. Additionally we have two Baggages and logic from OTel. Maybe it is time to clean this?

  15. tarekgh commented on Mar 28, 2024

    @tarekgh
    Member
  16. dominikjeske commented on Apr 15, 2024

    @dominikjeske

    For those who want to change this behavior - you should override DistributedContextPropagator.Current with own implementation instead LegacyPropagator. It is weird that default propagator is named LegacyPropagator :)
    The other confusing point is that when we set something in DistributedContextPropagator it could be ignored because TraceContextPropagator already set this value - I have this when setting traceparent.

  17. lmolkova commented on Jun 5, 2024

    @lmolkova

    The baggage has reached "W3C Candidate Recommendation" state!

    But I don't think it can be implemented without some API changes.
    One of the important features of the W3C baggage is that it does not require distributed tracing (W3C Trace Context) so it SHOULD NOT be part of the Activity.

    I.e. I should be able to use baggage with disabled/dropped activities.

    Leaving aside the baggage API, the propagator needs to be able to inject baggage. It currently has the following methods:

    • IEnumerable<KeyValuePair<string, string?>>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter)
    • void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceIf, out string? traceState)
    • void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter)

    and should get a new method like

    • void InjectBaggage(IEnumerable<KeyValuePair<string, string?>>, object? carrier, PropagatorSetterCallback? setter)

    Slightly off-topic - the propagator API is quite hard to use:

    • I might not have the full activity to inject - ActivityContext should be enough
    • it should be able to extract ActivityContext and {FutureBaggage}
    • the traceId should really be traceparent

    So along with W3C baggage support, it'd be great to consider some other usability improvements.

  18. jbogard commented on Jun 5, 2024

    @jbogard
    ContributorAuthor

    Ah great news!

  19. cijothomas commented on Jun 5, 2024

    @cijothomas
    Contributor

    I think, we can probably workaround as shown below with the existing APIs itself:

    public abstract void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter)
    {
    	// ignore the activity passed to it, as our baggage is not in this activity
    
    	// Obtain OTel’s Baggage using Current!
                     var baggage =	Baggage.Current;
    
    	// inject baggage into carrier using the provided setter.
    }
    

    (Assuming its too late for .NET 9 changes! The proposed additional method is already there, just marked internal.. not sure if that was intentional....)

  20. lmolkova commented on Jun 5, 2024

    @lmolkova

    I think, we can probably workaround as shown below with the existing APIs itself:

    We can definitely workaround in OTel, but not in the .NET implementation of the baggage which should eventually come.

    void InjectBaggage(IEnumerable<KeyValuePair<string, string?>>, object? carrier, PropagatorSetterCallback? setter)

    is probably non-controversial API addition which would still be enough to implement basic W3CPropagator (Trace Context + Baggage) - would be great to have it in .NET 9

  21. tarekgh commented on Jun 5, 2024

    @tarekgh
    Member

    I think the comment is mostly regarding W3C and ignoring the legacy cases.

    I might not have the full activity to inject - ActivityContext should be enough

    The Activity was needed in the injection to inject the Activity.Baggage which not part of the ActivityContext.

    the traceId should really be traceparent

    This is true only for W3C. for legacy activities will be RequestId. I think this is why named traceId covers both cases.

    should get a new method like void InjectBaggage(IEnumerable<KeyValuePair<string, string?>>, object? carrier, PropagatorSetterCallback? setter)

    This makes sense.

  22. fiha8 commented on Jul 9, 2024

    @fiha8

    Hi, I have slightly different scenario. Hope there is somebody able to explain, what is happening in this case:
    I have hybrid instrumentation approach - some services instrumented via OTEL SDK and some service instrumented via dotnet auto-instrumentation (using Kubernetes operator). We can simplify it only into 3 services:

    • Service 1 (instrumented via OTEL SDK)

    • Service 2 (instrumented via dotnet auto-instrumentation).

    • Service 3 (instrumented via OTEL SDK)

    • Service 1 sets OTEL Baggage.

    • I see only correct "baggage" header in Service 2

    • I see only header "Correlation-Context" in Service 3.

    Note: There is no custom logic in Service 2 handling anything around Baggage or Activities - purely auto-instrumented. My expectation is that "baggage" header is simply propagated further as it is.

    Anyone able to explain this behaviour?

  23. dominikjeske commented on Jul 9, 2024

    @dominikjeske

    @fiha8 See my analyse above from 28 March. .NET reads baggage and Correlation-Context but http client LegacyPropagator only understands Correlation-Context so Service2 is sending only this. To fix this you should use your own propagator or use OTel.

  24. dominikjeske commented on Jan 9, 2026

    @dominikjeske
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-System.Net.HttpenhancementProduct code improvement that does NOT require public API changes/additions

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions