Skip to content

feat: return a promise from flush() - #32

Merged
bengl merged 1 commit into
nodejs:mainfrom
bengl:bengl/awaitable-flush
Nov 3, 2026
Merged

bengl merged 1 commit into
nodejs:mainfrom
bengl:bengl/awaitable-flush

Conversation

@bengl

@bengl bengl commented Nov 2, 2026

Copy link
Copy Markdown
Member

flush() used to start an export and return nothing, so there was no way to wait for spans to reach the collector. Explicit exits skip the beforeExit flush, and so does a serverless platform that freezes the process after each invocation. Now flush() returns a promise:

process.on('SIGTERM', async () => {
  await otel.flush();
  process.exit(0);
});

What the promise covers. It resolves once the spans flush() took from the buffer, and every export already in flight, are done, whether they reached the collector or not. A flush() called while an export is in flight waits for that export too, so awaiting the latest call is enough. Spans that end after the call belong to a later flush.

It never rejects. Failed exports are already reported as OTelExportWarning warnings. A rejection would be unhandled in every caller that ignores the result, including microtel's own timer, buffer-full, and beforeExit flushes.

How an export settles. Each export gets a promise, kept in a set while its request is open. It settles on whichever comes first: the response ending, a response or request error, or the exportTimeout deadline, which destroys the request. An export that throws while sending destroys its request, so neither its socket nor its deadline outlives it.

Using it.

  • benchmark/export.bench.js awaits its flushes instead of counting requests at the collector, and drops its watchdog. A short exportTimeout keeps a stalled export from hanging a sample. Each sample now ends when the last response does, rather than when the collector finishes reading the last request, so its numbers are a little lower than main's.
  • Several tests waited a fixed delay(100) after flush() to see whether an export happened. They now await flush() itself.
  • The README documents the promise, shows a shutdown handler, and updates the note about explicit exits.

Not taken.

  • Resolving with counts of what was sent or dropped. It can be added later without breaking anything.
  • A timeout option for flush(). exportTimeout already bounds every export.

Tests. test/test-flush-promise.js covers:

  • the batch already being at the collector when the promise resolves
  • resolving right away with nothing to export, and before init()
  • resolving, not rejecting, when the collector is down, never answers, or resets mid-response
  • a second flush() waiting for an export still in flight
  • a send that throws, where the child must exit well before its 60 second exportTimeout

Each of these breaks fails the file: not waiting for exports in flight, settling by rejecting, resolving before the export ends, a response or request error that never settles, and a failed send that leaves its request open.

flush() started an export and returned nothing, so code had no way to
wait for its spans to be sent. Shutdown handlers and serverless
functions need to, since explicit exits and frozen processes skip the
beforeExit flush.

flush() now returns a promise that resolves once the spans it took
from the buffer and every export already in flight are done. It never
rejects. Failed exports are already reported as OTelExportWarning
warnings, and a rejection would be unhandled in every caller that
ignores the result. An export that fails to send destroys its
request, so neither its socket nor its deadline outlives it.

The export benchmark awaits its flushes instead of counting requests
at the collector, and several tests await flush() instead of waiting
a fixed delay for an export that might not come.

Signed-off-by: Bryan English <bryan@bryanenglish.com>
@bengl
bengl requested a review from a team November 2, 2026 21:40
@bengl
bengl marked this pull request as ready for review November 2, 2026 21:54

@AbhiPrasad AbhiPrasad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do it async should we let folks pass in a AbortController?

also maybe we should have a max timeout (allow for it to be configured?)

@bengl

bengl commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@AbhiPrasad wanna do those in a separate PR?

@AbhiPrasad AbhiPrasad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate pr sgtm

@bengl
bengl merged commit 7b57a03 into nodejs:main Nov 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants