You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
@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.
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.
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.
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.
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.
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).
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.
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).
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.
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.
Activity
davidsh commentedon Apr 23, 2020
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.
stephentoub commentedon Apr 23, 2020
Actually, DiagnosticHandler does:
runtime/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
Line 271 in 4168844
but only if the diagnostics are enabled:
runtime/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs
Line 220 in 4b03513
davidfowl commentedon Apr 23, 2020
See this thread for clarity dotnet/aspnetcore#21012 (comment)
davidfowl commentedon Apr 23, 2020
The docs are lacking in this area cc @noahfalk @shirhatti
shravan2x commentedon Apr 23, 2020
@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
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.
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.
davidsh commentedon Apr 23, 2020
@karelz @stephentoub
davidfowl commentedon Apr 23, 2020
At this point if we decide to revert the behavior the question will be:
cc @SergeyKanzhelev @lmolkova
shravan2x commentedon Apr 23, 2020
Following the usual pattern, something along the lines of
services.AddDiagnostics(options => options.IsActivityPropagationEnabled = true);seems expected.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.
davidfowl commentedon Apr 23, 2020
This would be HttpClientFactory specific? What would it be applied to? Which HttpClients?
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.
karlra commentedon Apr 23, 2020
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.
Why, you hold a particular dislike for me? :)
davidfowl commentedon Apr 23, 2020
I forgot about that. @karlr can you file another issue for that?
18 remaining items
kklldog commentedon Aug 25, 2020
I cant believe this is opt-in !!!! This waste 2 days for me , to find out why my request be ban . /cry
MihaZupan commentedon Jun 18, 2021
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
DiagnosticsHandlerfor the process by setting anAppContextswitch (may be useful for those using versions < 6 in the future).shravan2x commentedon Jun 18, 2021
@MihaZupan Does this work in .NET 5.0? Are there any side effects I should know about?
MihaZupan commentedon Jun 18, 2021
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.
shravan2x commentedon Jun 18, 2021
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).
MihaZupan commentedon Jun 18, 2021
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.
shravan2x commentedon Jun 19, 2021
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.
MihaZupan commentedon Jul 13, 2021
#50658 added a global setting that can be used to disable context propagation process-wide.
With #55392, the HttpClient stack honours that setting:
We are also exposing a property on
SocketsHttpHandler(#55556) that can be used to override behavior for just that instance:For .NET 5 (and 3.1), see the workaround mentioned in #35337 (comment)