Skip to content

fix(#2076): make db.response.error attribute selection consistent with other optional attributes#2086

Merged
grcevski merged 7 commits into
open-telemetry:mainfrom
NameHaibinZhang:feature/fix-2076
May 13, 2026
Merged

fix(#2076): make db.response.error attribute selection consistent with other optional attributes#2086
grcevski merged 7 commits into
open-telemetry:mainfrom
NameHaibinZhang:feature/fix-2076

Conversation

@NameHaibinZhang
Copy link
Copy Markdown
Member

@NameHaibinZhang NameHaibinZhang commented May 11, 2026

Description

When db.response.error is not included in attributes.select.traces, the span status message is now left empty instead of showing a placeholder hint. This makes the behavior consistent with how other optional attributes (e.g. db.query.text) work — when not selected, they are simply omitted.

Changes

  • Remove the DBErrorMessagePlaceholder constant
  • Update DBResponseErrorAttr() to return nil when the attribute is not selected
  • Update config schema description to clarify that attributes.select applies to both metrics and traces
  • Update tests to reflect the new behavior

Fixes #2076

@NameHaibinZhang NameHaibinZhang changed the title docs: document db.response.error trace selection (#2076) fix(#2076): add document db.response.error trace selection May 11, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.52%. Comparing base (a987882) to head (6adaec0).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2086      +/-   ##
==========================================
+ Coverage   69.19%   69.52%   +0.32%     
==========================================
  Files         289      289              
  Lines       36234    36286      +52     
==========================================
+ Hits        25073    25228     +155     
+ Misses       9805     9685     -120     
- Partials     1356     1373      +17     
Flag Coverage Δ
integration-test 53.65% <0.00%> (+0.55%) ⬆️
integration-test-arm 28.41% <0.00%> (+0.18%) ⬆️
integration-test-vm-x86_64-5.15.152 ?
integration-test-vm-x86_64-6.10.6 ?
k8s-integration-test 41.81% <0.00%> (-0.74%) ⬇️
oats-test 37.82% <100.00%> (-0.14%) ⬇️
unittests 61.44% <100.00%> (+0.52%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

CI Supervisor

Workflow Job Last state Re-running? Attempt
Pull request integration tests shard-1 (11 tests) failure Yes 1/2
Pull request checks Lint failure No 1/2
Pull request integration tests ARM shard-1 (1 tests) failure Yes 1/2

…eparate file

Update the Selection type comment and JSON schema description to mention
the traces key for trace attribute selection. Regenerate CONFIG.md to match
the auto-generated output (removing free-form sections that break the
check-config-schema CI check). Move the detailed db.response.error
documentation into devdocs/db-response-error.md.
Comment thread devdocs/db-response-error.md Outdated
Comment thread devdocs/attribute-selection.md Outdated
Rename devdocs/db-response-error.md to devdocs/attribute-selection.md and
update the title to be more general as suggested in review.
When db.response.error is not selected, omit the status message entirely
instead of adding a placeholder hint. This aligns with how other optional
attributes (e.g. db.query.text) behave — they are simply not present when
not selected, without any placeholder.
Copy link
Copy Markdown
Contributor

@marctc marctc left a comment

Choose a reason for hiding this comment

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

Unless I'm missing something, this should be documented in official OBI docs, like this open-telemetry/opentelemetry.io#9753
devdocs IMHO should be only used to documented stuff that's helpful for the developers of this project.

@NameHaibinZhang
Copy link
Copy Markdown
Member Author

Thanks @marctc — you're right. Looking at the devdocs/README.md ("documentation that is not useful for our users but might be useful for developers") and the pattern in devdocs/metrics.md (which explicitly links to the user-facing page on opentelemetry.io and labels itself a "developer-internal companion"), the attribute-selection.md content is clearly user-facing and belongs in the official OBI docs.

I'll remove the devdocs file from this PR and open a follow-up PR against opentelemetry.io to document the db.response.error trace-selection behavior there. The code changes (removing the placeholder constant, making the opt-in behavior consistent with db.query.text) and the config-schema description updates should still be valid independently.

Let me know if you'd also like the config-schema description change reverted, or if that can stay since it's developer-facing reference.

@marctc
Copy link
Copy Markdown
Contributor

marctc commented May 12, 2026

The code changes (removing the placeholder constant, making the opt-in behavior consistent with db.query.text) and the config-schema description updates should still be valid independently.

sounds good, thanks!

Per reviewer feedback, user-facing documentation should be in
official OBI docs (opentelemetry.io) rather than devdocs/.
devdocs/ is intended for project developer documentation only.

Will follow up with documentation PR to opentelemetry.io.
@NameHaibinZhang
Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Contributor

@marctc marctc left a comment

Choose a reason for hiding this comment

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

I'm happy to unblock the PR but the description and title doesn't match the PR anymore. Would you mind update it?

@NameHaibinZhang NameHaibinZhang changed the title fix(#2076): add document db.response.error trace selection fix(#2076): make db.response.error attribute selection consistent with other optional attributes May 13, 2026
@NameHaibinZhang
Copy link
Copy Markdown
Member Author

NameHaibinZhang commented May 13, 2026

@marctc Done! I've updated the PR title and description to reflect the current scope (code behavior change only, documentation moved to opentelemetry.io in a separate PR). Thanks for flagging this!

@NameHaibinZhang NameHaibinZhang requested a review from marctc May 13, 2026 08:42
Copy link
Copy Markdown
Contributor

@marctc marctc left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM!

@grcevski grcevski merged commit 2991725 into open-telemetry:main May 13, 2026
79 of 80 checks passed
@MrAlias MrAlias added this to the v0.10.0 milestone Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document the functionality of db.response.error

5 participants