Skip to content

fix(tracing): handle env var values containing =#18137

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
kowalski/fix-tracing-handle-env-var-values-containing
May 21, 2026
Merged

fix(tracing): handle env var values containing =#18137
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
kowalski/fix-tracing-handle-env-var-values-containing

Conversation

@KowalskiThomas
Copy link
Copy Markdown
Contributor

@KowalskiThomas KowalskiThomas commented May 18, 2026

Description

Previously, we would split without a component count limit, meaning if we had ENV=something=else an exception would be raised and we would fail to process it. Added a limit of 2 components so that we get the env variable name and discard all the rest, making it safer.

On top of that, the PR adds support for env var names containing digits -- those didn't use to match the regex and were thus ignored from scrubbing.

@KowalskiThomas KowalskiThomas added the Tracing Distributed Tracing label May 18, 2026
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented May 18, 2026

Benchmarks

Benchmark execution time: 2026-05-18 15:04:52

Comparing candidate commit 6353bcd in PR branch kowalski/fix-tracing-handle-env-var-values-containing with baseline commit 7ec0ac7 in branch main.

Found 0 performance improvements and 4 performance regressions! Performance is the same for 591 metrics, 10 unstable metrics.

scenario:iastaspects-stringio_aspect

  • 🟥 execution_time [+547.378µs; +584.518µs] or [+13.972%; +14.920%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+100.876µs; +107.477µs] or [+23.287%; +24.811%]

scenario:span-start

  • 🟥 execution_time [+1.477ms; +1.623ms] or [+9.489%; +10.431%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+235.548ns; +269.327ns] or [+11.227%; +12.837%]

…ubbing

`[A-Z_]+` excluded digits, so names like `BASE64` were not recognised
as env-var tokens and their values were never redacted.
@cit-pr-commenter-54b7da
Copy link
Copy Markdown

Codeowners resolved as

ddtrace/contrib/internal/subprocess/patch.py                            @DataDog/asm-python
releasenotes/notes/fix-subprocess-env-var-multi-equals-c8f20ac468cdd905.yaml  @DataDog/apm-python
tests/contrib/subprocess/test_subprocess.py                             @DataDog/asm-python

@datadog-datadog-prod-us1-2
Copy link
Copy Markdown
Contributor

datadog-datadog-prod-us1-2 Bot commented May 19, 2026

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: b0115b2 | Docs | Datadog PR Page | Give us feedback!

@KowalskiThomas KowalskiThomas marked this pull request as ready for review May 21, 2026 06:16
@KowalskiThomas KowalskiThomas requested review from a team as code owners May 21, 2026 06:16
@KowalskiThomas
Copy link
Copy Markdown
Contributor Author

/merge -f --reason "MQ failing for unrelated reasons (benchmark SLOs)"

@gh-worker-devflow-routing-ef8351
Copy link
Copy Markdown

gh-worker-devflow-routing-ef8351 Bot commented May 21, 2026

View all feedbacks in Devflow UI.

2026-05-21 12:26:27 UTC ℹ️ Start processing command /merge -f --reason "MQ failing for unrelated reasons (benchmark SLOs)"


2026-05-21 12:26:33 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 0s (p90).


2026-05-21 12:26:45 UTC ℹ️ MergeQueue: This merge request was merged

Warning

This change was merged without running any pre merge CI checks

Reason: MQ failing for unrelated reasons (benchmark SLOs)

@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d Bot merged commit 131e904 into main May 21, 2026
348 checks passed
@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d Bot deleted the kowalski/fix-tracing-handle-env-var-values-containing branch May 21, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tracing Distributed Tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants