Skip to content

[Http] Redact only named query string parameters - #4954

Open
PetarJerinic wants to merge 9 commits into
open-telemetry:mainfrom
PetarJerinic:http-sensitive-query-parameters
Open

[Http] Redact only named query string parameters#4954
PetarJerinic wants to merge 9 commits into
open-telemetry:mainfrom
PetarJerinic:http-sensitive-query-parameters

Conversation

@PetarJerinic

@PetarJerinic PetarJerinic commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #1791

Changes

OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_SENSITIVE_QUERY_PARAMETERS takes a comma-separated list of query parameter names. Those values are replaced with REDACTED in url.full; every other value survives.
Names are matched case-sensitively, per semconv. Applies to .NET Framework and up to .NET 8.

There are no default query parameters giving the user full control.

Open questions

  • RedactionHelper.SensitiveRedactedText - should it be a separate variable or should existing RedactionHelper.RedactedText be used? A new variable was added since semconv states that REDACTED should be uppercase. I am happy to use the existing one to maintain consistent behavior, or update all usages of the existing one to the new one.
  • ~In HttpHandlerDiagnosticListener (lines 150-159, .NET 9+ runtimes) - Runtime writes url.full itself and redacts the query to ?*. When this variable is set, the instrumentation overwrites that one attribute. This goes against the README's "does not override native instrumentation" guarantee (line 47) for opted-in users. Should this change be removed or should the README be amended?
    Resolved: per suggestion below, .NET 9+ behavior remains the same

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

…PARAMETERS env variable that names the parameters to redact and preserves the rest.
@github-actions github-actions Bot added the comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http label Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.93%. Comparing base (034bdf8) to head (8fc5afb).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4954      +/-   ##
==========================================
+ Coverage   77.88%   77.93%   +0.05%     
==========================================
  Files         473      473              
  Lines       20205    20248      +43     
==========================================
+ Hits        15736    15781      +45     
+ Misses       4469     4467       -2     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 90.22% <100.00%> (+0.30%) ⬆️
unittests-Exporter.Geneva 56.72% <ø> (ø)
unittests-Exporter.InfluxDB 97.96% <ø> (ø)
unittests-Exporter.OneCollector 95.60% <ø> (ø)
unittests-Extensions 95.74% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 85.35% <ø> (ø)
unittests-Instrumentation.AspNet 79.01% <ø> (ø)
unittests-Instrumentation.AspNetCore 87.20% <ø> (ø)
unittests-Instrumentation.Cassandra 92.30% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 83.53% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.36% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 81.02% <ø> (ø)
unittests-Instrumentation.EventCounters 75.45% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.12% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 78.09% <ø> (ø)
unittests-Instrumentation.Hangfire 88.88% <ø> (ø)
unittests-Instrumentation.Http 75.86% <100.00%> (+0.25%) ⬆️
unittests-Instrumentation.Kusto 92.42% <ø> (ø)
unittests-Instrumentation.Owin 89.02% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 77.77% <ø> (ø)
unittests-Instrumentation.Remoting 65.59% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 39.91% <ø> (ø)
unittests-Instrumentation.SqlClient 83.36% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 94.98% <ø> (ø)
unittests-Instrumentation.Wcf 82.64% <ø> (ø)
unittests-OpAmp.Client 87.05% <ø> (+0.17%) ⬆️
unittests-PersistentStorage 69.20% <ø> (ø)
unittests-Resources.AWS 72.78% <ø> (ø)
unittests-Resources.Azure 84.18% <ø> (ø)
unittests-Resources.Container 80.76% <ø> (ø)
unittests-Resources.Gcp 80.70% <ø> (ø)
unittests-Resources.Host 72.34% <ø> (ø)
unittests-Resources.OperatingSystem 77.16% <ø> (ø)
unittests-Resources.Process 90.90% <ø> (ø)
unittests-Resources.ProcessRuntime 80.76% <ø> (ø)
unittests-Sampler.AWS 96.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tion.Http/HttpClientTraceInstrumentationOptions.cs 100.00% <100.00%> (ø)
...tp/Implementation/HttpHandlerDiagnosticListener.cs 74.60% <100.00%> (ø)
...strumentation.Http/Implementation/HttpTagHelper.cs 100.00% <100.00%> (ø)
...plementation/HttpWebRequestActivitySource.netfx.cs 82.12% <100.00%> (ø)
src/Shared/RedactionHelper.cs 93.93% <100.00%> (+3.17%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@PetarJerinic
PetarJerinic marked this pull request as ready for review August 10, 2026 20:10
@PetarJerinic
PetarJerinic requested a review from a team as a code owner August 10, 2026 20:10
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 10, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-08-15 20:05 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@martincostello

Copy link
Copy Markdown
Member

In HttpHandlerDiagnosticListener (lines ~150-159, .NET 9+ runtimes) - Runtime writes url.full itself and redacts the query to ?*. When this variable is set, the instrumentation overwrites that one attribute. This goes against the README's "does not override native instrumentation" guarantee (line 47) for opted-in users. Should this change be removed or should the README be amended?

My initial thought is that we should stick with the README and not overwrite it, which effectively means implementing this request is two pieces of work, because the .NET runtime would also need to implement it. However they almost certainly won't implement it while it's non-stable.

Here's some further links and context here around how url.query redaction and configurability is handled in various parts of .NET (or not):

Comment thread src/OpenTelemetry.Instrumentation.Http/HttpClientTraceInstrumentationOptions.cs Outdated
@PetarJerinic

Copy link
Copy Markdown
Contributor Author

@martincostello I updated the SensitiveQueryParameters description and removed the change in HttpHandlerDiagnosticListener.

I also updated the description of PR, please re-review when time permits.

With your comment in mind for url.query redaction and configurability, there will be some wait time until this is supported in newer versions of .NET.

@martincostello

Copy link
Copy Markdown
Member

@rajkumar-rangaraj Thoughts on the path forward for this PR after our discussion in the SIG call this Tuesday?

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

Labels

comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Http Instrumentation] Make it easier to scrub sensitive URI details from output

2 participants