Skip to content
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

ref(replay): Update CLS web vitals to align with browserTracing #13058

Merged
merged 13 commits into from
Jul 30, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export const expectedLCPPerformanceSpan = {
endTimestamp: expect.any(Number),
data: {
value: expect.any(Number),
nodeId: expect.any(Number),
nodeId: expect.any(Array),
rating: expect.any(String),
size: expect.any(Number),
},
Expand All @@ -140,6 +140,7 @@ export const expectedCLSPerformanceSpan = {
endTimestamp: expect.any(Number),
data: {
value: expect.any(Number),
nodeId: expect.any(Array),
rating: expect.any(String),
size: expect.any(Number),
},
Expand All @@ -154,7 +155,7 @@ export const expectedFIDPerformanceSpan = {
value: expect.any(Number),
rating: expect.any(String),
size: expect.any(Number),
nodeId: expect.any(Number),
nodeId: expect.any(Array),
},
};

Expand All @@ -167,7 +168,7 @@ export const expectedINPPerformanceSpan = {
value: expect.any(Number),
rating: expect.any(String),
size: expect.any(Number),
nodeId: expect.any(Number),
nodeId: expect.any(Array),
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export const ReplayRecordingData = [
value: expect.any(Number),
size: expect.any(Number),
rating: expect.any(String),
nodeId: 16,
nodeId: [16],
},
},
},
Expand All @@ -239,6 +239,7 @@ export const ReplayRecordingData = [
value: expect.any(Number),
size: expect.any(Number),
rating: expect.any(String),
nodeId: [],
},
},
},
Expand All @@ -257,7 +258,7 @@ export const ReplayRecordingData = [
value: expect.any(Number),
size: expect.any(Number),
rating: expect.any(String),
nodeId: 10,
nodeId: [10],
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/replay-internal/src/types/performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export interface WebVitalData {
/**
* The recording id of the LCP node. -1 if not found
*/
nodeId?: number;
nodeId?: number | number[];
}

/**
Expand Down
35 changes: 25 additions & 10 deletions packages/replay-internal/src/util/createPerformanceEntries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,18 @@ export function getLargestContentfulPaint(metric: Metric): ReplayPerformanceEntr
* Add a CLS event to the replay based on a CLS metric.
*/
export function getCumulativeLayoutShift(metric: Metric): ReplayPerformanceEntry<WebVitalData> {
// get first node that shifts
const firstEntry = metric.entries[0] as (PerformanceEntry & { sources?: LayoutShiftAttribution[] }) | undefined;
const node = firstEntry
? firstEntry.sources && firstEntry.sources[0]
? firstEntry.sources[0].node
: undefined
: undefined;
return getWebVital(metric, 'cumulative-layout-shift', node);
const lastEntry = metric.entries[metric.entries.length - 1] as
| (PerformanceEntry & { sources?: LayoutShiftAttribution[] })
| undefined;
const nodes: Node[] = [];
if (lastEntry && lastEntry.sources) {
for (const source of lastEntry.sources) {
if (source.node) {
nodes.push(source.node);
}
}
}
return getWebVital(metric, 'cumulative-layout-shift', nodes);
}

/**
Expand All @@ -225,13 +229,24 @@ export function getInteractionToNextPaint(metric: Metric): ReplayPerformanceEntr
export function getWebVital(
metric: Metric,
name: string,
node: Node | undefined,
node: Node | Node[] | undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is getWebVital a public function? It might be better to change the parameters so that it accepts Node[] | undefined.

): ReplayPerformanceEntry<WebVitalData> {
const value = metric.value;
const rating = metric.rating;

const end = getAbsoluteTime(value);

const nodeIds: number[] = [];
if (Array.isArray(node)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be easier to follow and less code:

Suggested change
const nodeIds: number[] = [];
if (Array.isArray(node)) {
const nodes = Array.isArray(node) ? node : [];
const nodeIds = nodes.map(node => record.mirror.getId(node);

for (const n of node) {
nodeIds.push(record.mirror.getId(n));
}
} else {
if (node) {
nodeIds.push(record.mirror.getId(node));
}
}

const data: ReplayPerformanceEntry<WebVitalData> = {
type: 'web-vital',
name,
Expand All @@ -241,7 +256,7 @@ export function getWebVital(
value,
size: value,
rating,
nodeId: node ? record.mirror.getId(node) : undefined,
nodeId: node ? nodeIds : undefined,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('Unit | util | createPerformanceEntries', () => {
});

describe('getCumulativeLayoutShift', () => {
it('works with an CLS metric', async () => {
it('works with a CLS metric', async () => {
const metric = {
value: 5108.299,
rating: 'good' as const,
Expand All @@ -103,7 +103,7 @@ describe('Unit | util | createPerformanceEntries', () => {
name: 'cumulative-layout-shift',
start: 1672531205.108299,
end: 1672531205.108299,
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeId: undefined },
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeId: [] },
});
});
});
Expand Down
Loading