feat(hosting): add url.query redaction for telemetry sensitive parameter#65369
feat(hosting): add url.query redaction for telemetry sensitive parameter#65369claudiogodoy99 wants to merge 2 commits intodotnet:mainfrom
Conversation
…ters Introduce UrlQueryRedactionOptions to redact sensitive query string values (e.g., tokens, passwords, API keys) from the url.query OpenTelemetry attribute on HTTP request activities. Configurable sensitive parameter names and placeholder text. Wired through GenericWebHostService, HostingApplication, and HostingApplicationDiagnostics. Includes unit tests.
|
Thanks for your PR, @@claudiogodoy99. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in mechanism for emitting the OpenTelemetry url.query activity tag while redacting configured “sensitive” query-string parameters, to prevent secrets from being captured in HTTP server telemetry.
Changes:
- Introduces a new public
UrlQueryRedactionOptionsAPI to configure sensitive parameter names and a redaction placeholder. - Updates Hosting diagnostics/tag initialization to optionally add
url.querywith redaction applied. - Adds a helper (
HostingTelemetryHelpers.GetRedactedQueryString) and unit tests validating redaction behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs | Adds unit tests validating inclusion/exclusion and redaction behavior for url.query. |
| src/Hosting/Hosting/src/PublicAPI.Unshipped.txt | Declares the new public UrlQueryRedactionOptions API surface. |
| src/Hosting/Hosting/src/Internal/WebHost.cs | Attempts to resolve UrlQueryRedactionOptions from DI and pass to HostingApplication. |
| src/Hosting/Hosting/src/Internal/UrlQueryRedactionOptions.cs | Adds the new public options type and default sensitive parameter list. |
| src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs | Adds helper to redact sensitive query parameter values. |
| src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs | Optionally emits url.query tag during activity creation when options are provided. |
| src/Hosting/Hosting/src/Internal/HostingApplication.cs | Plumbs redaction options into diagnostics. |
| src/Hosting/Hosting/src/GenericHost/GenericWebHostService.cs | Attempts to resolve/pass redaction options in generic-host startup path. |
| IWebHostEnvironment hostingEnvironment, | ||
| HostingMetrics hostingMetrics) | ||
| HostingMetrics hostingMetrics, | ||
| IOptions<UrlQueryRedactionOptions>? urlQueryRedactionOptions) |
There was a problem hiding this comment.
IOptions<UrlQueryRedactionOptions> will typically always be resolvable because the hosting stack registers services.AddOptions() (e.g., WebHostBuilder.cs). As a result, this parameter will be provided even when the app never configured redaction, which defeats the intended opt-in behavior (the feature will look enabled by default). Consider gating enablement on the presence of actual configuration (e.g., IConfigureOptions<UrlQueryRedactionOptions> registrations) or using a dedicated/marker service so you can pass null unless explicitly enabled.
| HostingEnvironment = hostingEnvironment; | ||
| HostingMetrics = hostingMetrics; | ||
| UrlQueryRedactionOptions = urlQueryRedactionOptions?.Value; |
There was a problem hiding this comment.
Assigning UrlQueryRedactionOptions = urlQueryRedactionOptions?.Value will produce a non-null default options instance in most apps (since IOptions<T> is available for any T once options are registered). This will cause url.query tagging/redaction to run even when the app didn't opt in. To preserve backward compatibility, only set this property when redaction has been explicitly enabled/configured; otherwise leave it null.
| var httpContextFactory = _applicationServices.GetRequiredService<IHttpContextFactory>(); | ||
| var hostingMetrics = _applicationServices.GetRequiredService<HostingMetrics>(); | ||
| var hostingApp = new HostingApplication(application, _logger, diagnosticSource, activitySource, propagator, httpContextFactory, HostingEventSource.Log, hostingMetrics); | ||
| var urlQueryRedactionOptions = _applicationServices.GetService<IOptions<UrlQueryRedactionOptions>>()?.Value; |
There was a problem hiding this comment.
GetService<IOptions<UrlQueryRedactionOptions>>()?.Value is not a reliable way to detect whether the app configured redaction. Options are registered via services.AddOptions() during web host setup, so this call will almost always return a default instance, unintentionally enabling url.query tagging for all apps. If the intent is opt-in, gate on the presence of configuration/marker registrations (e.g., GetServices<IConfigureOptions<UrlQueryRedactionOptions>>().Any()) and only then resolve IOptions<UrlQueryRedactionOptions>.
| var urlQueryRedactionOptions = _applicationServices.GetService<IOptions<UrlQueryRedactionOptions>>()?.Value; | |
| var hasUrlQueryRedactionConfiguration = _applicationServices.GetServices<IConfigureOptions<UrlQueryRedactionOptions>>().Any(); | |
| UrlQueryRedactionOptions? urlQueryRedactionOptions = null; | |
| if (hasUrlQueryRedactionConfiguration) | |
| { | |
| urlQueryRedactionOptions = _applicationServices.GetRequiredService<IOptions<UrlQueryRedactionOptions>>().Value; | |
| } |
| /// <returns>The redacted query string, or null if the query string is empty.</returns> | ||
| public static string? GetRedactedQueryString(QueryString queryString, UrlQueryRedactionOptions options) | ||
| { | ||
| if (!queryString.HasValue) |
There was a problem hiding this comment.
The GetRedactedQueryString method declaration is mis-indented compared to the surrounding file, and the formatting doesn’t match the established style in this file. Please align indentation to the rest of the class for consistency.
Performance ConcernsI've Benchmark some approaches for the new method Bench Results| Method | Categories | Mean | Error | StdDev | Gen0 | Allocated |
|------------------- |--------------- |------------:|-----------:|-----------:|-------:|----------:|
| ManyParams | 12_Params | 439.2046 ns | 20.5173 ns | 13.5709 ns | 0.1125 | 944 B |
| | | | | | | |
| AllSensitiveParams | 3_AllSensitive | 201.2194 ns | 9.4309 ns | 4.9325 ns | 0.0715 | 600 B |
| | | | | | | |
| NoSensitiveParams | 3_NonSensitive | 151.1875 ns | 5.1225 ns | 3.3882 ns | 0.0410 | 344 B |
| | | | | | | |
| MixedParams | 5_Mixed | 279.1219 ns | 14.9330 ns | 9.8773 ns | 0.0858 | 720 B |
| | | | | | | |
| EmptyQueryString | Empty | 0.9674 ns | 0.0814 ns | 0.0484 ns | - | - |
| | | | | | | |
| SingleParam | SingleParam | 81.2347 ns | 5.7101 ns | 3.7769 ns | 0.0257 | 216 B |
| | | | | | | |
| SpecialCharsParams | SpecialChars | 218.3186 ns | 6.4507 ns | 3.8387 ns | 0.0639 | 536 B |Bench Code |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
Add configurable query string redaction for OpenTelemetry url.query attribute
Summary of the changes (Less than 80 chars)
Summary
This PR introduces configurable redaction of sensitive query string parameters in the
url.queryOpenTelemetry attribute for HTTP request activities, preventing sensitive data (tokens, passwords, API keys) from being exposed in telemetry.Changes
New
UrlQueryRedactionOptionsAPI:UrlQueryRedactionOptionsclass with configurable:SensitiveQueryParameters: HashSet of parameter names to redact (default includes:token,api_key,apikey,access_token,password,secret,auth)RedactedPlaceholder: Custom placeholder text (default:[Redacted])Core Integration:
GenericWebHostServiceto retrieve and passUrlQueryRedactionOptionsfrom DI containerHostingApplicationconstructor to accept optionalUrlQueryRedactionOptionsHostingApplicationDiagnosticsto apply redaction when settingurl.queryactivity tagHostingTelemetryHelpers.RedactQueryString()method to perform the actual redaction logic with URL encodingBehavior:
UrlQueryRedactionOptionsis NOT configured: The behavior remains exactly as before — theurl.queryattribute is not included in telemetry (fully backward compatible, no breaking changes)UrlQueryRedactionOptionsIS configured (via DI), theurl.queryattribute is included with sensitive values redactedTesting:
Motivation
Address issue: #64850