Skip to content

fix(api): align tracestate length-truncation with @opentelemetry/core - #7073

Open
zigzagdev wants to merge 5 commits into
open-telemetry:mainfrom
zigzagdev:fix/tracestate-length-truncation
Open

fix(api): align tracestate length-truncation with @opentelemetry/core#7073
zigzagdev wants to merge 5 commits into
open-telemetry:mainfrom
zigzagdev:fix/tracestate-length-truncation

Conversation

@zigzagdev

@zigzagdev zigzagdev commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Which problem is this PR solving?

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

TraceStateImpl (in @opentelemetry/api, api/src/trace/internal/tracestate-impl.ts) dropped the entire tracestate whenever the raw header string exceeded 512 characters (if (rawTraceState.length > MAX_TRACE_STATE_LEN) return;). This diverges from @opentelemetry/core's TraceState (packages/opentelemetry-core/src/trace/TraceState.ts), which instead accumulates valid members from the front (most recently updated first) and only skips the individual member that would push the running length over 512, continuing to check the remaining, potentially shorter, members.
Because createTraceState (api, deprecated) and TraceState (core, used in practice) are meant to have equivalent parsing semantics, so this inconsistency meant the same raw tracestate header could be parsed completely differently depending on which implementation handled it.

Fixes # (issue)

Short description of the changes

  • Rewrote TraceStateImpl._parse() to accumulate members left-to-right with a running length budget, matching @opentelemetry/core's TraceState._getState() behavior: a member that would exceed the 512-character limit is skipped, but subsequent (shorter) members are still evaluated instead of discarding the whole state.
  • Item-count truncation (max 32 members) is now handled in the same loop via early break, replacing the previous separate post-processing step.
  • Added a test mirroring @opentelemetry/core's equivalent case: a state with an oversized member followed by a short one keeps the short member instead of dropping everything.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Ran api/test/common/trace/tracestate.test.ts (transpile-only mocha run) — all 13 tests pass, including the new test.
  • Ran eslint on the changed files — no issues.
  • Ran tsc --noEmit — only pre-existing, unrelated errors from a missing generated src/version.ts file (not touched by this change).
  • Test A: npx mocha 'test/common/trace/tracestate.test.ts' in api/

Checklist:

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

@zigzagdev
fix(api): align tracestate length-truncation with @opentelemetry/core
a20d6cc 
TraceStateImpl previously dropped the entire tracestate whenever the raw
header exceeded 512 characters. @opentelemetry/core's TraceState instead
accumulates valid members left-to-right and skips only the members that
would push the running length over the limit, keeping shorter members
that still fit. Port the same accumulation logic so the two
implementations behave consistently.
@zigzagdev
test(api): verify tracestate keeps members that fit after a long memb…
5c23ad0 
…er is skipped

Mirrors the equivalent @opentelemetry/core test to cover the
accumulation-based length truncation.
@zigzagdev
zigzagdev requested a review from a team as a code owner September 6, 2026 07:50
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Welcome, contributor! Thank you for your contribution to opentelemetry-js.

Important reminders:

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-10-18 01:28 UTC

Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):

  • Top-level threads: 1
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.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

@opentelemetry-pr-dashboard opentelemetry-pr-dashboard Bot mentioned this pull request Sep 6, 2026
Open
@david-luna

david-luna commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

The tracestate class from the API was deprecated in #6459
There was no modification of its behaviour as it could be considered a breaking change.

@open-telemetry/javascript-approvers should we revisit this?

@pichlermarc pichlermarc self-assigned this Sep 23, 2026
pichlermarc
pichlermarc reviewed Sep 24, 2026
View reviewed changes

@pichlermarc pichlermarc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @zigzagdev - I'm open to aligning both implementations, but I think we should copy the TraceState impl from core verbatim where possible to avoid differences in behavior.

suggestion: copy TraceState from @opentelemetry/core, adjust class name and formatting where needed (you may also need to adjust tests)

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 1, 2026

Copy link
Copy Markdown

Hi @zigzagdev — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.

  • Replying is enough to hand it off — answer, explain why no change is needed, or ask a follow-up. The dashboard routes it onward once nothing on the list is waiting on you.
  • To hand it back for any other reason, including the dashboard getting this wrong, comment /dashboard route:reviewers.

@zigzagdev
fix(api): enforce max tracestate length on set/unset and defer parsing
3b8494e 
Parsing is now deferred until the state is actually read, and the
tracked length is updated incrementally on set/unset so mutations
that would exceed the W3C 512-char limit are rejected without
re-serializing the whole state.
@zigzagdev
test(api): cover tracestate set/unset length limits
c898bff 
Add cases for set()/unset() returning the same instance when a
mutation would be a no-op or exceed the max length, and update the
parse tests to match the new skip/truncate behavior.
@zigzagdev
Merge branch 'main' into fix/tracestate-length-truncation
d57f07f
@zigzagdev

zigzagdev commented Oct 18, 2026

Copy link
Copy Markdown
Author

@pichlermarc

I'll copy TraceState from @opentelemetry/core verbatim (renamed to TraceStateImpl) and update the tests accordingly.

@zigzagdev
zigzagdev requested a review from pichlermarc October 18, 2026 01:28
@zigzagdev zigzagdev mentioned this pull request Oct 23, 2026
Open
6 tasks done

This branch has not been deployed

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

Reviewers

@pichlermarc pichlermarc Awaiting requested review from pichlermarc

At least 1 approving review is required to merge this pull request.

Assignees

@pichlermarc pichlermarc

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zigzagdev @david-luna @pichlermarc