ref(browser): Tree-shake span streaming out of error-only bundles - #23362
Draft
Lms24 wants to merge 1 commit into
Draft
ref(browser): Tree-shake span streaming out of error-only bundles#23362Lms24 wants to merge 1 commit into
Lms24 wants to merge 1 commit into
Conversation
`init()` no longer pushes `spanStreamingIntegration()`. That single reference was the only thing retaining `SpanBuffer`, `captureSpan`, `estimateSize` and `scopeContextAttributes` in every browser bundle, including bundles for apps that never use tracing. Instead, browser-facing packages import their span-start APIs from `@sentry/core/browser`, which installs the integration on the client before the first span starts. A span can only end if it was started, so the `afterSpanEnd` listener is always registered in time. `init` alone drops from 28.52 kB to 26.95 kB gzipped, within 0.1 kB of the `__SENTRY_TRACING__: false` floor. Tracing users pay nothing extra. Both variants share their names, so a stray plain import compiles fine and silently stops sending spans. `sdk/no-unguarded-span-apis` is the only thing that catches that, hence it lands here rather than later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
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.
Stacked on #23361. Makes
spanStreamingIntegrationreachable only if span-creating code is reachable, so error-only apps drop the whole span streaming graph automatically — no__SENTRY_TRACING__flag needed. No public API changes:Sentry.startSpan()from@sentry/browserworks exactly as before.Measured with
size-limit(webpack, gzipped + minified):initinit+ treeshaking flagsinit+__SENTRY_TRACING__: falseinit, browserTracingIntegrationLanding within 0.10 kB of the
__SENTRY_TRACING__: falsefloor showsinit's reference was the only thing retaining span streaming — there is no second reference to hunt down. Tracing users pay nothing extra.This PR:
spanStreamingIntegration()push frompackages/browser/src/sdk.ts, along with the now-unused__SENTRY_TRACING__declaration.@sentry/core/browserspan-start APIs:browser-utils(performance/utils.ts,web-vitals/spans.ts),browser/src/integrations/fetchStreamPerformance.ts,svelte,sveltekit/src/client/load.ts,react-router/src/client/createClientInstrumentation.tsandeffect/src/tracer.ts. Import names are unchanged._INTERNAL_ensureBrowserSpanStreaming(client)frombrowserTracingIntegration'safterAllSetup. Not load-bearing for correctness, but it meansgetIntegrationByName('SpanStreaming')andevent.sdk.integrationsare correct for errors captured before the first span starts.nextSpanre-export frompackages/nextjs/src/client/index.ts, so the client keeps@sentry/react's span APIs.sdk/no-unguarded-span-apisoxlint rule, enabled once in.oxlintrc.base.json.init-only.size-limit.jsentries to under 1 KB headroom.packages/browser/test/tracing/spanStreamingWiring.test.ts, covering the wiring throughinitend-to-end.Notes for review:
Client.addIntegrationrunssetup()synchronously and is idempotent by name, so theafterSpanEndlistener is always registered before any span can end: a span can only end if it was started, and starting goes through a wrapper.afterAllSetup, notsetup.Client._setupIntegrationsassignsthis._integrations = setupIntegrations(...)after the setup loop, so anaddIntegrationfrom inside another integration'ssetup()gets clobbered in the index — the hooks would still fire, but the bookkeeping this step exists for would be lost.react-router/src/client/createClientInstrumentation.ts, which the original audit missed.effect/src/tracer.tsbacks both the client and the server entry, so it uses the guarded variant too. On the server that's a no-op:ServerRuntimeClientalready installs the integration eagerly andaddIntegrationdedupes by name. This is also why the rule's scope is expressed in code rather than as config globs — nofilesglob can say "this one file but not its siblings".nextjs/src/common/utils/nextSpan.tsare unreachable in the browser:isBuild()reads the server-onlyNEXT_PHASEenv var,isUseCacheFunction()is a React server-reference check, and thebrowserexport condition maps only toindex.client.js(client components are SSR'd through the server entry). So the client needs no wrapper, andisBuild/isUseCacheFunction/SentryNonRecordingSpandrop out of the client bundle.createChildOrRootSpan(same size win, one call site — but it installs core's variant, which lacks the browser variant's 500 ms per-trace flush onafterSegmentSpanEnd, meaning real data loss on navigation and tab close; it also makes core's hottest path depend on more of theClientsurface and charges Node a per-span cost for a browser-only benefit). Registering anAsyncContextStrategyfrominit()defeats its own purpose: the registration references the browserstartSpan, so nothing tree-shakes.