From 5cc3c9058d4f9672c4f58c34ae22a46ae6a8a544 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 10 Jul 2025 12:56:38 +0200 Subject: [PATCH] fix(browser): Ensure standalone CLS and LCP spans have traceId of pageload span --- .../web-vitals-cls-standalone-spans/test.ts | 12 +++++++---- .../web-vitals-lcp-standalone-spans/test.ts | 20 +++++++++++++------ packages/browser-utils/src/metrics/cls.ts | 2 +- packages/browser-utils/src/metrics/lcp.ts | 2 +- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts index 3db30586c909..b5c47fdd3ab7 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -348,10 +348,10 @@ sentryTest( sentryTest('sends CLS of the initial page when soft-navigating to a new page', async ({ getLocalTestUrl, page }) => { const url = await getLocalTestUrl({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + const pageloadEventData = await getFirstSentryEnvelopeRequest(page, url); - expect(eventData.type).toBe('transaction'); - expect(eventData.contexts?.trace?.op).toBe('pageload'); + expect(pageloadEventData.type).toBe('transaction'); + expect(pageloadEventData.contexts?.trace?.op).toBe('pageload'); const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( page, @@ -364,12 +364,16 @@ sentryTest('sends CLS of the initial page when soft-navigating to a new page', a await page.goto(`${url}#soft-navigation`); + const pageloadTraceId = pageloadEventData.contexts?.trace?.trace_id; + expect(pageloadTraceId).toMatch(/[a-f0-9]{32}/); + const spanEnvelope = (await spanEnvelopePromise)[0]; const spanEnvelopeItem = spanEnvelope[1][0][1]; // Flakey value dependent on timings -> we check for a range expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); - expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toMatch(/[a-f0-9]{16}/); + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadEventData.contexts?.trace?.span_id); + expect(spanEnvelopeItem.trace_id).toEqual(pageloadTraceId); }); sentryTest("doesn't send further CLS after the first navigation", async ({ getLocalTestUrl, page }) => { diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-standalone-spans/test.ts index 9ced3b2dee07..3310c7b95004 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-standalone-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-standalone-spans/test.ts @@ -3,10 +3,12 @@ import { expect } from '@playwright/test'; import type { Event as SentryEvent, EventEnvelope, SpanEnvelope } from '@sentry/core'; import { sentryTest } from '../../../../utils/fixtures'; import { + envelopeRequestParser, getFirstSentryEnvelopeRequest, getMultipleSentryEnvelopeRequests, properFullEnvelopeRequestParser, shouldSkipTracingTest, + waitForTransactionRequest, } from '../../../../utils/helpers'; sentryTest.beforeEach(async ({ browserName, page }) => { @@ -31,6 +33,8 @@ sentryTest('captures LCP vital as a standalone span', async ({ getLocalTestUrl, properFullEnvelopeRequestParser, ); + const pageloadEnvelopePromise = waitForTransactionRequest(page, e => e.contexts?.trace?.op === 'pageload'); + page.route('**', route => route.continue()); page.route('**/my/image.png', async (route: Route) => { return route.fulfill({ @@ -47,10 +51,14 @@ sentryTest('captures LCP vital as a standalone span', async ({ getLocalTestUrl, await hidePage(page); const spanEnvelope = (await spanEnvelopePromise)[0]; + const pageloadTransactionEvent = envelopeRequestParser(await pageloadEnvelopePromise); const spanEnvelopeHeaders = spanEnvelope[0]; const spanEnvelopeItem = spanEnvelope[1][0][1]; + const pageloadTraceId = pageloadTransactionEvent.contexts?.trace?.trace_id; + expect(pageloadTraceId).toMatch(/[a-f0-9]{32}/); + expect(spanEnvelopeItem).toEqual({ data: { 'sentry.exclusive_time': 0, @@ -80,7 +88,7 @@ sentryTest('captures LCP vital as a standalone span', async ({ getLocalTestUrl, segment_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), timestamp: spanEnvelopeItem.start_timestamp, // LCP is a point-in-time metric - trace_id: expect.stringMatching(/[a-f0-9]{32}/), + trace_id: pageloadTraceId, }); // LCP value should be greater than 0 @@ -95,7 +103,6 @@ sentryTest('captures LCP vital as a standalone span', async ({ getLocalTestUrl, sampled: 'true', trace_id: spanEnvelopeItem.trace_id, sample_rand: expect.any(String), - // no transaction, because span source is URL }, }); }); @@ -152,10 +159,10 @@ sentryTest('sends LCP of the initial page when soft-navigating to a new page', a const url = await getLocalTestUrl({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + const pageloadEventData = await getFirstSentryEnvelopeRequest(page, url); - expect(eventData.type).toBe('transaction'); - expect(eventData.contexts?.trace?.op).toBe('pageload'); + expect(pageloadEventData.type).toBe('transaction'); + expect(pageloadEventData.contexts?.trace?.op).toBe('pageload'); const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( page, @@ -173,7 +180,8 @@ sentryTest('sends LCP of the initial page when soft-navigating to a new page', a const spanEnvelopeItem = spanEnvelope[1][0][1]; expect(spanEnvelopeItem.measurements?.lcp?.value).toBeGreaterThan(0); - expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toMatch(/[a-f0-9]{16}/); + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadEventData.contexts?.trace?.span_id); + expect(spanEnvelopeItem.trace_id).toBe(pageloadEventData.contexts?.trace?.trace_id); }); sentryTest("doesn't send further LCP after the first navigation", async ({ getLocalTestUrl, page }) => { diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index dd3e1b11bd7b..b564a0187818 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -72,7 +72,7 @@ export function trackClsAsStandaloneSpan(): void { return; } - const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => { + const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => { // we only want to collect LCP if we actually navigate. Redirects should be ignored. if (!options?.isRedirect) { _collectClsOnce(); diff --git a/packages/browser-utils/src/metrics/lcp.ts b/packages/browser-utils/src/metrics/lcp.ts index 6519ced374ec..e34391a4a1d7 100644 --- a/packages/browser-utils/src/metrics/lcp.ts +++ b/packages/browser-utils/src/metrics/lcp.ts @@ -72,7 +72,7 @@ export function trackLcpAsStandaloneSpan(): void { return; } - const unsubscribeStartNavigation = client.on('startNavigationSpan', (_, options) => { + const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => { // we only want to collect LCP if we actually navigate. Redirects should be ignored. if (!options?.isRedirect) { _collectLcpOnce();