Repository navigation
feat(core): Add __SENTRY_EXCLUDE_STATIC_LIFECYCLE__ flag to tree-shake the static span lifecycle - #26144
feat(core): Add __SENTRY_EXCLUDE_STATIC_LIFECYCLE__ flag to tree-shake the static span lifecycle#26144sentry-junior[bot] wants to merge 5 commits into
__SENTRY_EXCLUDE_STATIC_LIFECYCLE__ flag to tree-shake the static span lifecycle#26144Conversation
…transaction code With span streaming, the code that builds and processes transaction events is dead but still ships. Replacing `__SENTRY_EXCLUDE_TRANSACTIONS__` with `true` makes the SDK always stream spans and lets bundlers drop those paths. Defaults are unchanged. Refs #22580 Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e6aa1e9. Configure here.
size-limit report 📦Compared: 4f047bc (PR test merge using develop at 83f29b7) Against: 83f29b7 (develop) · base run #65541 ✅ Baseline matches: this diff contains only this PR's changes.
|
…ns are excluded With `__SENTRY_EXCLUDE_TRANSACTIONS__`, `traceLifecycle: 'static'` is ignored, so the Cloudflare client must still add its own `spanStreamingIntegration` with `flushOnSegmentEnd: false` instead of inheriting the default one from `ServerRuntimeClient`. Also use the conventions constants in the web vitals test.
Optional E2E testsSome optional checks were not successful. They do not block the merge. 8 not successful, 49 successful. See the run.
Successful checks
|
|
Why does this PR result in larger bundle sizes when this flag isn't set? 🤔 This is expected for the npm output. The flags remain so users can choose what they want to filter, but this shouldn't be impacting our CDN bundles. |
|
The extra bytes come from the guards themselves. The browser path has 7 inline guards: 3 in I replaced all 7 guards with
So the guards account for basically the whole CI delta, and the raw numbers line up with the +612 B on the uncompressed CDN tracing bundles. Cheaper guard forms I tried:
How much of the flag's saving each site carries (esbuild raw, flag set):
Ways to cut the default cost:
I'm happy to do either one, or both. Just say which you'd prefer. |
This is the correct route! |
…lse` in CDN bundles CDN bundles always keep transaction support, so the unresolved runtime checks for the flag only added bytes there.
|
@timfish done in 479b891. The bundle-only replace plugin ( Local
None of the 54 built |
The flag removes `traceLifecycle: 'static'`, not transactions as a concept, and the new name matches `__SENTRY_EXCLUDE_REPLAY_WORKER__`. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
sentry-junior
Bot
changed the title
__SENTRY_EXCLUDE_TRANSACTIONS__ flag to tree-shake transaction code__SENTRY_EXCLUDE_STATIC_LIFECYCLE__ flag to tree-shake the static span lifecycle
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
Probably we should add node's _INTERNAL_setDeferSegmentSpanCapture (packages/node/src/sdk/client.ts line 87) to the "left in" list, because Node's bundle size isn't the main priority here.
Also, there's no way to actually set this yet, for most users, right? Probably we should either add the bundler-plugins fix as part of this, or at least have an issue that tracks it so we don't forget to follow up with one.
Some comments below.
| } | ||
|
|
||
| if (isTransactionEvent(processedEvent)) { | ||
| if ( |
There was a problem hiding this comment.
The guard wraps the whole isTransactionEvent block, which includes the beforeSendTransaction call (line 1869). But transaction events still reach this code: a user can call captureEvent({ type: 'transaction', ... }), and the PR body says _processEvent keeps its transaction bookkeeping on purpose to handle them. Without the flag, in stream mode, nothing stops these events from going through beforeSendTransaction, ignoreSpans, and a static beforeSendSpan (I checked _processEvent, lines 1560-1612: there is no lifecycle check before processBeforeSend). With the flag, the same event is sent with none of these hooks run. beforeSendTransaction is often where users scrub PII, so the flag turns a scrubbed event into an unscrubbed one. The new test does not run transaction processing on transaction events asserts this as intended behavior.
So the flag changes runtime behavior beyond "drop dead code": stream mode without the flag and stream mode with the flag treat the same event in different ways.
Suggestion: pick one of these and make it explicit.
- Drop transaction events outright under the flag (for example, in
_processEvent, with adebuglog and a dropped-event outcome). This matches "the SDK can only stream spans" and removes more code. - Or move the guard so it only covers the span-processing part (lines 1817-1867), and keep the
beforeSendTransactioncall. That keeps behavior identical to stream mode without the flag, and still drops the 1,322 B measured for this site (thebeforeSendTransactioncall is small).
Either way, change the test so it asserts the chosen behavior, not "the hook is not called but the event is sent".
| attachStacktrace: true, | ||
| ...options, | ||
| traceLifecycle: options.traceLifecycle === 'static' ? 'static' : 'stream', | ||
| traceLifecycle: |
There was a problem hiding this comment.
A user with traceLifecycle: 'static' and beforeSendSpan: Sentry.withStaticSpan(scrub) gets the lifecycle forced to 'stream'. Then captureSpan (packages/core/src/tracing/spans/captureSpan.ts line 84) skips the static callback, so scrub never runs. The only warning for this mismatch (packages/core/src/client.ts lines 296-308) is gated on DEBUG_BUILD, and people who set this flag will usually also set __SENTRY_DEBUG__: false (the new size-limit entry does). So the scrubber stops running with no message at all.
The other warning, maybeWarnAboutIgnoredTransactionOptions, does fire (it is not debug-gated), but it tells the user to "set traceLifecycle: 'static'", which cannot work under the flag.
Suggestion: when options.traceLifecycle === 'static' and the flag is set, emit one non-debug console.warn from the constructor (inside the flag-true branch, so it costs nothing in builds without the flag). Something like "traceLifecycle: 'static' is not available in this build (__SENTRY_EXCLUDE_STATIC_LIFECYCLE__). beforeSendTransaction, ignoreTransactions and withStaticSpan callbacks are ignored." Then the warning in packages/core/src/utils/warnAboutIgnoredTransactionOptions.ts does not need to change.
| // Extract gen_ai spans from transaction and convert to span v2 format. | ||
| // This mutates event.spans to remove the extracted spans. | ||
| const genAiSpanItem = extractGenAiSpansFromEvent(event, this); | ||
| const genAiSpanItem = |
There was a problem hiding this comment.
low: gen_ai extraction is skipped for manual transaction events.
Same root as the comment on packages/core/src/client.ts line 1812, but smaller: a manually captured transaction event with gen_ai spans keeps them inline instead of having them split into a v2 span item. This is probably fine, but it is a behavior change for the same input. If we resolve the other comment by dropping transaction events under the flag, this goes away.
… lifecycle With `__SENTRY_EXCLUDE_STATIC_LIFECYCLE__`, manually captured transaction events skipped `beforeSendTransaction`, so a PII scrubber could stop running. Only the span processing is skipped now. Setting `traceLifecycle: 'static'` under the flag also logs a non-debug warning, because the static callbacks would otherwise stop running silently.
|
Doesn't seem to be worth it. weighing it up, every npm user pays +60/+117 B and with no way to set the flag without build tools. Dropping transactions is already planned tho, so this dead transaction weight won't be be lingering for long. |
@logaretm how long do you expect the transaction removal to take? If it's temporary, the default cost is temporary too. Until then, apps that set the flag save a notable amount (about 1 KB gzip with esbuild, 1.4 KB with webpack) and have less code to parse at startup. Everyone else pays around 100 bytes, and CDN bundles pay nothing because they replace the flag with The current default cost: size-limit showed +60 B gzip for |
Why: Span streaming is the default in v11, but a streaming app still ships the code that builds and processes transaction events (
_convertSpanToTransaction, the transaction branch ofprocessBeforeSend, gen_ai extraction, transaction-event conversion helpers, the web vitals measurements branch). RuntimehasSpanStreamingEnabled(client)/traceLifecyclechecks keep it alive.__SENTRY_TRACING__: false(#22835) only helps apps that don't trace at all.This adds an opt-in build flag. Replacing
__SENTRY_EXCLUDE_STATIC_LIFECYCLE__withtruemeans the SDK always streams spans:traceLifecycle: 'static'is ignored (normalized to'stream', and server clients still addspanStreamingIntegration), and the transaction code paths become dead code. Segments are still sent as span envelopes. When the flag isn't defined, behavior is unchanged.The guard sits inline at each call site, like
__SENTRY_TRACING__. A per-package module constant (likeDEBUG_BUILD) doesn't work. Webpack still drops the branches with it, but with the flag set, esbuild 0.27 kept them and the whole saving was lost (131,164 vs 127,648 B). CDN bundles replace the flag withfalseat build time, so the guards cost them nothing.Size (esbuild 0.27, minify, ESM, browser,
__SENTRY_DEBUG__: false;BrowserClient+browserTracingIntegration,tracesSampleRate: 1,setCurrentClient,client.init()):developtrueWith webpack via size-limit (
init+browserTracingIntegration), the flag saves 1,405 B gzip (49,304 → 47,899). The new size-limit entry tracks it.I ran an end-to-end check against the built packages: with the flag, a
startSpansegment produces aspanenvelope item, including whentraceLifecycle: 'static'is passed. Without the flag, behavior is unchanged (spanfor stream,transactionfor static).Left in:
_processEvent(dropped-span outcomes,transaction_infosource), which is small and still handles manually capturedtype: 'transaction'eventsmaybeWarnAboutIgnoredTransactionOptionsmeasurementsingetSpanJSON/static span JSONhasSpanStreamingEnabled(client) ? … : …ternaries in integrations (span names, attribute keys), plus static branches inbrowser-utilsweb-vitals/spans.tsandinteractions.tstraceLifecycle !== 'static'checkbundleSizeOptimizations.excludeStaticLifecycleoption in the bundler plugins (left for a follow-up)How to review: Start with
sentrySpan.tsandclient.ts.excludeStaticLifecycle.test.tscovers the flag semantics.Unsure about:
__SENTRY_EXCLUDE_TRANSACTIONS__at first. It's now__SENTRY_EXCLUDE_STATIC_LIFECYCLE__, because it removestraceLifecycle: 'static', and it follows the__SENTRY_EXCLUDE_*__convention.@sentry/browserand +117 B for@sentry/browser (incl. Tracing)in size-limit. CDN bundles are within ±3 B ofdevelop.Refs #22580
via @BYK.
--
View Session in Sentry