fix(otel): tighten OTel-bridge Span spec compliance#8242
Draft
fix(otel): tighten OTel-bridge Span spec compliance#8242
Conversation
Pulls the OTel-to-Datadog translation logic out of `span.js` and `tracer.js` into a single `span-helpers.js` module exposing: `normalizeLinkContext`, `setOtelAttribute`, `setOtelAttributes`, `addOtelLink`, `recordException`, `setStatus`. No behavior change. Consolidating these small helpers in one place keeps the OTel-bridge translation layer easy to reason about and lets future bridge objects reuse the same logic without duplication.
The OTel-bridge `Span` class drifted from the JS SDK in three places that showed up as observable differences for OTel consumers. This commit fixes all three behind a single set of helpers, with focused unit coverage. 1. Mutations after `end()` were not consistently a no-op. `setAttribute`/`addEvent`/`addLink`/`recordException`/`setStatus` could still poke the underlying DD span after `end()` returned, contradicting the OTel spec which requires a finished span to be immutable. The writable-span gate (`_duration === undefined`) is now centralised in `span-helpers.js` so every helper short-circuits at the same point. 2. `setStatus` did not implement the OTel precedence rules. The spec says `OK` is final, `ERROR > UNSET`, and a later `UNSET` must never overwrite. The bridge tracked a boolean `_hasStatus`, so a second `OK` (after `ERROR`) silently kept the error and a second `ERROR` could not refresh the message. Replaced with a `#statusCode` private field and an `applyOtelStatus(currentCode, status) -> newCode` helper that encodes the precedence table once. 3. `addEvent`'s overloads were not normalised. `addEvent(name, hrTime)` was treated as attributes, so the timestamp ended up tagged on the event instead of as its time. `addOtelEvent` now detects `TimeInput` shapes and routes them to the right slot. The helpers also gained `addOtelLinks` so the bridge does not loop in the class layer, and `setOtelAttributes` no longer mutates its `attributes` input as a drive-by while the gate was being added. Test-only: new `packages/dd-trace/test/opentelemetry/span-helpers.spec.js` covers each helper in isolation (gate, precedence, normalisation, error tagging, `normalizeLinkContext` shape coverage). Existing `span.spec.js` gained the precedence describe block, post-end no-op coverage across all mutation methods, and the `addEvent` time-input regression test. No production behaviour change for code paths that were already spec- compliant; the gate and precedence fixes are observable only on paths that misused the bridge.
Contributor
Overall package sizeSelf size: 5.69 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
BridgeAR
changed the title
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: c9981b1 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-05-02 12:43:53 Comparing candidate commit c9981b1 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1346 metrics, 98 unstable metrics. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The OTel-bridge
Spanclass drifted from the JS SDK in three places thatshowed up as observable differences for OTel consumers. This commit fixes
all three behind a single set of helpers, with focused unit coverage.
Mutations after
end()were not consistently a no-op.setAttribute/addEvent/addLink/recordException/setStatuscouldstill poke the underlying DD span after
end()returned, contradictingthe OTel spec which requires a finished span to be immutable. The
writable-span gate (
_duration === undefined) is now centralised inspan-helpers.jsso every helper short-circuits at the same point.setStatusdid not implement the OTel precedence rules.The spec says
OKis final,ERROR > UNSET, and a laterUNSETmustnever overwrite. The bridge tracked a boolean
_hasStatus, so a secondOK(afterERROR) silently kept the error and a secondERRORcouldnot refresh the message. Replaced with a
#statusCodeprivate fieldand an
applyOtelStatus(currentCode, status) -> newCodehelper thatencodes the precedence table once.
addEvent's overloads were not normalised.addEvent(name, hrTime)was treated as attributes, so the timestampended up tagged on the event instead of as its time.
addOtelEventnow detects
TimeInputshapes and routes them to the right slot.The helpers also gained
addOtelLinksso the bridge does not loop in theclass layer, and
setOtelAttributesno longer mutates itsattributesinput as a drive-by while the gate was being added.
Test-only: new
packages/dd-trace/test/opentelemetry/span-helpers.spec.jscovers each helper in isolation (gate, precedence, normalisation, error
tagging,
normalizeLinkContextshape coverage). Existingspan.spec.jsgained the precedence describe block, post-end no-op coverage across all
mutation methods, and the
addEventtime-input regression test.No production behaviour change for code paths that were already spec-
compliant; the gate and precedence fixes are observable only on paths
that misused the bridge.