feat(node): distributed tracing spans - #4579
Open
turnipdabeets wants to merge 36 commits into
Open
Conversation
Contributor
posthog-node Compliance ReportDate: 2026-09-08 17:41:28 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
23 tasks
Contributor
posthog-js Compliance ReportDate: 2026-09-08 17:38:12 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
Contributor
|
Size Change: +147 kB (+0.71%) Total Size: 20.8 MB 📦 View Changed
ℹ️ View Unchanged
|
This was referenced Aug 20, 2026
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 21, 2026 12:45
a9ff420 to
5c7301c
Compare
23 tasks
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 24, 2026 11:25
e36495f to
9365a6c
Compare
23 tasks
Contributor
|
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
4 times, most recently
from
August 28, 2026 20:49
bfb08e8 to
cc7b6d8
Compare
This was referenced Aug 28, 2026
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
5 times, most recently
from
August 31, 2026 18:23
b946de6 to
ab4048c
Compare
23 tasks
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 31, 2026 18:45
ab4048c to
caf83b0
Compare
turnipdabeets
changed the base branch from
main
to
refactor/shared-otlp-attribute-encoder
August 31, 2026 18:45
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 31, 2026 19:39
caf83b0 to
075298b
Compare
…races-event-attribute-cap # Conflicts: # .changeset/node-span-limits.md
Config surface, limits and the IPostHog note are PR-body and docs material, not release notes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XBUa53f3QqECUyxnkSiWGA
…races-event-attribute-cap # Conflicts: # .changeset/node-distributed-tracing.md
rn-flags-disable-and-update-flags belongs to separate RN work; it was swept back in by a broad `git add`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XBUa53f3QqECUyxnkSiWGA
…races-event-attribute-cap
turnipdabeets
added a commit
that referenced
this pull request
Sep 4, 2026
Traces ships new in the same release through #4579, so naming it here reads as a fix to behavior that never existed. Also drops the split-and-isolate claim from the oversized entry: metrics drops the window rather than halving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XBUa53f3QqECUyxnkSiWGA
The cap is not one of the knobs the traces spec enumerates, so it stays a fixed 128 instead of a public `maxAttributesPerEvent` option. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQy8eimnfV3jVQKbzELc65
jonmcwest
reviewed
Sep 8, 2026
| * The handle to return when a span cannot be recorded: a pass-through when the | ||
| * caller supplied a usable `parent` header, the shared no-op otherwise. | ||
| */ | ||
| export function inertSpan(options?: { parent?: unknown; tracestate?: unknown }): Span { |
Contributor
There was a problem hiding this comment.
Follow-up: preserve the implicit pass-through parent.
A child without an explicit parent ignores an active PassThroughSpan: tracing off loses the context; tracing on can create an unrelated trace.
OTel's no-op rule requires preserving implicit parent context and recommends reusing an already non-recording parent. Reusing the handle on the tracing-on path is a PostHog fallback choice, not that rule's requirement. Cover both paths, including across await. Non-blocking for the MVP; settle before API stability.
Sources: OTel propagation · implementation.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Done in 2581b66ef. Both paths: inertSpan takes the active handle as the implicit parent, and _resolveParent adopts an active PassThroughSpan. Across-await is covered by a node test in c8f9a66ea, where AsyncLocalStorage actually applies. Taken your point that the tracing-on half is our fallback choice rather than the OTel rule.
jonmcwest
reviewed
Sep 8, 2026
| return inertSpan(options) | ||
| } | ||
|
|
||
| const explicitParent = options?.parent |
Contributor
There was a problem hiding this comment.
Correction: foreign OTel parents are optional interoperability work, not a docs bug.
parent accepts this SDK's Span | string. A foreign handle with traceparent() takes the inert path as documented; an OTel spanContext()-only object is outside that type.
I withdraw the documentation-fix request. OTel's tracing API takes a Context parent, not a raw Span, so accepting foreign span objects directly is a PostHog API choice. No merge action needed here.
Sources: current parent contract · OTel span creation · foreign-handle handling.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Thanks, reverted in c8f9a66ea. I had already rewritten the parent docs against your earlier note; they are back to the original wording. I kept one test pinning what an untyped spanContext()-only object does today, since it is cheap coverage either way.
jonmcwest
reviewed
Sep 8, 2026
| return this | ||
| } | ||
|
|
||
| addEvent(name: string, attributes?: SpanAttributes, timestamp?: SpanTimeInput): this { |
Contributor
There was a problem hiding this comment.
Covered by the follow-up; no extra change here.
OTel makes the per-event attribute limit optional, with a default of 128 when provided. This is not a missing mandatory limit.
On this branch, the encoder drops excess event attributes without a per-event dropped count. #4792 adds the cap and counter. Keeping this note to connect the concern to that follow-up. Non-blocking.
Sources: OTel span limits · event handling.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Agreed — landed in #4792 with the cap and the per-event counter.
jonmcwest
reviewed
Sep 8, 2026
| Metrics, | ||
| MetricsConfig, | ||
| } from './metrics/types' | ||
| export { PostHogTraces } from './traces' |
Contributor
There was a problem hiding this comment.
Nit: mark cross-package plumbing as @internal.
Please tag PostHogTraces, SyncSpanContextManager, inertSpan, runWithActiveSpan, resolveTracesConfig, ResolvedTracesConfig, and TraceSdkContext. They are exported from the published core package but are not intended as stable public API.
Use the existing cross-package JSDoc wording. Leave SpanContextManager public because it appears in the protected initialization hook. Non-blocking.
Sources: core exports · existing internal convention.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Done in 40278277f. All seven tagged at the declaration rather than the re-export, SpanContextManager left public. pnpm generate-references is unchanged by it.
jonmcwest
reviewed
Sep 8, 2026
| // Inert like its parent, never an orphan with invented ids — but a | ||
| // pass-through parent's inbound context carries to the child rather than | ||
| // the trace ending here. | ||
| this._logger.debug('Span parent is not a span from this SDK; returning an inert span') |
Contributor
There was a problem hiding this comment.
Optional API convenience: accept a single-element header array.
Node's headersDistinct.traceparent supplies an array, but our public parent type is Span | string. OTel's first-value getter rule concerns carrier extraction; it does not require our parent argument to accept arrays.
Consider unwrapping exactly one element while keeping the multi-element fallback. Also correct the nearby comment: Node's ordinary headers join duplicates into a string. Non-blocking; not an OTel conformance defect.
Sources: Node header arrays · OTel getter contract.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Done in 2581b66ef. Exactly one element is unwrapped, longer arrays are still ignored, and the nearby comment about req.headers is corrected.
jonmcwest
reviewed
Sep 8, 2026
| return undefined | ||
| } | ||
| const trimmed = value.trim() | ||
| if (!trimmed || trimmed.length > TRACESTATE_MAX_LENGTH) { |
Contributor
There was a problem hiding this comment.
Correction: distinguish tracestate size from validity.
W3C recommends propagating at least 512 characters; that is not a validity ceiling. For an otherwise valid header exceeding our size limit, truncate whole entries, preferably removing entries over 128 characters first, then from the right.
An incoming list with more than 32 members violates the grammar, so my earlier request to salvage that case was too broad. Narrow the test change to valid oversized headers. Non-blocking.
Sources: W3C tracestate limits · current validation · W3C member-count grammar.
Agent-assisted review.
Contributor
Author
There was a problem hiding this comment.
Rebuilt to your revised version in c8f9a66ea. My first pass had >32 members trimming to 32 — that is back to a rejection, since it is a grammar violation rather than a size problem. A valid over-long header now drops members above 128 characters first, then from the right. Tests narrowed to valid oversized input.
Contributor
|
Follow-up: preserve array positions for null values. OTel requires preserving null positions in arrays and permits empty-string substitution when the exporter cannot emit null. The current encoder skips null/undefined slots, shifting later elements and misaligning paired arrays. Given the receiver limitation documented here, emit Sources: OTel AnyValue · array encoding. Agent-assisted review. |
Contributor
|
Nit: distinguish send calls from HTTP attempts. The PR body's "exactly 1 request" counts Please say "one send per flush pass" instead. Optionally cover the real transport in the outage test; the current mock hides retries. Non-blocking. Sources: SDK retry defaults · transport retry loop. Agent-assisted review. |
A `beforeSpanSend` hook can write a count past the OTLP field's range onto an event, which is refused for the whole request rather than the one span. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjT26chjUzuPSyJZqh4AWQ
…rent contract A span from another tracer reports through `spanContext()`, so it is ignored rather than yielding an inert span; a test now pins that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjT26chjUzuPSyJZqh4AWQ
23 tasks
An inbound `traceparent` now parents a span that names no parent, on both the recording and inert paths, so a nested span no longer starts a new trace. Also accepts a one-element header array and trims an over-long `tracestate` instead of dropping it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjT26chjUzuPSyJZqh4AWQ
…uded Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjT26chjUzuPSyJZqh4AWQ
More than 32 tracestate members is malformed, not oversized, so it is rejected again; a valid over-long header drops members over 128 characters first, then from the right. Restores the original `parent` docs, whose fix was withdrawn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjT26chjUzuPSyJZqh4AWQ
Contributor
Author
|
@jonmcwest — on the send-calls vs HTTP-attempts nit: PR body updated. It now says "one send per flush pass" and names the transport retry, so the number is not read as an HTTP count. |
Contributor
Author
|
@jonmcwest — on preserving array positions for null values. Not taken in this release, and the reason is cross-SDK rather than cost. The receiver limitation you cite is real and I confirmed it: What stopped me is that this is not a posthog-js choice. All three SDKs drop nullish array elements identically today — The contract is also silent on this, rather than agreeing with either behaviour. Both capabilities say only that "a Suggested order, if you agree: settle it in |
…tracing #4584 folded into this branch, so `beforeSpanSend` and the caps go out in the same minor as `startSpan`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWoFx4BctNmXnpnKS79Qgz
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
5 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
Developers instrumenting a Node service with PostHog have no way to record spans. They can send events, logs and metrics, but nothing that shows where time went in a request or how work fans out across services. Pointing an OpenTelemetry SDK at PostHog works, but it means adding an OTel dependency and wiring the join to PostHog identity by hand — so traces end up disconnected from the person and session they belong to.
Part of a stacked series adding first-class tracing to the JS SDKs (Q3 Goal 4). Scoped to be releasable on its own, one commit, on top of the shared OTLP attribute encoder from #4708 (merged).
Changes
Adds
startSpan,withSpanandgetActiveSpantoposthog-node, behind a newtracesclient option. Spans are encoded as OpenTelemetry-shaped OTLP JSON and POSTed to/i/v1/traces— without an OpenTelemetry dependency.tracesoption means no spans. Every span API still returns a working (inert) handle, so calling code never branches on whether tracing is on.posthogDistinctIdandsessionId, taken fromwithContextor the existing Express/NestJS middleware. This is what makes a trace reachable from a person or session.parentaccepts an inboundtraceparentstring to continue a remote trace;span.traceparent()gives you the header to propagate onward.tracestateis preserved opaquely.awaiton Node, viaAsyncLocalStorageinjected at the Node entrypoint. Core stays runtime-agnostic (nonode:async_hooks), so the edge build and future browser/RN hosts work off the same engine.flush()drains spans, concurrently with the event queue. Serverless handlers callflush(), notshutdown(), so leaving spans on their own timer would silently lose them for the most commonposthog-nodedeployment. A span ending also refreshes thewaitUntilcycle, so a handler that only traces still holds its invocation open.maxLiveSpans(default 10000) caps how many spans may be open at once andmaxSpanAgeMs(default one hour) stops accounting for one that stays open longer, so code that starts spans and never ends them cannot grow the SDK's bookkeeping without limit. At the capstartSpanreturns an inert handle; an evicted span is never exported. Both drops go through the existing span-drop warning.Reviewer notes
awaitthere begin a new trace. Passparentexplicitly to nest them. This is the documented browser limitation too.IPostHoggains three required members (startSpan,withSpan,getActiveSpan). Consumers using it as a type annotation are unaffected; anyone implementing it (hand-written test doubles, DI wrappers) will get a compile error. Same shape asmetricsin feat(metrics): wire posthog.metrics into posthog-node #4117, which shipped under aminorbump._sendOtlpBatch(from fix(core): send logs and metrics through one OTLP batch sender #4623) rather than a third copy of the retry policy — the bearer-auth branch is reintroduced there for traces.flush()andshutdown()as span drains.Live span boundsrequirement is a registry of live span objects, which would keep every leaked handle alive for the length of the age bound. Instead the engine keeps aMapof span id to monotonic start — ids and numbers, never the span — so a handle the caller drops is still collected like any other object, and the count bound can be generous because a slot costs tens of bytes rather than a whole span.packages/core/src/traces/live-spans.spec.tskeeps itsWeakRefprobe, now as the guard that stops a later change from turning this into a registry of spans. Eviction is lazy atstartSpan, and sweeps before reading the bound, so a process that has leaked its way to the cap recovers on the first call after the leaks age out rather than losing tracing for the rest of its life.Retry-Afteris not honoured — the backoff is purely exponential, capped at 30s. This is a shared gap with the logs and metrics senders rather than something this PR introduces, and the service does not emit 429 yet. Tracked separately.beforeSpanSendand the per-span caps ship here. They were written as feat(node): beforeSpanSend hook and per-span limits #4584 and have since been folded into this branch, so this PR's publictracessurface isserviceName,serviceVersion,environment,resourceAttributes,flushIntervalMs,maxExportBatchSize,maxQueueSize,beforeSpanSend,maxAttributesPerSpan,maxEventsPerSpan,maxAttributeValueLength,maxLiveSpansandmaxSpanAgeMs. The per-event attribute cap stays internal.Verification
End-to-end against a real project (381971), twice: first a scripted run, then a real Express app using
setupExpressRequestContextwith two routes, a genuine HTTP hop between them, and an error path. Spans read back out of the product assembled correctly —POST /checkoutroot withdb.queryandhttp.post paymentsbeneath it,GET /payments/chargecontinued across the hop viatraceparent, and a separate trace for the thrown route carryingstatus_code: 2.posthogDistinctId/sessionIdcame from theX-POSTHOG-*request headers on every in-context span, with no manual plumbing.Also exercised against production ingestion: an oversized batch really does 413 (
11.5 MB → 413, halved to5.75 MB → 200twice, all spans queryable afterwards), and 50k spans push through at ~310k/sec with 204 KB average batches — the first real evidence that the 512-span default sits under the body cap. With the endpoint down, 200k spans cost exactly one send per flush pass rather than one per span — the transport still retries within a send, so a pass can make several HTTP attempts.packages/core1305 pass (62 suites),packages/node1000 pass (35 suites) — identical under the edge runtime environment — lint clean, public API references regenerated.Release info Sub-libraries affected
Libraries affected
@posthog/coreis also bumped (minor); it has no checkbox above.Checklist
Backwards compatibility caveat: all runtime surface is additive, but
IPostHoggains three required members — a compile-time break for implementors only, matching themetricsprecedent. Bundle: the traces module is core-resident and the browser is not wired up in this PR.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 the merged
tracescapability spec insdk-specs.Decisions worth flagging for review:
posthog.traces.*namespace — matches how the OTel API reads and keeps the common call short.OtlpSpanKeyValueis an alias of the logs/metricsOtlpKeyValuerather than a separate declaration — one encoder produces all three payloads. The span-flavoured name stays so the span types read as span types.@posthog/typesonly, matching how the logs and metrics wire types are already organised.Put through five rounds of independent fresh-context review, each reviewer required to demonstrate a finding by running it rather than by reading. Bugs they caught, all fixed here and each now pinned by a test that fails when the fix is reverted:
status.messageandtraceStateskipping the sanitiser attributes go through, so one lone surrogate would 400 an entire 512-span batchresourceAttributesrethrowing on every flush, exporting nothing, foreverwaitUntilserverless path having no traces coverage at all, and a span-only handler never registering with itSeveral of those were introduced by earlier rounds' own fixes, which is why the loop ran as long as it did.