Skip to content

HttpClient automatically adds Request-Id HTTP header #35337

Closed

Description

@shravan2x

I have an application that sends HTTP requests via HttpClient from an Asp.NET Core project. For unknown reasons, all sent requests seem to have a Request-Id HTTP header. I'm not able to repro this in a regular console application. The docs don't have any information on disabling it.

They seem to be added by the framework https://github.com/dotnet/runtime/search?q=RequestIdHeaderName&unscoped_q=RequestIdHeaderName .

Activity

  1. davidsh commented on Apr 23, 2020

    @davidsh
    Contributor

    For unknown reasons, all sent requests seem to have a Request-Id HTTP header.

    HttpClient API does not add "Request-Id" HTTP request header.

    How did you confirm that HttpClient was adding this request header? Was it by inspecting request headers received by the server? It's quite possible that a front-end proxy etc. is adding these headers on your behalf.

  2. stephentoub commented on Apr 23, 2020

    @stephentoub
    Member

    HttpClient API does not add "Request-Id" HTTP request header.

    Actually, DiagnosticHandler does:

    request.Headers.TryAddWithoutValidation(DiagnosticsHandlerLoggingStrings.RequestIdHeaderName, currentActivity.Id);

    but only if the diagnostics are enabled:

  3. davidfowl commented on Apr 23, 2020

    @davidfowl
    Member

    See this thread for clarity dotnet/aspnetcore#21012 (comment)

  4. davidfowl commented on Apr 23, 2020

    @davidfowl
    Member

    The docs are lacking in this area cc @noahfalk @shirhatti

  5. shravan2x commented on Apr 23, 2020

    @shravan2x
    Author

    @davidsh You can see how surprised I was when I realized the framework was doing this, and yet again just now when I heard that this is intentional behavior, and that there are no plans to fix it.

    @davidfowl

    That's fair, but we went with an opt-out model so that the "it just works when I call my other ASP.NET Core app or backend that supports the new W3C trace context" is on by default.

    I strongly disagree with this. Not only is it sending behavior modifying headers without explicitly opting-in, it leaks private information about my HTTP client.

    In what universe is it OK to post data that I didn't explicitly ask for to a remote location????

    As much I hate to say it, I agree with @karlra's strongly worded views on this.

    Even if it had been well documented, surely we don't expect every user of .NET to use a proxy, identify the weird header, google it, and then add code to opt-out? Why can't we ask users to add a line of code in their Startup.cs if they actually want such behavior?

    I doubt the vast majority of Asp.NET Core users know or care about features like distributed tracing. Such a feature should have been designed to be opt-in. I'm confident you'll hear the same opinion if a public poll was conducted for this.

  6. davidsh commented on Apr 23, 2020

    @davidsh
    Contributor
  7. davidfowl commented on Apr 23, 2020

    @davidfowl
    Member

    At this point if we decide to revert the behavior the question will be:

    • What is the gesture to turn this on for an outbound calls?
    • What about scenarios where you don't control how the HttpClient is constructed?

    cc @SergeyKanzhelev @lmolkova

  8. shravan2x commented on Apr 23, 2020

    @shravan2x
    Author

    What is the gesture to turn this on for an outbound calls?

    Following the usual pattern, something along the lines of services.AddDiagnostics(options => options.IsActivityPropagationEnabled = true); seems expected.

    What about scenarios where you don't control how the HttpClient is constructed?

    I'm unsure what such a scenario might be. However, I think having it be controlled by a flag like the example above is reasonable.

  9. davidfowl commented on Apr 23, 2020

    @davidfowl
    Member

    Following the usual pattern, something along the lines of services.AddDiagnostics(options => options.IsActivityPropagationEnabled = true); seems expected.

    This would be HttpClientFactory specific? What would it be applied to? Which HttpClients?

    I'm unsure what such a scenario might be. However, I think having it be controlled by a flag like the example above is reasonable.

    A global static? Where do I set it, which HttpClients does it affect? Today the HttpClients that are affected are ones that run within the current activity. That's why it can be disabled on a granular basis.

  10. karlra commented on Apr 23, 2020

    @karlra

    I strongly disagree with this. Not only is it sending behavior modifying headers without explicitly opting-in, it leaks private information about my HTTP client.

    I obviously also disagree strongly with this. Imagine the number of issues that will be raised on here about it.....The concatenation of the header that takes place if you try to set this header yourself is even worse.

    In what universe is it OK to post data that I didn't explicitly ask for to a remote location????

    As much I hate to say it, I agree with @karlra's strongly worded views on this.

    Why, you hold a particular dislike for me? :)

  11. davidfowl commented on Apr 23, 2020

    @davidfowl
    Member

    The concatenation of the header that takes place if you try to set this header yourself is even worse.

    I forgot about that. @karlr can you file another issue for that?

  12. 18 remaining items

  13. modified the milestones: Future, 6.0.0 on Aug 20, 2020
  14. kklldog commented on Aug 25, 2020

    @kklldog

    I cant believe this is opt-in !!!! This waste 2 days for me , to find out why my request be ban . /cry

  15. MihaZupan commented on Jun 18, 2021

    @MihaZupan
    Member

    While we are working on a proper solution for 6.0, if you don't care about distributed tracing at all, you can completely disable the DiagnosticsHandler for the process by setting an AppContext switch (may be useful for those using versions < 6 in the future).

    AppContext.SetSwitch("System.Net.Http.EnableActivityPropagation", false);
  16. shravan2x commented on Jun 18, 2021

    @shravan2x
    Author

    @MihaZupan Does this work in .NET 5.0? Are there any side effects I should know about?

  17. MihaZupan commented on Jun 18, 2021

    @MihaZupan
    Member

    Yes the switch has been there for quite a while.

    It will completely disable the DiagnosticsHandler.
    The effect is that the DiagnosticListener will never fire events from DiagnosticsHandler, no activity will be created inside http requests, and we won't touch any context headers (such as request id, tracestate, correlation context..).

    All of these would only matter if you care about being a part of the distributed trace.

  18. shravan2x commented on Jun 18, 2021

    @shravan2x
    Author

    Glad to hear! I strongly doubt any average user would want to opt-in to an analytics/tracking system like this without fully understanding it (and they probably haven't even heard of it).

  19. MihaZupan commented on Jun 18, 2021

    @MihaZupan
    Member

    I don't think having it be on by default was a bad choice since realistically it's helping users that care about it while not really affecting those who don't.
    If you don't care about it you risk running into very strict services that disallow extra headers, but that seems to me like the service is being too aggressive.

    It isn't leaking private information - these are just long random numbers for the most part.

  20. shravan2x commented on Jun 19, 2021

    @shravan2x
    Author

    If I understood the feature correctly, it leaks which other servers your program made a request to. This is especially concerning with large cloud solutions like AWS or Azure because they can tie different child requests together.

  21. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Jul 9, 2021
  22. MihaZupan commented on Jul 13, 2021

    @MihaZupan
    Member

    #50658 added a global setting that can be used to disable context propagation process-wide.
    With #55392, the HttpClient stack honours that setting:

    DistributedContextPropagator.Current = DistributedContextPropagator.CreateNoOutputPropagator();

    We are also exposing a property on SocketsHttpHandler (#55556) that can be used to override behavior for just that instance:

    using var client = new HttpClient(new SocketsHttpHandler
    {
        ActivityHeadersPropagator = null
    });

    For .NET 5 (and 3.1), see the workaround mentioned in #35337 (comment)

  23. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Jul 13, 2021
  24. ghost locked as resolved and limited conversation to collaborators on Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions