Skip to content

Sanitize query parameters in url.full telemetry#4088

Open
heaths wants to merge 1 commit intoAzure:mainfrom
heaths:sanitize-url.full
Open

Sanitize query parameters in url.full telemetry#4088
heaths wants to merge 1 commit intoAzure:mainfrom
heaths:sanitize-url.full

Conversation

@heaths
Copy link
Copy Markdown
Member

@heaths heaths commented Apr 3, 2026

Uses LoggingOptions::additional_allowed_query_params.

Uses `LoggingOptions::additional_allowed_query_params`.
@heaths heaths requested a review from LarryOsterman April 3, 2026 21:18
@heaths heaths requested a review from RickWinter as a code owner April 3, 2026 21:18
Copilot AI review requested due to automatic review settings April 3, 2026 21:18
@github-actions github-actions bot added the Azure.Core The azure_core crate label Apr 3, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates azure_core distributed tracing instrumentation to sanitize query parameter values in the url.full span attribute, aligning trace output with the existing logging allow-list behavior.

Changes:

  • Plumbs an allow-list of query parameter names into RequestInstrumentationPolicy and uses Sanitizer::sanitize when setting url.full.
  • Derives the allow-list from DEFAULT_ALLOWED_QUERY_PARAMETERS plus LoggingOptions::additional_allowed_query_params during ClientOptions::deconstruct.
  • Updates/extends tests to validate default redaction behavior and custom allow-list behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sdk/core/azure_core/src/http/policies/instrumentation/request_instrumentation.rs Adds allow-list support to request instrumentation and sanitizes url.full; updates and adds tests.
sdk/core/azure_core/src/http/policies/instrumentation/public_api_instrumentation.rs Updates tests to construct RequestInstrumentationPolicy with the new allow-list parameter.
sdk/core/azure_core/src/http/pipeline.rs Passes the merged allow-list into RequestInstrumentationPolicy when building the pipeline.
sdk/core/azure_core/src/http/options/mod.rs Computes merged allowed query params (defaults + logging additions) and stores them in CoreClientOptions.
sdk/core/azure_core/src/http/options/instrumentation.rs Documents that logging query allow-list impacts traced URL sanitization.
sdk/core/azure_core/CHANGELOG.md Adds an entry describing the new sanitization behavior.

@heaths heaths requested a review from jsquire April 3, 2026 23:31
Copy link
Copy Markdown
Member

@jsquire jsquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't trust my Rust knowledge beyond some very shallow recognition, but the comments that describe the intent seem to match the flow and sanitize appears to be called in the expected locations.

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

Labels

Azure.Core The azure_core crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants