feat(core): Add bindScopeToEmitter to bind a scope to an event emitter#21594
Open
mydea wants to merge 5 commits into
Open
feat(core): Add bindScopeToEmitter to bind a scope to an event emitter#21594mydea wants to merge 5 commits into
bindScopeToEmitter to bind a scope to an event emitter#21594mydea wants to merge 5 commits into
Conversation
Adds a new `bindScopeToEmitter(emitter, scope?)` API to core. It captures a scope (defaulting to the current scope) and patches the emitter's listener registration methods so that any listener added afterwards runs with that scope — and therefore the active span — active, even when it fires later in a different async context. This is useful when instrumenting APIs that hand back an event emitter (e.g. a streamed database query) whose `'data'`/`'error'`/`'end'` listeners would otherwise lose the trace context. It mirrors the event-emitter behavior of OpenTelemetry's `ContextManager.bind`, scoped to emitters only. Works with both Node.js `EventEmitter`s (`on`, `addListener`, ...) and DOM `EventTarget`s (`addEventListener`); the isolation scope is intentionally not captured as it is carried along by the active async context. Exported from all SDKs and covered by unit, node-integration and browser-integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mydea
requested review from
JPeer264,
Lms24,
andreiborza,
chargome,
logaretm and
s1gr1d
and removed request for
a team
Contributor
size-limit report 📦
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Keep the API on the npm packages only; CDN bundle size should not grow for a Node-oriented helper. Skip the browser integration test in bundle mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…copeToEmitter A listener can be registered for the same event more than once; each is an independent registration in Node's EventEmitter. Storing a single wrapper per (event, listener) meant later registrations overwrote earlier ones, so only the most recent wrapper could be removed via the original reference and earlier ones were orphaned. Keep a stack of wrappers and remove one per removeListener call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5f01351. Configure here.
Minting a fresh wrapper on every registration broke DOM EventTarget semantics: addEventListener dedupes by (type, callback, capture), so distinct wrapper refs defeated that idempotency (the listener fired once per call) and capture-aware removal could drop the wrong wrapper. Reuse a single stable wrapper per listener and forward the caller's options unchanged: the DOM dedupes correctly and Node's EventEmitter still counts duplicate registrations (removable one-per-call). This also subsumes the earlier per-event wrapper stack. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
Summary
Adds a new
bindScopeToEmitter(emitter, scope?)API to@sentry/core. It captures a scope (defaulting to the current scope) and patches the emitter's listener-registration methods so that any listener added afterwards runs with that scope — and therefore the active span — active, even when it fires later in a different async context.This is useful when instrumenting APIs that hand back an event emitter (e.g. a streamed database query) whose
'data'/'error'/'end'listeners would otherwise lose the trace context. It mirrors the event-emitter behavior of OpenTelemetry'sContextManager.bind, scoped to emitters only (the function-binding path of OTel'sbindis intentionally omitted — at any such call site you already hold the callback and can wrap it withwithScopedirectly).Behavior
EventEmitters (on,addListener,once,prependListener,prependOnceListener) and DOMEventTargets (addEventListener).removeListener/off/removeEventListener) are patched so removals via the original listener reference find the wrapped listener; theaddEventListeneroptionsargument is forwarded; non-function (EventListener-object) listeners pass through untouched.node:eventsimport).Surface
@sentry/core+ node / node-core / browser, and the platform/framework re-exporters).Note: I opted to not include this in CDN bundles right now, as this is really not that critical to browser stuff. We can always add it later if it seems necessary.
Tests
@sentry/core): 19 cases incl. explicit-scope arg and the DOMEventTarget/addEventListenerpath.public-api/bindScopeToEmitter): a listener firing in a fresh async context nests under the bound parent, with an unbound control on a separate trace.tracing/bindScopeToEmitter): a realEventTarget+dispatchEvent, verified on the ESM buildThe mysql instrumentation rewiring that consumes this API is kept on a separate branch and will follow once this lands.
🤖 Generated with Claude Code