fix(browser): Ensure standalone CLS and LCP spans have traceId of pageload span (#16864)

Fix a bug in standalone web vital spans where, if they were
sent because of a navigation, they would incorrectly be added to the
`navigation` span's trace instead of the initial `pageload` span's
trace. This surfaced while dogfooding these spans on sentry and
inspecting them in the EAP traceview.

To fix this, I added a lifecycle hook in #16863 that triggers right
before we start preparing the navigation span and hence, right before we
recycle the propagation context to the new traceId. This patch now makes
use of the new hook for CLS and LCP spans and it also makes the
integration tests stricter to check for the correct trace ids.

Author: Lms24

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<SentryEvent>(page, url);
+  const pageloadEventData = await getFirstSentryEnvelopeRequest<SentryEvent>(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<SpanEnvelope>(
     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 }) => {

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<SentryEvent>(page, url);
+  const pageloadEventData = await getFirstSentryEnvelopeRequest<SentryEvent>(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<SpanEnvelope>(
     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 }) => {

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();

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();