feat(perf): Add experimental option to improve LCP collection (#3879)

This will add an experimental option (options._metricOptions._reportAllChanges) that will set 'getLCP' to always report. Now that we are recording LCP element, it should be sufficient to collection the additional information and let the user pick the appropriate LCP element in the ui to improve accuracy dynamically instead. We'd like to test this out on Sentry first.

Co-authored-by: Abhijeet Prasad <[email protected]>

Author: k-fish

packages/tracing/src/browser/browsertracing.ts

@@ -7,7 +7,7 @@ import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../idletransaction';
 import { SpanStatus } from '../spanstatus';
 import { extractTraceparentData, secToMs } from '../utils';
 import { registerBackgroundTabDetection } from './backgroundtab';
-import { MetricsInstrumentation } from './metrics';
+import { DEFAULT_METRICS_INSTR_OPTIONS, MetricsInstrumentation, MetricsInstrumentationOptions } from './metrics';
 import {
   defaultRequestInstrumentationOptions,
   instrumentOutgoingRequests,
@@ -60,6 +60,15 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
    */
   markBackgroundTransactions: boolean;
 
+  /**
+   * _metricOptions allows the user to send options to change how metrics are collected.
+   *
+   * _metricOptions is currently experimental.
+   *
+   * Default: undefined
+   */
+  _metricOptions?: Partial<MetricsInstrumentationOptions>;
+
   /**
    * beforeNavigate is called before a pageload/navigation transaction is created and allows users to modify transaction
    * context data, or drop the transaction entirely (by setting `sampled = false` in the context).
@@ -116,7 +125,7 @@ export class BrowserTracing implements Integration {
 
   private _getCurrentHub?: () => Hub;
 
-  private readonly _metrics: MetricsInstrumentation = new MetricsInstrumentation();
+  private readonly _metrics: MetricsInstrumentation;
 
   private readonly _emitOptionsWarning: boolean = false;
 
@@ -139,6 +148,8 @@ export class BrowserTracing implements Integration {
       ..._options,
       tracingOrigins,
     };
+
+    this._metrics = new MetricsInstrumentation({ ...DEFAULT_METRICS_INSTR_OPTIONS, ...this.options._metricOptions });
   }
 
   /**

packages/tracing/src/browser/metrics.ts

@@ -14,6 +14,17 @@ import { NavigatorDeviceMemory, NavigatorNetworkInformation } from './web-vitals
 
 const global = getGlobalObject<Window>();
 
+/**
+ * Exports a way to add options to our metric collection. Currently experimental.
+ */
+export interface MetricsInstrumentationOptions {
+  _reportAllChanges: boolean;
+}
+
+export const DEFAULT_METRICS_INSTR_OPTIONS: MetricsInstrumentationOptions = {
+  _reportAllChanges: false,
+};
+
 /** Class tracking metrics  */
 export class MetricsInstrumentation {
   private _measurements: Measurements = {};
@@ -22,14 +33,14 @@ export class MetricsInstrumentation {
   private _lcpEntry: LargestContentfulPaint | undefined;
   private _clsEntry: LayoutShift | undefined;
 
-  public constructor() {
+  public constructor(_options: MetricsInstrumentationOptions) {
     if (!isNodeEnv() && global?.performance && global?.document) {
       if (global.performance.mark) {
         global.performance.mark('sentry-tracing-init');
       }
 
       this._trackCLS();
-      this._trackLCP();
+      this._trackLCP(_options._reportAllChanges);
       this._trackFID();
     }
   }
@@ -285,7 +296,7 @@ export class MetricsInstrumentation {
   }
 
   /** Starts tracking the Largest Contentful Paint on the current page. */
-  private _trackLCP(): void {
+  private _trackLCP(reportAllChanges: boolean): void {
     getLCP(metric => {
       const entry = metric.entries.pop();
 
@@ -299,7 +310,7 @@ export class MetricsInstrumentation {
       this._measurements['lcp'] = { value: metric.value };
       this._measurements['mark.lcp'] = { value: timeOrigin + startTime };
       this._lcpEntry = entry as LargestContentfulPaint;
-    });
+    }, reportAllChanges);
   }
 
   /** Starts tracking the First Input Delay on the current page. */

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

@@ -11,6 +11,7 @@ import {
   getHeaderContext,
   getMetaContent,
 } from '../../src/browser/browsertracing';
+import { DEFAULT_METRICS_INSTR_OPTIONS, MetricsInstrumentation } from '../../src/browser/metrics';
 import { defaultRequestInstrumentationOptions } from '../../src/browser/request';
 import { instrumentRoutingWithDefaults } from '../../src/browser/router';
 import * as hubExtensions from '../../src/hubextensions';
@@ -32,6 +33,8 @@ jest.mock('@sentry/utils', () => {
   };
 });
 
+jest.mock('../../src/browser/metrics');
+
 const { logger } = jest.requireActual('@sentry/utils');
 const warnSpy = jest.spyOn(logger, 'warn');
 
@@ -493,4 +496,31 @@ describe('BrowserTracing', () => {
       );
     });
   });
+
+  describe('metrics', () => {
+    beforeEach(() => {
+      // @ts-ignore mock clear
+      MetricsInstrumentation.mockClear();
+    });
+
+    it('creates metrics instrumentation', () => {
+      createBrowserTracing(true, {});
+
+      expect(MetricsInstrumentation).toHaveBeenCalledTimes(1);
+      expect(MetricsInstrumentation).toHaveBeenLastCalledWith(DEFAULT_METRICS_INSTR_OPTIONS);
+    });
+
+    it('creates metrics instrumentation with custom options', () => {
+      createBrowserTracing(true, {
+        _metricOptions: {
+          _reportAllChanges: true,
+        },
+      });
+
+      expect(MetricsInstrumentation).toHaveBeenCalledTimes(1);
+      expect(MetricsInstrumentation).toHaveBeenLastCalledWith({
+        _reportAllChanges: true,
+      });
+    });
+  });
 });

packages/tracing/test/browser/metrics.test.ts

@@ -1,5 +1,11 @@
 import { Span, Transaction } from '../../src';
-import { _startChild, addResourceSpans, MetricsInstrumentation, ResourceEntry } from '../../src/browser/metrics';
+import {
+  _startChild,
+  addResourceSpans,
+  DEFAULT_METRICS_INSTR_OPTIONS,
+  MetricsInstrumentation,
+  ResourceEntry,
+} from '../../src/browser/metrics';
 import { addDOMPropertiesToGlobal } from '../testutils';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-var
@@ -182,7 +188,7 @@ describe('MetricsInstrumentation', () => {
       jest.spyOn(MetricsInstrumentation.prototype as any, tracker),
     );
 
-    new MetricsInstrumentation();
+    new MetricsInstrumentation(DEFAULT_METRICS_INSTR_OPTIONS);
 
     trackers.forEach(tracker => expect(tracker).not.toBeCalled());
   });
@@ -196,8 +202,7 @@ describe('MetricsInstrumentation', () => {
     const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker =>
       jest.spyOn(MetricsInstrumentation.prototype as any, tracker),
     );
-
-    new MetricsInstrumentation();
+    new MetricsInstrumentation(DEFAULT_METRICS_INSTR_OPTIONS);
     global.process = backup;
 
     trackers.forEach(tracker => expect(tracker).toBeCalled());