Skip to content

Commit c94d2a9

Browse files
authored
fix(browser): Use current start timestamp for CLS span when CLS is 0 (#17800)
When there's no layout shift, the standalone CLS span would previously have the same start time as the pageload span (`performance.timeOrigin`). This caused the cls span to be incorrectly interpreted as the trace root in the Sentry UI. We can fix this by setting the span start timestamp as the startTime instead if there's no layout shift. In all other cases, we let the span start at the time the last CLS update occured.
1 parent 5ce4435 commit c94d2a9

File tree

2 files changed

+239
-4
lines changed

2 files changed

+239
-4
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE,
1010
SEMANTIC_ATTRIBUTE_SENTRY_OP,
1111
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
12+
timestampInSeconds,
1213
} from '@sentry/core';
1314
import { DEBUG_BUILD } from '../debug-build';
1415
import { addClsInstrumentationHandler } from './instrument';
@@ -42,28 +43,31 @@ export function trackClsAsStandaloneSpan(client: Client): void {
4243
}, true);
4344

4445
listenForWebVitalReportEvents(client, (reportEvent, pageloadSpanId) => {
45-
sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId, reportEvent);
46+
_sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId, reportEvent);
4647
cleanupClsHandler();
4748
});
4849
}
4950

50-
function sendStandaloneClsSpan(
51+
/**
52+
* Exported only for testing!
53+
*/
54+
export function _sendStandaloneClsSpan(
5155
clsValue: number,
5256
entry: LayoutShift | undefined,
5357
pageloadSpanId: string,
5458
reportEvent: WebVitalReportEvent,
5559
) {
5660
DEBUG_BUILD && debug.log(`Sending CLS span (${clsValue})`);
5761

58-
const startTime = msToSec((browserPerformanceTimeOrigin() || 0) + (entry?.startTime || 0));
62+
const startTime = entry ? msToSec((browserPerformanceTimeOrigin() || 0) + entry.startTime) : timestampInSeconds();
5963
const routeName = getCurrentScope().getScopeData().transactionName;
6064

6165
const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';
6266

6367
const attributes: SpanAttributes = {
6468
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
6569
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
66-
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
70+
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: 0,
6771
// attach the pageload span id to the CLS span so that we can link them in the UI
6872
'sentry.pageload.span_id': pageloadSpanId,
6973
// describes what triggered the web vital to be reported
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
import * as SentryCore from '@sentry/core';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3+
import { _sendStandaloneClsSpan } from '../../src/metrics/cls';
4+
import * as WebVitalUtils from '../../src/metrics/utils';
5+
6+
// Mock all Sentry core dependencies
7+
vi.mock('@sentry/core', async () => {
8+
const actual = await vi.importActual('@sentry/core');
9+
return {
10+
...actual,
11+
browserPerformanceTimeOrigin: vi.fn(),
12+
timestampInSeconds: vi.fn(),
13+
getCurrentScope: vi.fn(),
14+
htmlTreeAsString: vi.fn(),
15+
};
16+
});
17+
18+
describe('_sendStandaloneClsSpan', () => {
19+
const mockSpan = {
20+
addEvent: vi.fn(),
21+
end: vi.fn(),
22+
};
23+
24+
const mockScope = {
25+
getScopeData: vi.fn().mockReturnValue({
26+
transactionName: 'test-transaction',
27+
}),
28+
};
29+
30+
afterEach(() => {
31+
vi.clearAllMocks();
32+
});
33+
34+
beforeEach(() => {
35+
vi.mocked(SentryCore.getCurrentScope).mockReturnValue(mockScope as any);
36+
vi.mocked(SentryCore.browserPerformanceTimeOrigin).mockReturnValue(1000);
37+
vi.mocked(SentryCore.timestampInSeconds).mockReturnValue(1.5);
38+
vi.mocked(SentryCore.htmlTreeAsString).mockImplementation((node: any) => `<${node?.tagName || 'div'}>`);
39+
vi.spyOn(WebVitalUtils, 'startStandaloneWebVitalSpan').mockReturnValue(mockSpan as any);
40+
});
41+
42+
it('sends a standalone CLS span with entry data', () => {
43+
const clsValue = 0.1;
44+
const mockEntry: LayoutShift = {
45+
name: 'layout-shift',
46+
entryType: 'layout-shift',
47+
startTime: 100,
48+
duration: 0,
49+
value: clsValue,
50+
hadRecentInput: false,
51+
sources: [
52+
// @ts-expect-error - other properties are irrelevant
53+
{
54+
node: { tagName: 'div' } as Element,
55+
},
56+
],
57+
toJSON: vi.fn(),
58+
};
59+
const pageloadSpanId = '123';
60+
const reportEvent = 'navigation';
61+
62+
_sendStandaloneClsSpan(clsValue, mockEntry, pageloadSpanId, reportEvent);
63+
64+
expect(WebVitalUtils.startStandaloneWebVitalSpan).toHaveBeenCalledWith({
65+
name: '<div>',
66+
transaction: 'test-transaction',
67+
attributes: {
68+
'sentry.origin': 'auto.http.browser.cls',
69+
'sentry.op': 'ui.webvital.cls',
70+
'sentry.exclusive_time': 0,
71+
'sentry.pageload.span_id': '123',
72+
'sentry.report_event': 'navigation',
73+
'cls.source.1': '<div>',
74+
},
75+
startTime: 1.1, // (1000 + 100) / 1000
76+
});
77+
78+
expect(mockSpan.addEvent).toHaveBeenCalledWith('cls', {
79+
'sentry.measurement_unit': '',
80+
'sentry.measurement_value': 0.1,
81+
});
82+
83+
expect(mockSpan.end).toHaveBeenCalledWith(1.1);
84+
});
85+
86+
it('sends a standalone CLS span without entry data', () => {
87+
const clsValue = 0;
88+
const pageloadSpanId = '456';
89+
const reportEvent = 'pagehide';
90+
91+
_sendStandaloneClsSpan(clsValue, undefined, pageloadSpanId, reportEvent);
92+
93+
expect(SentryCore.timestampInSeconds).toHaveBeenCalled();
94+
expect(SentryCore.browserPerformanceTimeOrigin).not.toHaveBeenCalled();
95+
96+
expect(WebVitalUtils.startStandaloneWebVitalSpan).toHaveBeenCalledWith({
97+
name: 'Layout shift',
98+
transaction: 'test-transaction',
99+
attributes: {
100+
'sentry.origin': 'auto.http.browser.cls',
101+
'sentry.op': 'ui.webvital.cls',
102+
'sentry.exclusive_time': 0,
103+
'sentry.pageload.span_id': pageloadSpanId,
104+
'sentry.report_event': 'pagehide',
105+
},
106+
startTime: 1.5,
107+
});
108+
109+
expect(mockSpan.end).toHaveBeenCalledWith(1.5);
110+
expect(mockSpan.addEvent).toHaveBeenCalledWith('cls', {
111+
'sentry.measurement_unit': '',
112+
'sentry.measurement_value': 0,
113+
});
114+
});
115+
116+
it('handles entry with multiple sources', () => {
117+
const clsValue = 0.15;
118+
const mockEntry: LayoutShift = {
119+
name: 'layout-shift',
120+
entryType: 'layout-shift',
121+
startTime: 200,
122+
duration: 0,
123+
value: clsValue,
124+
hadRecentInput: false,
125+
sources: [
126+
// @ts-expect-error - other properties are irrelevant
127+
{
128+
node: { tagName: 'div' } as Element,
129+
},
130+
// @ts-expect-error - other properties are irrelevant
131+
{
132+
node: { tagName: 'span' } as Element,
133+
},
134+
],
135+
toJSON: vi.fn(),
136+
};
137+
const pageloadSpanId = '789';
138+
139+
vi.mocked(SentryCore.htmlTreeAsString)
140+
.mockReturnValueOnce('<div>') // for the name
141+
.mockReturnValueOnce('<div>') // for source 1
142+
.mockReturnValueOnce('<span>'); // for source 2
143+
144+
_sendStandaloneClsSpan(clsValue, mockEntry, pageloadSpanId, 'navigation');
145+
146+
expect(SentryCore.htmlTreeAsString).toHaveBeenCalledTimes(3);
147+
expect(WebVitalUtils.startStandaloneWebVitalSpan).toHaveBeenCalledWith({
148+
name: '<div>',
149+
transaction: 'test-transaction',
150+
attributes: {
151+
'sentry.origin': 'auto.http.browser.cls',
152+
'sentry.op': 'ui.webvital.cls',
153+
'sentry.exclusive_time': 0,
154+
'sentry.pageload.span_id': '789',
155+
'sentry.report_event': 'navigation',
156+
'cls.source.1': '<div>',
157+
'cls.source.2': '<span>',
158+
},
159+
startTime: 1.2, // (1000 + 200) / 1000
160+
});
161+
});
162+
163+
it('handles entry without sources', () => {
164+
const clsValue = 0.05;
165+
const mockEntry: LayoutShift = {
166+
name: 'layout-shift',
167+
entryType: 'layout-shift',
168+
startTime: 50,
169+
duration: 0,
170+
value: clsValue,
171+
hadRecentInput: false,
172+
sources: [],
173+
toJSON: vi.fn(),
174+
};
175+
const pageloadSpanId = '101';
176+
177+
_sendStandaloneClsSpan(clsValue, mockEntry, pageloadSpanId, 'navigation');
178+
179+
expect(WebVitalUtils.startStandaloneWebVitalSpan).toHaveBeenCalledWith({
180+
name: '<div>',
181+
transaction: 'test-transaction',
182+
attributes: {
183+
'sentry.origin': 'auto.http.browser.cls',
184+
'sentry.op': 'ui.webvital.cls',
185+
'sentry.exclusive_time': 0,
186+
'sentry.pageload.span_id': '101',
187+
'sentry.report_event': 'navigation',
188+
},
189+
startTime: 1.05, // (1000 + 50) / 1000
190+
});
191+
});
192+
193+
it('handles when startStandaloneWebVitalSpan returns undefined', () => {
194+
vi.spyOn(WebVitalUtils, 'startStandaloneWebVitalSpan').mockReturnValue(undefined);
195+
196+
const clsValue = 0.1;
197+
const pageloadSpanId = '123';
198+
199+
expect(() => {
200+
_sendStandaloneClsSpan(clsValue, undefined, pageloadSpanId, 'navigation');
201+
}).not.toThrow();
202+
203+
expect(mockSpan.addEvent).not.toHaveBeenCalled();
204+
expect(mockSpan.end).not.toHaveBeenCalled();
205+
});
206+
207+
it('handles when browserPerformanceTimeOrigin returns null', () => {
208+
vi.mocked(SentryCore.browserPerformanceTimeOrigin).mockReturnValue(undefined);
209+
210+
const clsValue = 0.1;
211+
const mockEntry: LayoutShift = {
212+
name: 'layout-shift',
213+
entryType: 'layout-shift',
214+
startTime: 200,
215+
duration: 0,
216+
value: clsValue,
217+
hadRecentInput: false,
218+
sources: [],
219+
toJSON: vi.fn(),
220+
};
221+
const pageloadSpanId = '123';
222+
223+
_sendStandaloneClsSpan(clsValue, mockEntry, pageloadSpanId, 'navigation');
224+
225+
expect(WebVitalUtils.startStandaloneWebVitalSpan).toHaveBeenCalledWith(
226+
expect.objectContaining({
227+
startTime: 0.2,
228+
}),
229+
);
230+
});
231+
});

0 commit comments

Comments
 (0)