Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling/filtering query parameters for URIs in HTTP tracing #74340

Closed
CarnaViire opened this issue Aug 22, 2022 · 3 comments · Fixed by #104970
Closed

Disabling/filtering query parameters for URIs in HTTP tracing #74340

CarnaViire opened this issue Aug 22, 2022 · 3 comments · Fixed by #104970
Assignees
Labels
area-System.Net enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@CarnaViire
Copy link
Member

Similar to #68675, we should consider some kind of configuration for disabling/filtering query parameters for URIs in HTTP tracing.

We should also discuss whether it should be a single config for tracing and logging, or they should be different.

Last time we discussed it, we believed it will be better to keep them separate for discoverability -- configuring HttpClientFactory's logging would make more sense as a part of general HttpClientFactory configuration.

Consuming tracing requires more conscious effort, so a setting there might be less evident (e.g. AppContext switch).

We also discussed that we generally don't like messing with all Uris at once (e.g. setting that will apply globally to all Uri.ToString)

Note that Barry's opinion was to have a single setting:

It’s difficult, because I can see folks taking trace events and saving them for debugging or security parsing. And once that happens it’s exactly the same problem. I’d say match the two.

So I wonder maybe tracing's setting can apply to logging, but not the other way around.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 22, 2022
@ghost
Copy link

ghost commented Aug 22, 2022

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

Issue Details

Similar to #68675, we should consider some kind of configuration for disabling/filtering query parameters for URIs in HTTP tracing.

We should also discuss whether it should be a single config for tracing and logging, or they should be different.

Last time we discussed it, we believed it will be better to keep them separate for discoverability -- configuring HttpClientFactory's logging would make more sense as a part of general HttpClientFactory configuration.

Consuming tracing requires more conscious effort, so a setting there might be less evident (e.g. AppContext switch).

We also discussed that we generally don't like messing with all Uris at once (e.g. setting that will apply globally to all Uri.ToString)

Note that Barry's opinion was to have a single setting:

It’s difficult, because I can see folks taking trace events and saving them for debugging or security parsing. And once that happens it’s exactly the same problem. I’d say match the two.

So I wonder maybe tracing's setting can apply to logging, but not the other way around.

Author: CarnaViire
Assignees: -
Labels:

area-System.Net

Milestone: -

@MihaZupan
Copy link
Member

Just to confirm, are we are only concerned about the pathAndQuery parameter of the RequestStart event for System.Net.Http?

If we were to consider the path as well, we should work with ASP.NET as they also have tracing that includes such info (e.g. KestrelEventSource.RequestStart).

@karelz karelz added this to the 8.0.0 milestone Aug 23, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 23, 2022
@karelz karelz added the enhancement Product code improvement that does NOT require public API changes/additions label Aug 23, 2022
@karelz
Copy link
Member

karelz commented Aug 23, 2022

Triage: Would be nice to get it in 8.0 as it makes security easier for specific set of users.

@karelz karelz modified the milestones: 8.0.0, Future Jun 9, 2023
@karelz karelz modified the milestones: Future, 9.0.0 Jul 18, 2023
@antonfirsov antonfirsov self-assigned this Jun 4, 2024
@CarnaViire CarnaViire assigned CarnaViire and unassigned antonfirsov Jul 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jul 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants