feat(traces): propagate the inbound sampled flag and parent remoteness - #4728
Draft
turnipdabeets wants to merge 4 commits into
Draft
feat(traces): propagate the inbound sampled flag and parent remoteness#4728turnipdabeets wants to merge 4 commits into
turnipdabeets wants to merge 4 commits into
Conversation
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. Exported spans also set OTel's parent-remoteness bits.
Contributor
Contributor
|
Size Change: +2.41 kB (+0.01%) Total Size: 20.9 MB 📦 View Changed
ℹ️ View Unchanged
|
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.
Problem
Stacked on #4579 — review that first.
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.