Skip to content

fix(browser): Skip profiling for unsampled active root spans - #25498

Merged
Lms24 merged 1 commit into
getsentry:developfrom
Jxxunnn:fix/browser-profiling-trace-unsampled-root-span
Oct 19, 2026
Merged

Lms24 merged 1 commit into
getsentry:developfrom
Jxxunnn:fix/browser-profiling-trace-unsampled-root-span

Conversation

@Jxxunnn

@Jxxunnn Jxxunnn commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
  • Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked.

Closes #25497

With the trace profile lifecycle in @sentry/browser, notifyRootSpanActive() started the profiler for a root span that was already active at setup. It did not check whether that span was sampled. As a result, an unsampled pageload span started UI profiling and sent a profile_chunk, even with tracesSampleRate: 0.

This change returns early in notifyRootSpanActive() when the root span is not recording. This matches the check in the spanStart listener.

Two tests were added:

  • A regression test for an unsampled pageload span with the trace profile lifecycle.
  • A test for a sampled root span that is already active when the integration is added. Here only notifyRootSpanActive() can start the profiler, so the test covers that path directly. It also checks that the second notification from the timeout does not start another profiler.

Validation (in packages/browser):

  • yarn test: 763 tests passed, including the two new tests
  • yarn lint: no errors; oxfmt --check passed on the changed files
  • The regression test fails without the fix
  • The new active-root-span test fails if notifyRootSpanActive() is disabled
  • Browser integration tests (Playwright) were not run

@Jxxunnn
Jxxunnn requested a review from a team as a code owner October 19, 2026 06:24
@Jxxunnn
Jxxunnn requested review from logaretm and msonnb and removed request for a team October 19, 2026 06:24
@github-actions github-actions Bot added the external PR from an external contributor label Oct 19, 2026

@Lms24 Lms24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Lms24 Lms24 self-assigned this Oct 19, 2026
@Lms24 Lms24 changed the title fix(browserProfiling): Skip profiling for unsampled active root spans fix(browser): Skip profiling for unsampled active root spans Oct 19, 2026
@Lms24
Lms24 merged commit 686e75b into getsentry:develop Oct 19, 2026
218 checks passed
Lms24 pushed a commit that referenced this pull request Oct 19, 2026
)

Backport of: #25498

## Differences to the original PR

- `packages/browser/test/profiling/integration.test.ts`: the two new
tests use v10's existing `import * as Sentry from '@sentry/browser'`
style (`Sentry.init`, `Sentry.addIntegration`, etc.) instead of named
imports from `../../src/index`. That import block conflicted because
v10's test file still uses the namespace import.
- Same file: the unsampled-pageload test uses `Sentry.spanToJSON`
instead of `spanToStaticSpanJSON`, because `spanToStaticSpanJSON`
doesn't exist on v10.

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Lms24 added a commit that referenced this pull request Oct 19, 2026
This PR adds the external contributor to the CHANGELOG.md file, so that
they are credited for their contribution. See #25498

Co-authored-by: Lms24 <8420481+Lms24@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external PR from an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsampled pageload span is profiled when profileLifecycle is 'trace' (browser UI profiling)

2 participants