Repository navigation
Conversation
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>
Previously, a caller could not bound how long flush() waited. Now
flush({ signal }) takes an AbortSignal, such as AbortSignal.timeout(ms),
the way core APIs take one, and rejects with an AbortError shaped like
core's when it aborts. Export failures still resolve the promise.
Aborting stops the waiting, not the export, which keeps running,
bounded by exportTimeout. With a signal that has already aborted,
flush() rejects right away and starts no export, so the spans stay
buffered for the next flush. The abort listener runs even when an
earlier listener stops the event's propagation, as core's do.
validateAbortSignal is ported from core into lib/validate.js, with
parity tests.
Signed-off-by: Bryan English <bryan@bryanenglish.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
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.
Builds on #32, so its first commit is #32's. The second commit is this change.
What changed.
flush({ signal })takes anAbortSignal, such asAbortSignal.timeout(ms), the way Node core APIs take one. When it aborts, the promise rejects with anAbortErrorshaped like core's: codeABORT_ERR, messageThe operation was aborted, and the signal's reason as itscause. Core builds that error from an internal class, so microtel builds the same shape. Export failures still resolve the promise, so only an abort the caller asked for rejects.Aborting stops the waiting, not the export. The export keeps running, bounded by
exportTimeout, and its spans still reach the collector if it succeeds. With a signal that has already aborted, the promise rejects right away and no export starts, so the spans stay buffered for the next flush. The listener goes on throughaddAbortListener(), so it runs even when an earlier listener stops the event's propagation, as core's own abort listeners do. It comes off once the flush settles, so one signal can serve many flushes.Validation. Options must be an object and
signalanAbortSignal, checked the way core checks them.validateAbortSignalis ported from core intolib/validate.js, with parity tests against core's own. ThebeforeExithandler now callsflush()with no arguments, since Node passes it the exit code.Not taken. A separate
timeoutoption.AbortSignal.timeout()already covers it.Tests. In
test/test-flush-promise.js:ERR_INVALID_ARG_TYPEAbortSignal.timeout()rejects with itsTimeoutErroras the causeflush()settles