fix(traces): keep inbound context and flag a future span start - #4790
Merged
turnipdabeets merged 2 commits intoSep 4, 2026
Conversation
Four conformance gaps an independent review of the traces stack found, each reproduced before it was changed. A `traceparent` above version `00` was rebuilt from the four fields this SDK reads, so a peer that understands the version's extra fields received a header still labelled with it but missing them. It is now forwarded whole. A handle returned while tracing was off carried the inbound trace context, but passing it back as `parent` produced a bare no-op, so everything below it started a fresh trace. The child stays inert, as the spec requires, and now echoes that context. A `startTime` in the future was accepted silently and cost the span its duration; ingestion stores it un-clamped. It now warns, matching how a deep backdate is handled. A batch dropped as non-retriable or too large left the consecutive-failure count standing, which held the depth trigger off and the flush interval at its ceiling until an unrelated send succeeded. A removed batch is progress, so the count clears with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MTHb6UUMhVJSWC2cWBzSxc
Contributor
Prompt To Fix All With AI### Issue 1
packages/core/src/traces/sanitize.ts:81
**Overstated duration warning**
The warning says every future `startTime` will produce zero duration. However, if the span remains open until the wall clock passes that start time, it exports a positive duration. Using “may export” would keep the diagnostic accurate for slightly future-dated starts.
```suggestion
logger?.debug('Span startTime is in the future; the span may export with a zero duration')
```
### Issue 2
.changeset/traces-context-and-time-fidelity.md:5
**Incomplete release notes**
The changeset covers traceparent forwarding, inert-parent propagation, and future-start warnings, but omits the exporter backoff recovery change. This violates the repository requirement that the change express every idea that needs to be expressed, so the release notes must include the fatal and oversized-batch counter resets before merging.
```suggestion
Forward an inbound `traceparent` whose version is above `00` whole, including the fields that version adds, rather than trimming it to the four this SDK reads. Keep the inbound context when a handle returned with tracing off is passed back as `parent`, so a child no longer starts a fresh trace. Warn when a span's `startTime` is in the future, which costs it its duration. Restore normal flush scheduling after fatally rejected or individually oversized batches are dropped.
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(traces): keep inbound context and fl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Contributor
|
Size Change: 0 B Total Size: 20.9 MB ℹ️ View Unchanged
|
dustinbyrne
approved these changes
Sep 4, 2026
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
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
An independent review of the traces stack found four conformance gaps. All four are in #4579's surface, all four reproduce, and none are regressions from #4584 — they were simply never exercised. Each is small on its own; together they are the difference between a distributed trace surviving a hop and quietly splitting in two.
A
traceparentabove version00lost the fields that version defines.normalizeTraceparentrebuilt the header from the four fields this SDK reads, so02-<id>-<id>-01-extrawas forwarded as02-<id>-<id>-01. That is the one clearly wrong shape: forwarding it verbatim is correct pass-through, and relabelling it00-is correct mutation, but a header still labelled02with its02-defined fields removed is one a v02-aware peer must treat as malformed. #4776 tightened the neighbouring cases — trailing fields on00, uppercase ids — and sharpened this one: the regex now captures the trailing group, uses it to reject00, then discards it.A pass-through handle used as a
parentsevered the trace. With tracing off,startSpan('x', { parent: req.headers.traceparent })returns a handle that carries the inbound context. Passing that handle back asparentproduced a bareNOOP_SPAN, whosetraceparent()isnull— so everything below it started a fresh trace, on the exact path the pass-through handle exists to keep whole.A
startTimein the future was accepted silently. The option is documented as "Backdate the span's start", and the SDK already warns when a backdate is deep enough for the server to clamp it. The future direction had no guard at all, and it costs the span its duration: a backdated span reads the wall clock atend(), so an end before the start clamps to zero. Verified against production ingestion — the server stores the future timestamp un-clamped, so the span lands an hour ahead of its trace withduration_nano: 0.A dropped batch left the export loop backed off.
_consecutiveFlushFailuresgates the queue-depth flush trigger and drives the backoff, and only the success path cleared it. After a 503 burst, a non-retriable400or atoo-largedrop cleared_headBatchFailuresbut not this one, so a healthy endpoint kept the depth trigger disabled and the interval pinned at its 30s ceiling until an unrelated send happened to succeed.Changes
normalizeTraceparentreturns the header as received rather than rebuilding it. Its only caller is the pass-through handle, whose job is to echo.inertSpanresolves a handle parent, readingtraceparent()/tracestate()behind a guard. Centralised there rather than instartSpan, so it covers all three inert paths — opted out, disabled, and the live-span bound — not just the one the finding named.resolveStartTimewarns on a future start, matching the deep-backdate rule.fatalandtoo-largedrop paths clear_consecutiveFlushFailures, as the retry-exhaustion path already did.Two places the review's suggested remedy was wrong
Worth flagging, because both would have been changes for the worse:
traceparent()and recording a real child. The spec says the opposite: "A child started with a pass-through handle asparentSHALL itself be inert." The child here stays inert and records nothing — it just stops throwing the context away. There is a test pinning that it enqueues nothing even when the instance is recording.nowfor a futurestartTime. The spec requires only range validity, and explicitly contemplates "a future-dated leaked span" when it specifies the live-span age bound against the monotonic clock. Silently rewriting a caller's value would also diverge from every other port. Deep backdating is aSHOULDwarn, not a clamp, so this warns.Verification
Each fix is pinned by a test that fails when it is reverted — checked one at a time (1 test, 2, 1, 2 respectively).
packages/core1402 pass,packages/node988 pass, oxlint and oxfmt clean.Run as a real Express service against production ingestion (project 381971), spans read back out of the product rather than off the wire — 11/11:
02-…-01-extraparent forwarded whole,tracestateintact4BF92F3577B34DA6A3CE929D0E0E4736, parented to the inbound span id17:11against its sibling's16:11withduration_nano: 0The failure-counter fix is covered by unit tests rather than the live run: reproducing it needs an injected
503followed by a400, which is not something to provoke against production ingestion.Release info Sub-libraries affected
Libraries affected
@posthog/coreonly; it has no checkbox above. All four sites are inside the traces feature #4579 introduces, which has not shipped, so nothing released changes behaviour.Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, directed by @turnipdabeets. The four findings came from a review pass over the #4584 stack; each was reproduced before being changed, and the spec was consulted before fixing, which is what caught the two suggested remedies that would have contradicted it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MTHb6UUMhVJSWC2cWBzSxc