Skip to content

fix(browser): Default pageload span start time to timeOrigin #23502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Lms24 merged 2 commits into from
Aug 24, 2026
Merged

fix(browser): Default pageload span start time to timeOrigin #23502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb43562
fix(browser): Default pageload span start time to `timeOrigin`
Lms24 Aug 24, 2026
71a1c1d
remove leftover from rebase
Lms24 Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/astro/src/client/browserTracingIntegration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
} from '@sentry/browser';
import type { Client, Integration, TransactionSource } from '@sentry/core';
import {
browserPerformanceTimeOrigin,
debug,
hasSpanStreamingEnabled,
PAGELOAD_SPAN_NAME_FALLBACK,
Expand Down Expand Up @@ -40,14 +39,10 @@ export function browserTracingIntegration(

if (WINDOW.location) {
if (options.instrumentPageLoad != false) {
const origin = browserPerformanceTimeOrigin();

const { name, source } = getPageloadSpanName(client);

startBrowserTracingPageLoadSpan(client, {
name,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: origin ? origin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.astro',
Expand Down
19 changes: 13 additions & 6 deletions packages/browser/src/tracing/browserTracingIntegration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type {
} from '@sentry/core/browser';
import {
addNonEnumerableProperty,
browserPerformanceTimeOrigin,
consoleSandbox,
dateTimestampInSeconds,
debug,
Expand All @@ -37,6 +36,7 @@ import {
startInactiveSpan,
timestampInSeconds,
TRACING_DEFAULTS,
browserPerformanceTimeOrigin,
} from '@sentry/core/browser';
import {
addHistoryInstrumentationHandler,
Expand Down Expand Up @@ -636,13 +636,10 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption

if (WINDOW.location) {
if (instrumentPageLoad) {
const origin = browserPerformanceTimeOrigin();
startBrowserTracingPageLoadSpan(client, {
// With span streaming, span names have to be low cardinality, and there is no route
// information available here.

@sentry sentry Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@Lms24 Lms24 Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

name: hasSpanStreamingEnabled(client) ? PAGELOAD_SPAN_NAME_FALLBACK : WINDOW.location.pathname,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: origin ? origin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
Expand Down Expand Up @@ -720,12 +717,22 @@ export function startBrowserTracingPageLoadSpan(
spanOptions: StartSpanOptions,
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
): Span | undefined {
client.emit('startPageLoadSpan', spanOptions, traceOptions);

// `Pageload` is a low-cardinality span name, not a description of the page. The scope's
// transaction name is what error events are grouped by, so it keeps the URL instead.
const isFallbackSpanName = spanOptions.name === PAGELOAD_SPAN_NAME_FALLBACK;
getCurrentScope().setTransactionName(isFallbackSpanName ? WINDOW.location?.pathname : spanOptions.name);
// A pageload span always covers the entire page load, no matter how late the SDK or a routing
// instrumentation gets around to starting it. Everything that happened before (DNS, TLS, TTFB,
// HTML parsing, chunk loading) is part of the page load and the performance child spans we attach
// later are anchored at the time origin anyway.
const timeOrigin = browserPerformanceTimeOrigin();
const pageloadSpanOptions: StartSpanOptions = {
...spanOptions,
// startTime needs to be in seconds, not ms
startTime: spanOptions.startTime ?? (timeOrigin ? timeOrigin / 1000 : undefined),
};

client.emit('startPageLoadSpan', pageloadSpanOptions, traceOptions);

const pageloadSpan = getActiveIdleSpan(client);

Expand Down
35 changes: 35 additions & 0 deletions packages/browser/test/tracing/browserTracingIntegration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,41 @@ describe('browserTracingIntegration', () => {
expect(spanIsSampled(span!)).toBe(true);
});

it('starts the span at the time origin if no start time is provided', () => {
const client = new BrowserClient(
getDefaultBrowserClientOptions({
tracesSampleRate: 1,
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
}),
);
setCurrentClient(client);
client.init();

// Simulate the SDK (and therefore the routing instrumentation) only starting up 5s into the page load
vi.setSystemTime(browserPerformanceTimeOrigin()! + 5_000);

const span = startBrowserTracingPageLoadSpan(client, { name: 'test span' });

expect(spanToJSON(span!).start_timestamp).toBe(browserPerformanceTimeOrigin()! / 1000);
});

it('respects an explicitly passed start time', () => {
const client = new BrowserClient(
getDefaultBrowserClientOptions({
tracesSampleRate: 1,
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
}),
);
setCurrentClient(client);
client.init();

const startTime = browserPerformanceTimeOrigin()! / 1000 + 12;

const span = startBrowserTracingPageLoadSpan(client, { name: 'test span', startTime });

expect(spanToJSON(span!).start_timestamp).toBe(startTime);
});

it('allows to overwrite properties', () => {
const client = new BrowserClient(
getDefaultBrowserClientOptions({
Expand Down
3 changes: 0 additions & 3 deletions packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Client, Span } from '@sentry/core';
import {
browserPerformanceTimeOrigin,
GLOBAL_OBJ,
hasSpanStreamingEnabled,
PAGELOAD_SPAN_NAME_FALLBACK,
Expand Down Expand Up @@ -61,12 +60,10 @@ const currentRouterPatchingNavigationSpanRef: NavigationSpanRef = { current: und
export function appRouterInstrumentPageLoad(client: Client): void {
const pathname = stripTrailingSlash(WINDOW.location.pathname);
const parameterizedPathname = maybeParameterizeRoute(pathname);
const origin = browserPerformanceTimeOrigin();
startBrowserTracingPageLoadSpan(client, {
// With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
name: parameterizedPathname ?? (hasSpanStreamingEnabled(client) ? PAGELOAD_SPAN_NAME_FALLBACK : pathname),
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: origin ? origin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.app_router_instrumentation',
Expand Down
4 changes: 0 additions & 4 deletions packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Client, TransactionSource } from '@sentry/core';
import {
browserPerformanceTimeOrigin,
debug,
hasSpanStreamingEnabled,
PAGELOAD_SPAN_NAME_FALLBACK,
Expand Down Expand Up @@ -123,13 +122,10 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
name = name.replace(/^(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS|TRACE|CONNECT)\s+/i, '');
}

const origin = browserPerformanceTimeOrigin();
startBrowserTracingPageLoadSpan(
client,
{
name,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
startTime: origin ? origin / 1000 : undefined,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation',
Expand Down
Loading