Skip to content

Commit cb43562

Browse files
Lms24claude
andcommitted
fix(browser): Default pageload span start time to timeOrigin
Fixes #23469 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ab7f84f commit cb43562

5 files changed

Lines changed: 49 additions & 18 deletions

File tree

packages/astro/src/client/browserTracingIntegration.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
} from '@sentry/browser';
66
import type { Client, Integration, TransactionSource } from '@sentry/core';
77
import {
8-
browserPerformanceTimeOrigin,
98
debug,
109
hasSpanStreamingEnabled,
1110
PAGELOAD_SPAN_NAME_FALLBACK,
@@ -40,14 +39,10 @@ export function browserTracingIntegration(
4039

4140
if (WINDOW.location) {
4241
if (options.instrumentPageLoad != false) {
43-
const origin = browserPerformanceTimeOrigin();
44-
4542
const { name, source } = getPageloadSpanName(client);
4643

4744
startBrowserTracingPageLoadSpan(client, {
4845
name,
49-
// pageload should always start at timeOrigin (and needs to be in s, not ms)
50-
startTime: origin ? origin / 1000 : undefined,
5146
attributes: {
5247
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
5348
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.astro',

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type {
1010
} from '@sentry/core/browser';
1111
import {
1212
addNonEnumerableProperty,
13-
browserPerformanceTimeOrigin,
1413
consoleSandbox,
1514
dateTimestampInSeconds,
1615
debug,
@@ -37,6 +36,7 @@ import {
3736
startInactiveSpan,
3837
timestampInSeconds,
3938
TRACING_DEFAULTS,
39+
browserPerformanceTimeOrigin,
4040
} from '@sentry/core/browser';
4141
import {
4242
addHistoryInstrumentationHandler,
@@ -636,13 +636,10 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
636636

637637
if (WINDOW.location) {
638638
if (instrumentPageLoad) {
639-
const origin = browserPerformanceTimeOrigin();
640639
startBrowserTracingPageLoadSpan(client, {
641640
// With span streaming, span names have to be low cardinality, and there is no route
642641
// information available here.
643642
name: hasSpanStreamingEnabled(client) ? PAGELOAD_SPAN_NAME_FALLBACK : WINDOW.location.pathname,
644-
// pageload should always start at timeOrigin (and needs to be in s, not ms)
645-
startTime: origin ? origin / 1000 : undefined,
646643
attributes: {
647644
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
648645
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
@@ -720,12 +717,23 @@ export function startBrowserTracingPageLoadSpan(
720717
spanOptions: StartSpanOptions,
721718
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
722719
): Span | undefined {
723-
client.emit('startPageLoadSpan', spanOptions, traceOptions);
724-
725720
// `Pageload` is a low-cardinality span name, not a description of the page. The scope's
726721
// transaction name is what error events are grouped by, so it keeps the URL instead.
727722
const isFallbackSpanName = spanOptions.name === PAGELOAD_SPAN_NAME_FALLBACK;
728723
getCurrentScope().setTransactionName(isFallbackSpanName ? WINDOW.location?.pathname : spanOptions.name);
724+
// A pageload span always covers the entire page load, no matter how late the SDK or a routing
725+
// instrumentation gets around to starting it. Everything that happened before (DNS, TLS, TTFB,
726+
// HTML parsing, chunk loading) is part of the page load and the performance child spans we attach
727+
// later are anchored at the time origin anyway.
728+
const timeOrigin = browserPerformanceTimeOrigin();
729+
const pageloadSpanOptions: StartSpanOptions = {
730+
...spanOptions,
731+
// startTime needs to be in seconds, not ms
732+
startTime: spanOptions.startTime ?? (timeOrigin ? timeOrigin / 1000 : undefined),
733+
};
734+
735+
client.emit('startPageLoadSpan', pageloadSpanOptions, traceOptions);
736+
getCurrentScope().setTransactionName(pageloadSpanOptions.name);
729737

730738
const pageloadSpan = getActiveIdleSpan(client);
731739

packages/browser/test/tracing/browserTracingIntegration.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,41 @@ describe('browserTracingIntegration', () => {
497497
expect(spanIsSampled(span!)).toBe(true);
498498
});
499499

500+
it('starts the span at the time origin if no start time is provided', () => {
501+
const client = new BrowserClient(
502+
getDefaultBrowserClientOptions({
503+
tracesSampleRate: 1,
504+
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
505+
}),
506+
);
507+
setCurrentClient(client);
508+
client.init();
509+
510+
// Simulate the SDK (and therefore the routing instrumentation) only starting up 5s into the page load
511+
vi.setSystemTime(browserPerformanceTimeOrigin()! + 5_000);
512+
513+
const span = startBrowserTracingPageLoadSpan(client, { name: 'test span' });
514+
515+
expect(spanToJSON(span!).start_timestamp).toBe(browserPerformanceTimeOrigin()! / 1000);
516+
});
517+
518+
it('respects an explicitly passed start time', () => {
519+
const client = new BrowserClient(
520+
getDefaultBrowserClientOptions({
521+
tracesSampleRate: 1,
522+
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
523+
}),
524+
);
525+
setCurrentClient(client);
526+
client.init();
527+
528+
const startTime = browserPerformanceTimeOrigin()! / 1000 + 12;
529+
530+
const span = startBrowserTracingPageLoadSpan(client, { name: 'test span', startTime });
531+
532+
expect(spanToJSON(span!).start_timestamp).toBe(startTime);
533+
});
534+
500535
it('allows to overwrite properties', () => {
501536
const client = new BrowserClient(
502537
getDefaultBrowserClientOptions({

packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { Client, Span } from '@sentry/core';
22
import {
3-
browserPerformanceTimeOrigin,
43
GLOBAL_OBJ,
54
hasSpanStreamingEnabled,
65
PAGELOAD_SPAN_NAME_FALLBACK,
@@ -61,12 +60,10 @@ const currentRouterPatchingNavigationSpanRef: NavigationSpanRef = { current: und
6160
export function appRouterInstrumentPageLoad(client: Client): void {
6261
const pathname = stripTrailingSlash(WINDOW.location.pathname);
6362
const parameterizedPathname = maybeParameterizeRoute(pathname);
64-
const origin = browserPerformanceTimeOrigin();
6563
startBrowserTracingPageLoadSpan(client, {
6664
// With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
6765
name: parameterizedPathname ?? (hasSpanStreamingEnabled(client) ? PAGELOAD_SPAN_NAME_FALLBACK : pathname),
6866
// pageload should always start at timeOrigin (and needs to be in s, not ms)
69-
startTime: origin ? origin / 1000 : undefined,
7067
attributes: {
7168
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
7269
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.app_router_instrumentation',

packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { Client, TransactionSource } from '@sentry/core';
22
import {
3-
browserPerformanceTimeOrigin,
43
debug,
54
hasSpanStreamingEnabled,
65
PAGELOAD_SPAN_NAME_FALLBACK,
@@ -123,13 +122,10 @@ export function pagesRouterInstrumentPageLoad(client: Client): void {
123122
name = name.replace(/^(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS|TRACE|CONNECT)\s+/i, '');
124123
}
125124

126-
const origin = browserPerformanceTimeOrigin();
127125
startBrowserTracingPageLoadSpan(
128126
client,
129127
{
130128
name,
131-
// pageload should always start at timeOrigin (and needs to be in s, not ms)
132-
startTime: origin ? origin / 1000 : undefined,
133129
attributes: {
134130
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
135131
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.nextjs.pages_router_instrumentation',

0 commit comments

Comments
 (0)