Conversation
Contributor
size-limit report 📦
|
mydea
force-pushed
the
fn/move-browser-span-apis-to-browser-utils
branch
from
September 1, 2026 08:34
1d990e7 to
9b6ce01
Compare
mydea
force-pushed
the
fn/move-browser-span-apis-to-browser-utils
branch
from
September 1, 2026 08:36
9b6ce01 to
ae31677
Compare
mydea
marked this pull request as ready for review
September 1, 2026 08:57
mydea
requested review from
logaretm,
msonnb,
nicohrubec and
s1gr1d
and removed request for
a team
September 1, 2026 08:57
mydea
requested review from
JPeer264 and
isaacs
and removed request for
a team
September 1, 2026 10:07
Moves the guarded startSpan/startInactiveSpan/startSpanManual wrappers and the ensureBrowserSpanStreaming gate from @sentry/core/browser into @sentry/browser-utils, and decouples startIdleSpan from span streaming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mydea
force-pushed
the
fn/move-browser-span-apis-to-browser-utils
branch
from
September 1, 2026 10:07
ae31677 to
951561d
Compare
Contributor
JPeer264
approved these changes
Sep 8, 2026
| // The pageload/navigation idle span is started through the unguarded `startIdleSpan`, so - unlike | ||
| // the guarded `startSpan` APIs - nothing installs span streaming for it. We ensure it here. | ||
| // `addIntegration` is idempotent by name, so this is safe even if a span already installed it. | ||
| if (hasSpanStreamingEnabled(client)) { |
Member
There was a problem hiding this comment.
l: Can't we export _INTERNAL_ensureBrowserSpanStreaming from @sentry/browser-utils and use it here instead?
|
|
||
| // Interaction spans are started through `startIdleSpan`, which - unlike the guarded `startSpan` | ||
| // APIs - does not install span streaming itself, so we ensure it here. | ||
| ensureBrowserSpanStreaming(client); |
Member
There was a problem hiding this comment.
q: This means we would need this for every integration that uses startIdleSpan from @sentry/core/browser, is there any safeguard that would prevent missing ensureBrowserSpanStreaming in such integrations? Or maybe a startIdleSpan from @sentry/browser-utils we can use internally that mirrors what startSpan does?
logaretm
reviewed
Sep 14, 2026
| browserPerformanceTimeOrigin, | ||
| } from '@sentry/core'; | ||
| import { _INTERNAL_ensureBrowserSpanStreaming, startIdleSpan, startInactiveSpan } from '@sentry/core/browser'; | ||
| import { startIdleSpan } from '@sentry/core/browser'; |
Member
There was a problem hiding this comment.
l: Do you think it would be confusing to import certain APIs from borwser-utils and the rest from core/browser?
I noticed startIdleSpan is the only API that is not present in the spanApis in the browser utils, any reason for that?
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
3 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.
Stacked on top of #23762 — review/merge that first.
Follow-up to the core entrypoint split: the guarded browser span-start APIs do not belong in
@sentry/core. They exist purely so thatspanStreamingIntegrationis reachable only from code that starts a span, which is a browser-SDK concern, so this moves them down into@sentry/browser-utils(which already sits below@sentry/browserand the framework SDKs).What moves out of
@sentry/core/browser:startSpan/startInactiveSpan/startSpanManual— the guarded wrappers that install span streaming before delegating to the plain core APIs — now live in@sentry/browser-utils. Everything they need is already public on@sentry/core.ensureBrowserSpanStreaming(previously_INTERNAL_ensureBrowserSpanStreaming) moves too, and is now package-private to@sentry/browser-utils. Its only callers are the wrappers andinteractionsIntegration.startIdleSpanstays in@sentry/core/browser, and no longer installs span streaming itself. That responsibility now sits with its callers:interactionsIntegrationensures it insetup(), andbrowserTracingIntegrationcontinues to ensure it inafterAllSetup(guaranteeing the pageload segment streams with browser tracing alone).Decisions
@sentry/core/browseris introduced by ref(core)!: Split browser/server-only exports out of the default entrypoint #23762 and unreleased, and@sentry/browserstill re-exports the wrappers unchanged. The framework SDKs (effect, react-router, svelte, sveltekit) were repointed to packages they already depend on (@sentry/browser/@sentry/svelte), so no new dependencies were added.no-unguarded-span-apissuppression — it is itself the guarded variant the rule steers callers toward.browserTracingIntegrationcan no longer reach the now-private gate, so it installs span streaming inline via core’\s publichasSpanStreamingEnabled+spanStreamingIntegration. This is a small, idempotent duplication of the gate logic; the alternative was keepingensureBrowserSpanStreamingexported, which we chose not to do.XhrBreadcrumbData/XhrBreadcrumbHint/BrowserClientReplayOptionstypes stay in@sentry/core:XhrBreadcrumbHintis referenced by core’\s ownClientsignatures (moving it would invert the dependency), andBrowserClientReplayOptionswas deliberately parked in core to avoid a browser↔replay cycle.