fix(deno): Gate tracing integrations on hasSpansEnabled - #24070
Merged
Conversation
Node and Bun both skip getTracingIntegrations() when tracing is disabled; the Deno SDK spread them unconditionally, so 22 tracing integrations were added and eagerly subscribed with tracing off. init() also no longer writes the computed default set back onto the caller's options object: the set is now tracing-dependent, so caching it there would pin the first init's result for a reused object. Local computation matches how the Node SDK handles the same step. Test updates: default-integration assertions across the Deno suites now init with tracing enabled, since that is the condition under which tracing integrations are defaults at all; mod.test.ts snapshots lose the 22 tracing names (the bug made visible in the event payload); new tests pin both directions of the gate, the public getDefaultIntegrations() behaviour, and the no-mutation contract. Fixes getsentry#23893
JosephDoUrden
requested review from
isaacs and
mydea
and removed request for
a team
September 4, 2026 09:32
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
# Conflicts: # packages/deno/test/__snapshots__/mod.test.ts.snap
… the snapshot ClientOptions no longer carries defaultIntegrations on develop, so the object literal failed the type check after the merge. The local still feeds getIntegrationsToSetup. The snapshot loses the tracing integrations, which is what the gate is for.
Contributor
isaacs
requested changes
Sep 9, 2026
Member
There was a problem hiding this comment.
Thanks for submitting this!
There are a few changes needed to get this landed, and a rebase onto (or merge from) origin/develop as well.
I'm happy to take that on if you don't get to it in the next few days (commit and changelog will credit you regardless :)
--
UPDATE: I see that you actually landed the fix for the only blocking issue already, so I'll get this landed asap.
isaacs
self-requested a review
September 9, 2026 15:31
Co-authored-by: isaacs <i@izs.me>
isaacs
approved these changes
Sep 9, 2026
isaacs
enabled auto-merge (squash)
September 9, 2026 15:33
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
2 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.
yarn lint) & (yarn test).Gates getTracingIntegrations() on hasSpansEnabled(options), same as Node (sdk/index.ts:92) and Bun already do.
On the confirm-first question in the issue: I checked all 22 and the honest answer is five of them do capture errors, firebase (server-utils/src/integrations/firebase/functions.ts:92), vercel-ai (vercel-ai-dc-subscriber.ts:680), and the anthropic, openai and google-genai streaming paths (ai/anthropic-ai/utils.ts:58, ai/openai/streaming.ts:138, ai/google-genai/streaming.ts:39). All five capture inside their span wrappers though, error capture in addition to tracing, which is literally the getErrorIntegrations criterion, so none of them is error capture only and nothing needs to move. Arguably they are misfiled upstream, but Node and Bun already apply this exact gate, so Deno moves onto the shared behaviour rather than away from it, and re-filing would be an upstream-wide call, not something for this PR.
Two things in the diff beyond the gate. getDefaultIntegrations is exported, so getDefaultIntegrations({}) now returns no tracing integrations whatever the init options say, same footgun Node and Bun already have, added a test pinning both directions. And init() no longer writes the computed default set back onto the caller's options object, the set is tracing dependent now so that cache goes stale if the object is reused, Node computes into a local the same way. Test pins the no-mutation contract too.
The snapshot updates are the bug made visible btw, the sdk.integrations list in the event payload drops 22 tracing names once tracing is off. Test updates run wider than one file for the same reason: every defaults assertion that expects a tracing integration now inits with tracing on, which is the redis test plus 20 orchestrion suites and the direct-client scenario in dev-packages/deno-integration-tests. Both suites pass.
Full deno suite passed, oxlint and oxfmt clean.
Closes #23893