Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 4 additions & 1 deletion packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -190,7 +192,7 @@ export class BrowserTracing implements Integration {
this.options.tracePropagationTargets = _options.tracingOrigins;
}

startTrackingWebVitals();
this._collectWebVitals = startTrackingWebVitals();
if (this.options.enableLongTask) {
startTrackingLongTasks();
}
Expand Down Expand Up @@ -311,6 +313,7 @@ export class BrowserTracing implements Integration {
heartbeatInterval,
);
idleTransaction.registerBeforeFinishCallback(transaction => {
this._collectWebVitals();
addPerformanceEntries(transaction);
});

Expand Down
27 changes: 20 additions & 7 deletions packages/tracing/src/browser/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
14 changes: 10 additions & 4 deletions packages/tracing/src/browser/web-vitals/getCLS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>;

Expand Down Expand Up @@ -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;
};
8 changes: 6 additions & 2 deletions packages/tracing/src/browser/web-vitals/getLCP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {};

Expand All @@ -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>;
Expand Down Expand Up @@ -75,5 +75,9 @@ export const onLCP = (onReport: ReportCallback): void => {
});

onHidden(stopListening, true);

return stopListening;
}

return;
};
2 changes: 2 additions & 0 deletions packages/tracing/src/browser/web-vitals/types/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
22 changes: 21 additions & 1 deletion packages/tracing/test/browser/browsertracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -57,6 +64,8 @@ describe('BrowserTracing', () => {
hub = new Hub(new BrowserClient(options));
makeMain(hub);
document.head.innerHTML = '';

mockStartTrackingWebVitals.mockClear();
});

afterEach(() => {
Expand Down Expand Up @@ -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', () => {
Expand Down