Skip to content

Commit 8e27ff9

Browse files
billyvgLms24
andauthored
fix(replay): Fix timestamps on LCP (#7225)
`value` was being used as ms but start/end timestamps are in seconds. Changed start and end to be equal because LCP does not have a duration, it is a single point in time. --------- Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
1 parent e4d3146 commit 8e27ff9

File tree

6 files changed

+62
-7
lines changed

6 files changed

+62
-7
lines changed

packages/integration-tests/suites/replay/customEvents/test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
expectedNavigationPerformanceSpan,
1111
getExpectedReplayEvent,
1212
} from '../../../utils/replayEventTemplates';
13+
import type { PerformanceSpan } from '../../../utils/replayHelpers';
1314
import {
1415
getCustomRecordingEvents,
1516
getReplayEvent,
@@ -68,6 +69,13 @@ sentryTest(
6869
expectedMemoryPerformanceSpan,
6970
]),
7071
);
72+
73+
const lcpSpan = collectedPerformanceSpans.find(
74+
s => s.description === 'largest-contentful-paint',
75+
) as PerformanceSpan;
76+
77+
// LCP spans should be point-in-time spans
78+
expect(lcpSpan?.startTimestamp).toBeCloseTo(lcpSpan?.endTimestamp);
7179
},
7280
);
7381

packages/integration-tests/utils/replayHelpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { Page, Request } from 'playwright';
66
import { envelopeRequestParser } from './helpers';
77

88
type CustomRecordingEvent = { tag: string; payload: Record<string, unknown> };
9-
type PerformanceSpan = {
9+
export type PerformanceSpan = {
1010
op: string;
1111
description: string;
1212
startTimestamp: number;

packages/replay/src/util/createPerformanceEntries.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,19 @@ function createLargestContentfulPaint(entry: PerformanceEntry & { size: number;
117117
startTimeOrNavigationActivation = (navEntry && navEntry.activationStart) || 0;
118118
}
119119

120-
const start = getAbsoluteTime(startTimeOrNavigationActivation);
120+
// value is in ms
121121
const value = Math.max(startTime - startTimeOrNavigationActivation, 0);
122+
// LCP doesn't have a "duration", it just happens at a single point in time.
123+
// But the UI expects both, so use end (in seconds) for both timestamps.
124+
const end = getAbsoluteTime(startTimeOrNavigationActivation) + value / 1000;
122125

123126
return {
124127
type: entryType,
125128
name: entryType,
126-
start,
127-
end: start + value,
129+
start: end,
130+
end,
128131
data: {
129-
value,
132+
value, // LCP "duration" in ms
130133
size,
131134
// Not sure why this errors, Node should be correct (Argument of type 'Node' is not assignable to parameter of type 'INode')
132135
// eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/replay/test/fixtures/performanceEntry/lcp.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ export function PerformanceEntryLcp(obj?: Partial<PerformancePaintTiming>): Perf
44
const entry = {
55
name: '',
66
entryType: 'largest-contentful-paint',
7-
startTime: 108.299,
7+
startTime: 5108.299,
88
duration: 0,
99
size: 7619,
10-
renderTime: 108.299,
10+
renderTime: 5108.299,
1111
loadTime: 0,
1212
firstAnimatedFrameTime: 0,
1313
id: '',

packages/replay/test/fixtures/performanceEntry/navigation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { PerformanceNavigationTiming } from '../../../src/types';
22

33
export function PerformanceEntryNavigation(obj?: Partial<PerformanceNavigationTiming>): PerformanceNavigationTiming {
44
const entry = {
5+
activationStart: 0,
56
name: 'https://sentry.io/organizations/sentry/discover/',
67
entryType: 'navigation',
78
startTime: 0,

packages/replay/test/unit/util/createPerformanceEntry.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,32 @@
1+
jest.useFakeTimers().setSystemTime(new Date('2023-01-01'));
2+
3+
jest.mock('@sentry/utils', () => ({
4+
...jest.requireActual('@sentry/utils'),
5+
browserPerformanceTimeOrigin: Date.now(),
6+
}));
7+
8+
import { WINDOW } from '../../../src/constants';
19
import { createPerformanceEntries } from '../../../src/util/createPerformanceEntries';
10+
import { PerformanceEntryLcp } from '../../fixtures/performanceEntry/lcp';
11+
import { PerformanceEntryNavigation } from '../../fixtures/performanceEntry/navigation';
212

313
describe('Unit | util | createPerformanceEntries', () => {
14+
beforeEach(function () {
15+
if (!WINDOW.performance.getEntriesByType) {
16+
WINDOW.performance.getEntriesByType = jest.fn((type: string) => {
17+
if (type === 'navigation') {
18+
return [PerformanceEntryNavigation()];
19+
}
20+
throw new Error(`entry ${type} not mocked`);
21+
});
22+
}
23+
});
24+
25+
afterAll(function () {
26+
jest.clearAllMocks();
27+
jest.useRealTimers();
28+
});
29+
430
it('ignores sdks own requests', function () {
531
const data = {
632
name: 'https://ingest.f00.f00/api/1/envelope/?sentry_key=dsn&sentry_version=7',
@@ -31,4 +57,21 @@ describe('Unit | util | createPerformanceEntries', () => {
3157
// @ts-ignore Needs a PerformanceEntry mock
3258
expect(createPerformanceEntries([data])).toEqual([]);
3359
});
60+
61+
it('has correct LCP entry when no navigation event', function () {
62+
const result = createPerformanceEntries([PerformanceEntryLcp()]);
63+
expect(result).toEqual([
64+
{
65+
data: {
66+
nodeId: -1,
67+
size: 7619,
68+
value: 5108.299,
69+
},
70+
name: 'largest-contentful-paint',
71+
type: 'largest-contentful-paint',
72+
end: 1672531205.108299,
73+
start: 1672531205.108299,
74+
},
75+
]);
76+
});
3477
});

0 commit comments

Comments
 (0)