refactor(browser): consolidate web vitals tracking

Author: logaretm

packages/browser-utils/src/index.ts

@@ -8,19 +8,18 @@ export {
 
 export {
   addPerformanceEntries,
+  registerInpInteractionListener,
   startTrackingInteractions,
   startTrackingLongTasks,
   startTrackingLongAnimationFrames,
   startTrackingWebVitals,
-  registerInpInteractionListener,
 } from './metrics/browserMetrics';
+export type { WebVitalName } from './metrics/browserMetrics';
 
 export { elementTimingIntegration, startTrackingElementTiming } from './metrics/elementTiming';
 
 export { extractNetworkProtocol } from './metrics/utils';
 
-export { trackClsAsSpan, trackInpAsSpan, trackLcpAsSpan } from './metrics/webVitalSpans';
-
 export { addClickKeypressInstrumentationHandler } from './instrument/dom';
 
 export { addHistoryInstrumentationHandler } from './instrument/history';

packages/browser-utils/src/metrics/browserMetrics.ts

@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */
-import type { Measurements, Span, SpanAttributes, SpanAttributeValue, StartSpanOptions } from '@sentry/core';
+import type { Client, Measurements, Span, SpanAttributes, SpanAttributeValue, StartSpanOptions } from '@sentry/core';
 import {
   browserPerformanceTimeOrigin,
   debug,
@@ -23,6 +23,7 @@ import { getBrowserPerformanceAPI, isMeasurementValue, msToSec, startAndEndSpan
 import { getActivationStart } from './web-vitals/lib/getActivationStart';
 import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry';
 import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher';
+import { trackClsAsSpan, trackInpAsSpan, trackLcpAsSpan } from './webVitalSpans';
 import { DEBUG_BUILD } from '../debug-build';
 interface NavigatorNetworkInformation {
   readonly connection?: NetworkInformation;
@@ -69,6 +70,7 @@ let _performanceCursor: number = 0;
 let _measurements: Measurements = {};
 
 type PageloadWebVitalName = 'ttfb' | 'fp' | 'fcp';
+export type WebVitalName = PageloadWebVitalName | 'lcp' | 'cls' | 'inp';
 
 const DEFAULT_PAGELOAD_WEB_VITALS = new Set<PageloadWebVitalName>(['ttfb', 'fp', 'fcp']);
 
@@ -78,17 +80,26 @@ let _collectTtfb: (() => void) | undefined;
 
 /**
  * Start tracking web vitals.
- *
- * LCP, CLS and INP are handled separately as spans by `webVitalsIntegration`;
- * this function tracks pageload web vitals which are attached as attributes.
  */
-export function startTrackingWebVitals(options: { disable?: Array<'ttfb' | 'fp' | 'fcp'> } = {}): void {
+export function startTrackingWebVitals(client: Client, disabled: ReadonlySet<WebVitalName>): void {
+  startTrackingPageloadWebVitals(disabled);
+
+  if (!disabled.has('lcp')) {
+    trackLcpAsSpan(client);
+  }
+  if (!disabled.has('cls')) {
+    trackClsAsSpan(client);
+  }
+  if (!disabled.has('inp')) {
+    trackInpAsSpan();
+  }
+}
+
+function startTrackingPageloadWebVitals(disabled: ReadonlySet<WebVitalName>): void {
   _collectTtfb?.();
   _collectTtfb = undefined;
 
-  _enabledPageloadWebVitals = new Set(
-    Array.from(DEFAULT_PAGELOAD_WEB_VITALS).filter(vital => !options.disable?.includes(vital)),
-  );
+  _enabledPageloadWebVitals = new Set(Array.from(DEFAULT_PAGELOAD_WEB_VITALS).filter(vital => !disabled.has(vital)));
 
   const performance = getBrowserPerformanceAPI();
   if (performance && browserPerformanceTimeOrigin()) {

packages/browser/src/integrations/webVitals.ts

@@ -1,17 +1,9 @@
 import type { IntegrationFn } from '@sentry/core';
 import { debug, defineIntegration, hasSpansEnabled } from '@sentry/core';
-import {
-  startTrackingWebVitals,
-  registerInpInteractionListener,
-  trackClsAsSpan,
-  trackInpAsSpan,
-  trackLcpAsSpan,
-} from '@sentry-internal/browser-utils';
+import { registerInpInteractionListener, startTrackingWebVitals } from '@sentry-internal/browser-utils';
+import type { WebVitalName } from '@sentry-internal/browser-utils';
 import { DEBUG_BUILD } from '../debug-build';
 
-type WebVitalName = 'lcp' | 'cls' | 'inp' | 'ttfb' | 'fp' | 'fcp';
-type PageloadWebVitalName = Extract<WebVitalName, 'ttfb' | 'fp' | 'fcp'>;
-
 interface WebVitalsOptions {
   /**
    * How web vitals are emitted.
@@ -42,7 +34,6 @@ export const INTEGRATION_NAME = 'WebVitals';
  */
 export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions = {}) => {
   const disabled = new Set(options.disable ?? []);
-  const disabledPageloadWebVitals = options.disable?.filter(isPageloadWebVitalName);
 
   return {
     name: INTEGRATION_NAME,
@@ -54,19 +45,7 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
           );
       }
 
-      startTrackingWebVitals({
-        disable: disabledPageloadWebVitals,
-      });
-
-      if (!disabled.has('lcp')) {
-        trackLcpAsSpan(client);
-      }
-      if (!disabled.has('cls')) {
-        trackClsAsSpan(client);
-      }
-      if (!disabled.has('inp')) {
-        trackInpAsSpan();
-      }
+      startTrackingWebVitals(client, disabled);
     },
     afterAllSetup() {
       if (!disabled.has('inp')) {
@@ -75,7 +54,3 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
     },
   };
 }) satisfies IntegrationFn;
-
-function isPageloadWebVitalName(vital: WebVitalName): vital is PageloadWebVitalName {
-  return vital === 'ttfb' || vital === 'fp' || vital === 'fcp';
-}

packages/browser/test/integrations/webVitals.test.ts

@@ -4,16 +4,10 @@ import { webVitalsIntegration } from '../../src/integrations/webVitals';
 
 const mockRegisterInpInteractionListener = vi.hoisted(() => vi.fn());
 const mockStartTrackingWebVitals = vi.hoisted(() => vi.fn());
-const mockTrackClsAsSpan = vi.hoisted(() => vi.fn());
-const mockTrackInpAsSpan = vi.hoisted(() => vi.fn());
-const mockTrackLcpAsSpan = vi.hoisted(() => vi.fn());
 
 vi.mock('@sentry-internal/browser-utils', () => ({
   registerInpInteractionListener: mockRegisterInpInteractionListener,
   startTrackingWebVitals: mockStartTrackingWebVitals,
-  trackClsAsSpan: mockTrackClsAsSpan,
-  trackInpAsSpan: mockTrackInpAsSpan,
-  trackLcpAsSpan: mockTrackLcpAsSpan,
 }));
 
 describe('webVitalsIntegration', () => {
@@ -35,12 +29,8 @@ describe('webVitalsIntegration', () => {
     integration.setup?.(client as never);
     integration.afterAllSetup?.(client as never);
 
-    expect(mockStartTrackingWebVitals).toHaveBeenCalledWith({
-      disable: undefined,
-    });
-    expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client);
-    expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client);
-    expect(mockTrackInpAsSpan).toHaveBeenCalledTimes(1);
+    expect(mockStartTrackingWebVitals).toHaveBeenCalledWith(client, expect.any(Set));
+    expect(mockStartTrackingWebVitals.mock.calls[0]?.[1]).toEqual(new Set());
     expect(mockRegisterInpInteractionListener).toHaveBeenCalledTimes(1);
     expect(debug.warn).not.toHaveBeenCalled();
   });
@@ -54,12 +44,8 @@ describe('webVitalsIntegration', () => {
     integration.setup?.(client as never);
     integration.afterAllSetup?.(client as never);
 
-    expect(mockStartTrackingWebVitals).toHaveBeenCalledWith({
-      disable: ['ttfb', 'fcp'],
-    });
-    expect(mockTrackLcpAsSpan).not.toHaveBeenCalled();
-    expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client);
-    expect(mockTrackInpAsSpan).not.toHaveBeenCalled();
+    expect(mockStartTrackingWebVitals).toHaveBeenCalledWith(client, expect.any(Set));
+    expect(mockStartTrackingWebVitals.mock.calls[0]?.[1]).toEqual(new Set(['ttfb', 'fcp', 'lcp', 'inp']));
     expect(mockRegisterInpInteractionListener).not.toHaveBeenCalled();
   });