fix(browser): Default pageload span start time to timeOrigin - #23502
Merged
Conversation
Contributor
size-limit report 📦
|
Fixes #23469 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lms24
force-pushed
the
fix/pageload-span-time-origin
branch
from
August 24, 2026 11:51
d23b79a to
cb43562
Compare
Lms24
marked this pull request as ready for review
August 24, 2026 11:52
Lms24
requested review from
chargome,
logaretm,
msonnb and
nicohrubec
and removed request for
a team
August 24, 2026 11:52
![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 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb43562. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const origin = browserPerformanceTimeOrigin(); | ||
| startBrowserTracingPageLoadSpan(client, { | ||
| // With span streaming, span names have to be low cardinality, and there is no route | ||
| // information available here. |
Contributor
There was a problem hiding this comment.
Bug: The isRedirect() check fails because it uses a fixed 1.5s threshold against the page's load time, not accounting for SDK initialization delays, breaking redirect detection.
Severity: HIGH
Suggested Fix
Update the isRedirect() function to account for the change in the pageload span's start_timestamp. The logic should be revised to correctly identify early-lifecycle redirects, potentially by comparing against the SDK initialization time rather than the page's origin time, or by using a different mechanism to track the first navigation event after initialization.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/browser/src/tracing/browserTracingIntegration.ts#L641
Potential issue: The logic for detecting redirects in `isRedirect()` is flawed due to a
timing mismatch introduced in the pull request. The pageload span's `start_timestamp` is
now correctly set to the page's actual load time (`browserPerformanceTimeOrigin()`), but
the redirect check still compares the current time against this timestamp using a fixed
1.5-second threshold. In production, the SDK often initializes more than 1.5 seconds
after the page begins to load. This delay causes the check `now - startTimestamp >
REDIRECT_THRESHOLD` to incorrectly evaluate to true, effectively disabling redirect
detection for any navigation that occurs after this brief window. This results in the
creation of new, separate root transactions instead of correctly grouping redirects as
child spans of the initial pageload.
Did we get this right? 👍 / 👎 to inform future reviews.
Member
Author
There was a problem hiding this comment.
given we're fixing a bug here, this is pre-existing behaviour and should already be adjusted to what we expect here. I think 1.5s is okay here but we might wanna increase if this ever becomes an issue
JPeer264
pushed a commit
that referenced
this pull request
Aug 25, 2026
This PR makes `startBrowserTracingPageLoadSpan` default `startTime` to `browserPerformanceTimeOrigin()` when the caller doesn't pass one. Only `browser`, `astro` and the two Next.js routers passed it, so pageload spans from the react-router, TanStack Router, SvelteKit, Ember and Remix instrumentations started whenever `Sentry.init`which was too late. Important: Pageload durations for the affected framework integrations will visibly increase after this fix but IMHO this is a correctness fix that leads to more honest pageload times. Furthermore, pageload durations shouldn't be trusted too much anyway given the idle/debouncing time behaviour Known affected SDKs: react-router, TanStack Router, SvelteKit, Ember, and Remix instrumentations started whenever Concrete changes: - Default `startTime` to the time origin in `startBrowserTracingPageLoadSpan` - Emit the augmented options on the `startPageLoadSpan` hook so `beforeStartSpan` and hook listeners see the real start time - Remove the now redundant `startTime` overrides from `browser`, `astro` and both Next.js routing instrumentations Fixes #23469
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.
This PR makes
startBrowserTracingPageLoadSpandefaultstartTimetobrowserPerformanceTimeOrigin()when the caller doesn't pass one. Onlybrowser,astroand the two Next.js routers passed it, so pageload spans from the react-router, TanStack Router, SvelteKit, Ember and Remix instrumentations started wheneverSentry.initwhich was too late.Important: Pageload durations for the affected framework integrations will visibly increase after this fix but IMHO this is a correctness fix that leads to more honest pageload times. Furthermore, pageload durations shouldn't be trusted too much anyway given the idle/debouncing time behaviour
Known affected SDKs: react-router, TanStack Router, SvelteKit, Ember, and Remix instrumentations started whenever
Concrete changes:
startTimeto the time origin instartBrowserTracingPageLoadSpanstartPageLoadSpanhook sobeforeStartSpanand hook listeners see the real start timestartTimeoverrides frombrowser,astroand both Next.js routing instrumentationsFixes #23469