Skip to content

fix(span): enforce StatusCode precedence rules in setStatus per specification#6461

Merged
trentm merged 7 commits intoopen-telemetry:mainfrom
newbee1939:fix/setstatus-spec-compliance
Mar 5, 2026
Merged

fix(span): enforce StatusCode precedence rules in setStatus per specification#6461
trentm merged 7 commits intoopen-telemetry:mainfrom
newbee1939:fix/setstatus-spec-compliance

Conversation

@newbee1939
Copy link
Copy Markdown
Contributor

@newbee1939 newbee1939 commented Mar 3, 2026

Which problem is this PR solving?

SpanImpl#setStatus() does not enforce StatusCode precedence rules defined in the Trace API spec — Set Status.

Specifically:

  1. message is not ignored for Ok / Unset — Spec requires: "Description MUST be IGNORED for StatusCode Ok & Unset values."
  2. Total order (Ok > Error > Unset) is not enforced — Once Ok is set, Error or Unset can still overwrite it. Unset is not ignored.

Fixes #6455

Short description of the changes

  • Span.ts: Rewrite setStatus() to enforce spec-defined precedence (Ok > Error > Unset). Unset is always ignored, Ok is final, and message is only kept for Error.
  • span.ts (API): Update JSDoc to document the status precedence rules and message behavior.
  • Span.test.ts: Add comprehensive tests for all status transitions and consolidate existing setStatus tests into a dedicated describe('setStatus') block.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit tests covering all status transition rules (Ok > Error > Unset)
  • Unit tests for message handling (ignored for Ok/Unset, kept for Error, non-string dropped with warning)
  • Unit test for immutability after span.end()

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

…fication

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
@newbee1939 newbee1939 changed the title fix(span): enforce StatusCode precedence rules in setStatus per specification [WIP] fix(span): enforce StatusCode precedence rules in setStatus per specification Mar 3, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.70%. Comparing base (30f94fe) to head (18536bd).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6461   +/-   ##
=======================================
  Coverage   95.70%   95.70%           
=======================================
  Files         364      364           
  Lines       11772    11777    +5     
  Branches     2743     2747    +4     
=======================================
+ Hits        11266    11271    +5     
  Misses        506      506           
Files with missing lines Coverage Δ
packages/opentelemetry-sdk-trace-base/src/Span.ts 97.83% <100.00%> (+0.06%) ⬆️
🚀 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.

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
…tatusCode precedence rules in setStatus

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
…tatusCode precedence

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
…nce rules

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
…nd precedence rules

Signed-off-by: h6o <48004352+newbee1939@users.noreply.github.com>
@newbee1939 newbee1939 changed the title [WIP] fix(span): enforce StatusCode precedence rules in setStatus per specification fix(span): enforce StatusCode precedence rules in setStatus per specification Mar 3, 2026
@newbee1939 newbee1939 marked this pull request as ready for review March 3, 2026 13:52
@newbee1939 newbee1939 requested review from a team as code owners March 3, 2026 13:52
Copy link
Copy Markdown
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!
I'll leave it for a day or two in case others might want to review it as well.

Copy link
Copy Markdown
Member

@raphael-theriault-swi raphael-theriault-swi left a comment

Choose a reason for hiding this comment

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

LGTM too !

@trentm trentm added this pull request to the merge queue Mar 5, 2026
Merged via the queue into open-telemetry:main with commit 356ddee Mar 5, 2026
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot bot commented Mar 5, 2026

Thank you for your contribution @newbee1939! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

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.

Span#setStatus() does not enforce StatusCode precedence rules per specification

3 participants