Skip to content

[OpenTelemetry] Fix-up redaction behaviour - #8104

Open
martincostello wants to merge 3 commits into
open-telemetry:mainfrom
martincostello:fix/redact-resource-attributes-empty-pairs
Open

martincostello wants to merge 3 commits into
open-telemetry:mainfrom
martincostello:fix/redact-resource-attributes-empty-pairs

Conversation

@martincostello

Copy link
Copy Markdown
Member

Fixes #8102 (comment).

Changes

Avoid emitting dangling separators.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, successful build/tests, etc.)
  • Tests added/updated
  • CHANGELOG.md file(s) updated for non-trivial changes
  • Changes in public API reviewed (if applicable)
  • AI usage disclosure:
    • No AI used
    • AI-assisted
    • Bulk AI-generated
  • I have the experience and knowledge necessary to understand, review, and validate all content in this PR1

Footnotes

  1. Yes, I can answer maintainer questions about the content of this PR without using AI. ↩

@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Nov 10, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-11-10 12:52 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The self-diagnostics output change for OTEL_RESOURCE_ATTRIBUTES lacks the required Unreleased changelog entry with a PR link.

1 open issue
What changed in this PR

This follow-up to #8102 fixes dangling separators in the SDK's self-diagnostics display of OTEL_RESOURCE_ATTRIBUTES, while preserving redaction of malformed non-empty entries.

Changes:

  • Skip empty attribute pairs and emit commas only between retained entries.
  • Expand regression tests to cover leading, trailing, repeated, and empty-only separators.
File Description
test/​OpenTelemetry.Tests/​Diagnostics/​SelfDiagnosticsEnvironmentVariablePolicyTests.cs Adds parameterized coverage for malformed and empty attribute pairs.
src/​OpenTelemetry/​Internal/​Diagnostics/​SelfDiagnosticsEnvironmentVariablePolicy.cs Moves empty-pair handling into the parsing loop to prevent dangling commas.

Review effort: Balanced

@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.03%. Comparing base (bbf1923) to head (a434dd7).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8104      +/-   ##
==========================================
- Coverage   93.04%   93.03%   -0.02%     
==========================================
  Files         399      399              
  Lines       23476    23475       -1     
  Branches     4389     4389              
==========================================
- Hits        21843    21839       -4     
- Misses       1113     1115       +2     
- Partials      520      521       +1     
Flag Coverage Δ
unittests-Project-Experimental 93.13% <100.00%> (-0.03%) ⬇️
unittests-Project-Stable 93.12% <100.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...ostics/SelfDiagnosticsEnvironmentVariablePolicy.cs 98.63% <100.00%> (-0.01%) ⬇️

... and 3 files with indirect coverage changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the fix to dangling commas in self-diagnostics output.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@martincostello
martincostello marked this pull request as ready for review November 10, 2026 12:32
@martincostello
martincostello requested a review from a team as a code owner November 10, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants