diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/init.js new file mode 100644 index 000000000000..39a212ae5fa5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/init.js @@ -0,0 +1,15 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window._testBaseTimestamp = performance.timeOrigin / 1000; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({ enableReportPageLoaded: true })], + tracesSampleRate: 1, + debug: true, +}); + +setTimeout(() => { + Sentry.reportPageLoaded(); +}, 2500); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/test.ts new file mode 100644 index 000000000000..cb346c55745a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/default/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'waits for Sentry.reportPageLoaded() to be called when `enableReportPageLoaded` is true', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const eventData = envelopeRequestParser(await pageloadEventPromise); + + const traceContextData = eventData.contexts?.trace?.data; + const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; + + expect(traceContextData).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'reportPageLoaded', + }); + + // We wait for 2.5 seconds before calling Sentry.reportPageLoaded() + // the margins are to account for timing weirdness in CI to avoid flakes + expect(spanDurationSeconds).toBeGreaterThan(2); + expect(spanDurationSeconds).toBeLessThan(3); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/init.js new file mode 100644 index 000000000000..7ec015be44dd --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/init.js @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window._testBaseTimestamp = performance.timeOrigin / 1000; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({ enableReportPageLoaded: true, finalTimeout: 3000 })], + tracesSampleRate: 1, + debug: true, +}); + +// not calling Sentry.reportPageLoaded() on purpose! diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/test.ts new file mode 100644 index 000000000000..df90ed1443f6 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/finalTimeout/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'final timeout cancels the pageload span even if `enableReportPageLoaded` is true', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const eventData = envelopeRequestParser(await pageloadEventPromise); + + const traceContextData = eventData.contexts?.trace?.data; + const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; + + expect(traceContextData).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'finalTimeout', + }); + + // We wait for 3 seconds before calling Sentry.reportPageLoaded() + // the margins are to account for timing weirdness in CI to avoid flakes + expect(spanDurationSeconds).toBeGreaterThan(2.5); + expect(spanDurationSeconds).toBeLessThan(3.5); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/init.js new file mode 100644 index 000000000000..65e9938f7985 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/init.js @@ -0,0 +1,19 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window._testBaseTimestamp = performance.timeOrigin / 1000; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration({ enableReportPageLoaded: true, instrumentNavigation: false })], + tracesSampleRate: 1, + debug: true, +}); + +setTimeout(() => { + Sentry.startBrowserTracingNavigationSpan(Sentry.getClient(), { name: 'custom_navigation' }); +}, 1000); + +setTimeout(() => { + Sentry.reportPageLoaded(); +}, 2500); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/test.ts new file mode 100644 index 000000000000..75789cdc6de9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/reportPageLoaded/navigation/test.ts @@ -0,0 +1,40 @@ +import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/browser'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'starting a navigation span cancels the pageload span even if `enableReportPageLoaded` is true', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + + const eventData = envelopeRequestParser(await pageloadEventPromise); + + const traceContextData = eventData.contexts?.trace?.data; + const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; + + expect(traceContextData).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'cancelled', + }); + + // ending span after 1s but adding a margin of 0.5s to account for timing weirdness in CI to avoid flakes + expect(spanDurationSeconds).toBeLessThan(1.5); + }, +); diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts index 5da27d74cb8d..fc805c82a4e5 100644 --- a/packages/browser/src/index.bundle.tracing.replay.feedback.ts +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -23,6 +23,8 @@ export { startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; +export { reportPageLoaded } from './tracing/reportPageLoaded'; + export { getFeedback, sendFeedback } from '@sentry-internal/feedback'; export { feedbackAsyncIntegration as feedbackAsyncIntegration, feedbackAsyncIntegration as feedbackIntegration }; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index 47a2a16ae06b..f77d2774c36e 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -22,6 +22,9 @@ export { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; + +export { reportPageLoaded } from './tracing/reportPageLoaded'; + export { feedbackIntegrationShim as feedbackAsyncIntegration, feedbackIntegrationShim as feedbackIntegration }; export { replayIntegration, getReplay } from '@sentry-internal/replay'; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index 72e2a40d07a1..c32e806f1de8 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -23,6 +23,8 @@ export { startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; +export { reportPageLoaded } from './tracing/reportPageLoaded'; + export { feedbackIntegrationShim as feedbackAsyncIntegration, feedbackIntegrationShim as feedbackIntegration, diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 35de77f611d0..8353477a2975 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -42,6 +42,7 @@ export { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from './tracing/browserTracingIntegration'; +export { reportPageLoaded } from './tracing/reportPageLoaded'; export type { RequestInstrumentationOptions } from './tracing/request'; export { registerSpanErrorInstrumentation, diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 305b1e0322a0..a79f629855d7 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -250,6 +250,23 @@ export interface BrowserTracingOptions { */ consistentTraceSampling: boolean; + /** + * If set to `true`, the pageload span will not end itself automatically, unless it + * runs until the {@link BrowserTracingOptions.finalTimeout} (30 seconds by default) is reached. + * + * Set this option to `true`, if you want full control over the pageload span duration. + * You can use `Sentry.reportPageLoaded()` to manually end the pageload span whenever convenient. + * Be aware that you have to ensure that this is always called, regardless of the chosen route + * or path in the application. + * + * @default `false`. By default, the pageload span will end itself automatically, based on + * the {@link BrowserTracingOptions.finalTimeout}, {@link BrowserTracingOptions.idleTimeout} + * and {@link BrowserTracingOptions.childSpanTimeout}. This is more convenient to use but means + * that the pageload duration can be arbitrary and might not be fully representative of a perceived + * page load time. + */ + enableReportPageLoaded: boolean; + /** * _experiments allows the user to send options to define how this integration works. * @@ -297,6 +314,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { detectRedirects: true, linkPreviousTrace: 'in-memory', consistentTraceSampling: false, + enableReportPageLoaded: false, _experiments: {}, ...defaultRequestInstrumentationOptions, }; @@ -310,7 +328,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { * * We explicitly export the proper type here, as this has to be extended in some cases. */ -export const browserTracingIntegration = ((_options: Partial = {}) => { +export const browserTracingIntegration = ((options: Partial = {}) => { const latestRoute: RouteInfo = { name: undefined, source: undefined, @@ -345,18 +363,21 @@ export const browserTracingIntegration = ((_options: Partial void); let lastInteractionTimestamp: number | undefined; + let _pageloadSpan: Span | undefined; + /** Create routing idle transaction. */ function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions, makeActive = true): void { - const isPageloadTransaction = startSpanOptions.op === 'pageload'; + const isPageloadSpan = startSpanOptions.op === 'pageload'; const initialSpanName = startSpanOptions.name; const finalStartSpanOptions: StartSpanOptions = beforeStartSpan @@ -390,7 +411,7 @@ export const browserTracingIntegration = ((_options: Partial { // This will generally always be defined here, because it is set in `setup()` of the integration // but technically, it is optional, so we guard here to be extra safe @@ -415,9 +436,19 @@ export const browserTracingIntegration = ((_options: Partial { emitFinish(); }); @@ -573,7 +605,15 @@ export const browserTracingIntegration = ((_options: Partial { + if (enableReportPageLoaded && _pageloadSpan) { + _pageloadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, 'reportPageLoaded'); + _pageloadSpan.end(); + } + }); }, + afterAllSetup(client) { let startingUrl: string | undefined = getLocationHref(); diff --git a/packages/browser/src/tracing/reportPageLoaded.ts b/packages/browser/src/tracing/reportPageLoaded.ts new file mode 100644 index 000000000000..2d3d4a4991a4 --- /dev/null +++ b/packages/browser/src/tracing/reportPageLoaded.ts @@ -0,0 +1,14 @@ +import type { Client } from '@sentry/core'; +import { getClient } from '@sentry/core'; + +/** + * Manually report the end of the page load, resulting in the SDK ending the pageload span. + * This only works if {@link BrowserTracingOptions.enableReportPageLoaded} is set to `true`. + * Otherwise, the pageload span will end itself based on the {@link BrowserTracingOptions.finalTimeout}, + * {@link BrowserTracingOptions.idleTimeout} and {@link BrowserTracingOptions.childSpanTimeout}. + * + * @param client - The client to use. If not provided, the global client will be used. + */ +export function reportPageLoaded(client: Client | undefined = getClient()): void { + client?.emit('endPageloadSpan'); +} diff --git a/packages/browser/test/tracing/reportPageLoaded.test.ts b/packages/browser/test/tracing/reportPageLoaded.test.ts new file mode 100644 index 000000000000..48329b748970 --- /dev/null +++ b/packages/browser/test/tracing/reportPageLoaded.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it, vi } from 'vitest'; +import { BrowserClient, setCurrentClient } from '../../src'; +import { reportPageLoaded } from '../../src/tracing/reportPageLoaded'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; + +describe('reportPageLoaded', () => { + it('emits the endPageloadSpan event on the global client if no client is passed', () => { + const client = new BrowserClient(getDefaultBrowserClientOptions({})); + setCurrentClient(client); + + const emitSpy = vi.spyOn(client, 'emit'); + reportPageLoaded(); + + expect(emitSpy).toHaveBeenCalledWith('endPageloadSpan'); + }); + + it('emits the endPageloadSpan event on the passed client', () => { + const client = new BrowserClient(getDefaultBrowserClientOptions({})); + const emitSpy = vi.spyOn(client, 'emit'); + reportPageLoaded(client); + + expect(emitSpy).toHaveBeenCalledWith('endPageloadSpan'); + }); +}); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 6a7451fe96cb..7e8c434ed976 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -606,6 +606,12 @@ export abstract class Client { ) => void, ): () => void; + /** + * A hook for the browser tracing integrations to trigger the end of a page load span. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on(hook: 'endPageloadSpan', callback: () => void): () => void; + /** * A hook for the browser tracing integrations to trigger after the pageload span was started. * @returns {() => void} A function that, when executed, removes the registered callback. @@ -800,6 +806,11 @@ export abstract class Client { traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined }, ): void; + /** + * Emit a hook event for browser tracing integrations to trigger the end of a page load span. + */ + public emit(hook: 'endPageloadSpan'): void; + /** * Emit a hook event for browser tracing integrations to trigger aafter the pageload span was started. */ diff --git a/packages/core/src/tracing/idleSpan.ts b/packages/core/src/tracing/idleSpan.ts index c8b37adec436..b35e31322ecd 100644 --- a/packages/core/src/tracing/idleSpan.ts +++ b/packages/core/src/tracing/idleSpan.ts @@ -75,8 +75,16 @@ interface IdleSpanOptions { * Defaults to `false`. */ disableAutoFinish?: boolean; + /** Allows to configure a hook that is called when the idle span is ended, before it is processed. */ beforeSpanEnd?: (span: Span) => void; + + /** + * If set to `true`, the idle span will be trimmed to the latest span end timestamp of its children. + * + * @default `true`. + */ + trimIdleSpanEndTimestamp?: boolean; } /** @@ -108,6 +116,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti finalTimeout = TRACING_DEFAULTS.finalTimeout, childSpanTimeout = TRACING_DEFAULTS.childSpanTimeout, beforeSpanEnd, + trimIdleSpanEndTimestamp = true, } = options; const client = getClient(); @@ -151,8 +160,11 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti // Ensure we end with the last span timestamp, if possible const spans = getSpanDescendants(span).filter(child => child !== span); + const spanJson = spanToJSON(span); + // If we have no spans, we just end, nothing else to do here - if (!spans.length) { + // Likewise, if users explicitly ended the span, we simply end the span without timestamp adjustment + if (!spans.length || !trimIdleSpanEndTimestamp) { onIdleSpanEnded(spanEndTimestamp); return Reflect.apply(target, thisArg, [spanEndTimestamp, ...rest]); } @@ -174,7 +186,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti }, undefined); // In reality this should always exist here, but type-wise it may be undefined... - const spanStartTimestamp = spanToJSON(span).start_timestamp; + const spanStartTimestamp = spanJson.start_timestamp; // The final endTimestamp should: // * Never be before the span start timestamp