fix(tracing): Record LCP and CLS on transaction finish (#7386)

Author: AbhiPrasad

packages/integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts

@@ -17,8 +17,6 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN
   const url = await getLocalTestPath({ testDir: __dirname });
   await page.goto(url);
 
-  // Force closure of LCP listener.
-  await page.click('body');
   const eventData = await getFirstSentryEnvelopeRequest<Event>(page);
 
   expect(eventData.measurements).toBeDefined();

packages/tracing/src/browser/browsertracing.ts

@@ -169,6 +169,8 @@ export class BrowserTracing implements Integration {
   private _latestRouteName?: string;
   private _latestRouteSource?: TransactionSource;
 
+  private _collectWebVitals: () => void;
+
   public constructor(_options?: Partial<BrowserTracingOptions>) {
     this.options = {
       ...DEFAULT_BROWSER_TRACING_OPTIONS,
@@ -190,7 +192,7 @@ export class BrowserTracing implements Integration {
       this.options.tracePropagationTargets = _options.tracingOrigins;
     }
 
-    startTrackingWebVitals();
+    this._collectWebVitals = startTrackingWebVitals();
     if (this.options.enableLongTask) {
       startTrackingLongTasks();
     }
@@ -311,6 +313,7 @@ export class BrowserTracing implements Integration {
       heartbeatInterval,
     );
     idleTransaction.registerBeforeFinishCallback(transaction => {
+      this._collectWebVitals();
       addPerformanceEntries(transaction);
     });
 

packages/tracing/src/browser/metrics/index.ts

@@ -33,17 +33,30 @@ let _clsEntry: LayoutShift | undefined;
 
 /**
  * Start tracking web vitals
+ *
+ * @returns A function that forces web vitals collection
  */
-export function startTrackingWebVitals(): void {
+export function startTrackingWebVitals(): () => void {
   const performance = getBrowserPerformanceAPI();
   if (performance && browserPerformanceTimeOrigin) {
     if (performance.mark) {
       WINDOW.performance.mark('sentry-tracing-init');
     }
-    _trackCLS();
-    _trackLCP();
     _trackFID();
+    const clsCallback = _trackCLS();
+    const lcpCallback = _trackLCP();
+
+    return (): void => {
+      if (clsCallback) {
+        clsCallback();
+      }
+      if (lcpCallback) {
+        lcpCallback();
+      }
+    };
   }
+
+  return () => undefined;
 }
 
 /**
@@ -100,11 +113,11 @@ export function startTrackingInteractions(): void {
 }
 
 /** Starts tracking the Cumulative Layout Shift on the current page. */
-function _trackCLS(): void {
+function _trackCLS(): ReturnType<typeof onCLS> {
   // See:
   // https://web.dev/evolving-cls/
   // https://web.dev/cls-web-tooling/
-  onCLS(metric => {
+  return onCLS(metric => {
     const entry = metric.entries.pop();
     if (!entry) {
       return;
@@ -117,8 +130,8 @@ function _trackCLS(): void {
 }
 
 /** Starts tracking the Largest Contentful Paint on the current page. */
-function _trackLCP(): void {
-  onLCP(metric => {
+function _trackLCP(): ReturnType<typeof onLCP> {
+  return onLCP(metric => {
     const entry = metric.entries.pop();
     if (!entry) {
       return;

packages/tracing/src/browser/web-vitals/getCLS.ts

@@ -18,7 +18,7 @@ import { bindReporter } from './lib/bindReporter';
 import { initMetric } from './lib/initMetric';
 import { observe } from './lib/observe';
 import { onHidden } from './lib/onHidden';
-import type { CLSMetric, ReportCallback } from './types';
+import type { CLSMetric, ReportCallback, StopListening } from './types';
 
 /**
  * Calculates the [CLS](https://web.dev/cls/) value for the current page and
@@ -41,7 +41,7 @@ import type { CLSMetric, ReportCallback } from './types';
  * hidden. As a result, the `callback` function might be called multiple times
  * during the same page load._
  */
-export const onCLS = (onReport: ReportCallback): void => {
+export const onCLS = (onReport: ReportCallback): StopListening | undefined => {
   const metric = initMetric('CLS', 0);
   let report: ReturnType<typeof bindReporter>;
 
@@ -89,9 +89,15 @@ export const onCLS = (onReport: ReportCallback): void => {
   if (po) {
     report = bindReporter(onReport, metric);
 
-    onHidden(() => {
+    const stopListening = (): void => {
       handleEntries(po.takeRecords() as CLSMetric['entries']);
       report(true);
-    });
+    };
+
+    onHidden(stopListening);
+
+    return stopListening;
   }
+
+  return;
 };

packages/tracing/src/browser/web-vitals/getLCP.ts

@@ -20,7 +20,7 @@ import { getVisibilityWatcher } from './lib/getVisibilityWatcher';
 import { initMetric } from './lib/initMetric';
 import { observe } from './lib/observe';
 import { onHidden } from './lib/onHidden';
-import type { LCPMetric, ReportCallback } from './types';
+import type { LCPMetric, ReportCallback, StopListening } from './types';
 
 const reportedMetricIDs: Record<string, boolean> = {};
 
@@ -30,7 +30,7 @@ const reportedMetricIDs: Record<string, boolean> = {};
  * relevant `largest-contentful-paint` performance entry used to determine the
  * value). The reported value is a `DOMHighResTimeStamp`.
  */
-export const onLCP = (onReport: ReportCallback): void => {
+export const onLCP = (onReport: ReportCallback): StopListening | undefined => {
   const visibilityWatcher = getVisibilityWatcher();
   const metric = initMetric('LCP');
   let report: ReturnType<typeof bindReporter>;
@@ -75,5 +75,9 @@ export const onLCP = (onReport: ReportCallback): void => {
     });
 
     onHidden(stopListening, true);
+
+    return stopListening;
   }
+
+  return;
 };

packages/tracing/src/browser/web-vitals/types/base.ts

@@ -104,3 +104,5 @@ export interface ReportOpts {
  *   loading. This is equivalent to the corresponding `readyState` value.
  */
 export type LoadState = 'loading' | 'dom-interactive' | 'dom-content-loaded' | 'complete';
+
+export type StopListening = () => void;

packages/tracing/test/browser/browsertracing.test.ts

@@ -28,7 +28,14 @@ jest.mock('@sentry/utils', () => {
   };
 });
 
-jest.mock('../../src/browser/metrics');
+const mockStartTrackingWebVitals = jest.fn().mockReturnValue(() => () => {});
+
+jest.mock('../../src/browser/metrics', () => ({
+  addPerformanceEntries: jest.fn(),
+  startTrackingInteractions: jest.fn(),
+  startTrackingLongTasks: jest.fn(),
+  startTrackingWebVitals: () => mockStartTrackingWebVitals(),
+}));
 
 const instrumentOutgoingRequestsMock = jest.fn();
 jest.mock('./../../src/browser/request', () => {
@@ -57,6 +64,8 @@ describe('BrowserTracing', () => {
     hub = new Hub(new BrowserClient(options));
     makeMain(hub);
     document.head.innerHTML = '';
+
+    mockStartTrackingWebVitals.mockClear();
   });
 
   afterEach(() => {
@@ -371,6 +380,17 @@ describe('BrowserTracing', () => {
         jest.advanceTimersByTime(2000);
         expect(mockFinish).toHaveBeenCalledTimes(1);
       });
+
+      it('calls `_collectWebVitals` if enabled', () => {
+        createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting });
+        const transaction = getActiveTransaction(hub) as IdleTransaction;
+
+        const span = transaction.startChild(); // activities = 1
+        span.finish(); // activities = 0
+
+        jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout);
+        expect(mockStartTrackingWebVitals).toHaveBeenCalledTimes(1);
+      });
     });
 
     describe('heartbeatInterval', () => {