feat(node): flush() drains queued spans - #4580
Draft
turnipdabeets wants to merge 3 commits into
Draft
Conversation
23 tasks
Contributor
Contributor
|
Size Change: +500 B (0%) Total Size: 20.2 MB 📦 View Changed
ℹ️ View Unchanged
|
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 21, 2026 12:45
a9ff420 to
5c7301c
Compare
turnipdabeets
force-pushed
the
feat/traces-node-flush-contract
branch
from
August 21, 2026 12:45
a745df5 to
a4dff03
Compare
A serverless handler calls flush() rather than shutdown(), so ended spans were left queued until the container was reused.
turnipdabeets
force-pushed
the
feat/traces-node-mvp
branch
from
August 24, 2026 11:25
e36495f to
9365a6c
Compare
turnipdabeets
force-pushed
the
feat/traces-node-flush-contract
branch
from
August 24, 2026 11:25
a4dff03 to
2e9bfa0
Compare
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.
In a serverless handler you call
flush(), notshutdown(): the container is reused across invocations, so tearing the client down would throw away the connection pool and the flag cache. Butflush()drained only the event queue, so ended spans sat in the span queue until the flush timer happened to fire or the container was frozen.That makes the most common
posthog-nodedeployment shape the one that silently loses spans. Ending a span and callingflush()looks like it worked.Changes
flush()now drains the span queue alongside the event queue.flush()is a method callers already treat as safe, and adding a rejection path to it would be a worse regression than the one being fixed.waitUntilpath too, not just direct calls, so it holds on Vercel/Cloudflare-style runtimes.tracesis unconfigured:flush()returns the event-queue promise unchanged.This is a contract change to an existing shipped method, which is why it is its own PR with its own changeset and release note rather than being folded into #4579.
How much later does
flush()resolve? Nothing changes withouttracesconfigured. With it, an empty span queue returns immediately, and otherwise it's one POST overlapping the events POST — so usually no measurable difference. A slow endpoint is bounded the same way events already are (10s timeout, 3 retries per batch).Reviewer notes
flushes on its own interval, not with the events pipeline) encoded the contract being changed here, so it was rewritten rather than deleted — the "own interval" half is still asserted separately.shutdown()is unchanged; it already drained spans.Release info Sub-libraries affected
Libraries affected
Checklist
Deliberately unchecked: this changes the timing contract of
flush()for anyone withtracesconfigured — it resolves later than before. No signature or type changes, and no effect when tracing is off.If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, directed by @turnipdabeets.
This started as PR 7 in the tracing plan. While reviewing #4579's description, @turnipdabeets questioned why it sat that late in the stack; the deferral rationale ("the only outright contract change") justified giving it its own PR but not its position behind five others, given the failure mode is silent data loss for the most common serverless shape. Re-ordered to stack directly on the MVP.
The one design call worth review: swallowing span-export failures instead of surfacing them. The alternative — letting
flush()reject — matches how event flushing behaves, but adds a new rejection path to a method most callers do not wrap in atry. Chose the conservative option, matching whatshutdown()already does.