feat(traces): propagate the inbound sampled flag and parent remoteness - #4728
Merged
Merged
Conversation
Contributor
Contributor
|
Size Change: +2.41 kB (+0.01%) Total Size: 20.9 MB 📦 View Changed
ℹ️ View Unchanged
|
A continued trace now carries the caller's trace-flags byte in both `traceparent()` and the exported span, rather than always sending `01`, so a downstream parent-based sampler is not handed a decision this SDK invented. Bits version 00 does not define are zeroed. Exported spans also set OTel's parent-remoteness bits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MmYSjZ7Mr2UZp5acLDuEHe
turnipdabeets
force-pushed
the
feat/traces-flags-fidelity
branch
from
September 2, 2026 14:36
66ec189 to
8ec489e
Compare
turnipdabeets
marked this pull request as ready for review
September 2, 2026 21:08
Contributor
Prompt To Fix All With AI### Issue 1
packages/core/src/traces/otlp.ts:45
**Root remoteness is misclassified**
For a root span with no `parentSpanId`, `spanFlags` still sets `SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE`, encoding `0x101` and incorrectly asserting a known-local parent instead of leaving the parent-remoteness bits unset.
```suggestion
return record.parentSpanId
? w3c | SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE | (record.parentIsRemote ? SPAN_FLAGS_CONTEXT_IS_REMOTE : 0)
: w3c
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(traces): propagate the inbound samp..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Member
|
i merged the vitest migration, no more jest, do we need to change anything here? |
…into tmp/4728-work
Greptile read the known bit on a root span as asserting a local parent; the OTel Go and Java exporters set it there too. Names the existing test for what it covers and adds the local-parent case.
Contributor
Author
|
Yes — handled, in the base PR #4579 and merged up into this one. #4579 had the merge conflict with the migration, so the port lives there: the traces suites moved to One thing worth knowing beyond a rename: vitest's default Green on both branches: core 1360 pass / 63 suites, node 1017 pass / 36 suites, identical under |
marandaneto
approved these changes
Sep 3, 2026
turnipdabeets
added a commit
that referenced
this pull request
Sep 3, 2026
Resolve span.ts end(): keep the per-span-limit record shape and add the traceFlags and parentIsRemote fields from #4728. Carry both through beforeSpanSend, which the public record does not expose.
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
2 participants
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.
Problem
Two gaps in what the trace-flags byte says, both raised by @jonmcwest reviewing #4579.
The inbound sampled flag is dropped and
01is always sent.formatTraceparenthardcoded-01andparseTraceparentdiscarded the inbound flags entirely. Legal under W3C — "update sampled" is a permitted mutation when parent-id changes — but it breaks interop with head-sampled fleets. Service A samples a trace out (00); a PostHog-traced service B continues it and propagates01; a downstream OTel service C with the defaultParentBasedsampler then records a trace its own head sampler had already rejected, and its backend pays for fragments with no root.Parent remoteness is never expressed.
flagswas the constant1, so OTel's bits 8-9 stayed unset and every span reads as "parent remoteness unknown". This SDK always knows the answer: atraceparent-string parent is remote, a handle parent is local.Changes
The flags byte is carried, not overridden.
parseTraceparentreturns the inboundflags, a span keeps it,traceparent()propagates it, andchildContext()hands it down so the whole local chain agrees. A trace started here is still01, because it is recorded.Spans are still recorded and exported when the caller sampled the trace out — this is OTel's
RECORD_ONLYshape, not a sampling decision. What changes is only what the header and the wire say.flagsnow carries the remoteness bits.0x100(remoteness known) is always set,0x200added when the parent arrived as a header. So a service-entry span reads0x301and a local child0x101. The W3C byte comes from the span's own flags, defaulting to sampled if a hostile value ever got that far.SpanRecordgainstraceFlagsandparentIsRemote— additive, and the wire type'sflagsdoc updated to match.Reviewer notes
flags=0the same as any other value — @jonmcwest asked this be confirmed before propagating a sampled-out flag, and it checks out in the monorepo (c281827b26e):rust/capture-logs/src/trace_record.rscopiesflags: span.flags as i32onto the row with no branch, it is stored as aUInt32column (posthog/clickhouse/traces/spans.py), and the only span-dropping logic —check_restrictionsinrust/capture/src/otel/filtering.rs— keys on event name and distinct id, never on flags. No query inproducts/tracing/reads the column.Verification
packages/core1344 pass (62 suites),packages/node1004 pass (35 suites), identical under the edge runtime environment, lint clean, public API references regenerated (no diff — the wire type is not in them).New tests: an inbound
00propagates as00through the span and its children and exports0x300; a header parent exports0x301while its local child exports0x101; an unusable flags byte falls back to sampled; and the node client hands the next service the flag the caller sent.Release info Sub-libraries affected
Libraries affected
@posthog/coreis also bumped (minor); it has no checkbox above.Checklist
Ships alongside the unreleased #4579, so no released behaviour changes.
SpanRecordgains two required fields; it is constructed by the SDK, not by callers.If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, directed by @turnipdabeets, from review feedback on #4579.
Kept separate from #4579 rather than folded into it, so the sampling-semantics change gets its own review — it is the one change in this stack that alters what other vendors' samplers see.