Skip to content

Commit d132f35

Browse files
vaindvaind
committed
feat: separate metrics measurements for sentry and sentry+replay
1 parent 5e86a8e commit d132f35

File tree

11 files changed

+204
-123
lines changed

11 files changed

+204
-123
lines changed

‎packages/replay/metrics/configs/ci/collect.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import { JankTestScenario } from '../../src/scenarios.js';
44
import { printStats } from '../../src/util/console.js';
55
import { latestResultFile } from './env.js';
66

7-
function checkStdDev(stats: MetricsStats, name: string, provider: NumberProvider, max: number): boolean {
8-
const value = stats.stddev(provider);
7+
function checkStdDev(results: Metrics[], name: string, provider: NumberProvider, max: number): boolean {
8+
const value = MetricsStats.stddev(results, provider);
99
if (value == undefined) {
1010
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is undefined`);
1111
return false;
@@ -21,23 +21,25 @@ function checkStdDev(stats: MetricsStats, name: string, provider: NumberProvider
2121
const collector = new MetricsCollector({ headless: true, cpuThrottling: 2 });
2222
const result = await collector.execute({
2323
name: 'jank',
24-
a: new JankTestScenario(false), // No sentry
25-
b: new JankTestScenario(true), // Sentry + Replay
24+
scenarios: [
25+
new JankTestScenario('index.html'),
26+
new JankTestScenario('with-sentry.html'),
27+
new JankTestScenario('with-replay.html'),
28+
],
2629
runs: 10,
2730
tries: 10,
2831
async shouldAccept(results: Metrics[]): Promise<boolean> {
29-
const stats = new MetricsStats(results);
30-
await printStats(stats);
32+
await printStats(results);
3133

32-
if (!checkStdDev(stats, 'lcp', MetricsStats.lcp, 50)
33-
|| !checkStdDev(stats, 'cls', MetricsStats.cls, 0.1)
34-
|| !checkStdDev(stats, 'cpu', MetricsStats.cpu, 1)
35-
|| !checkStdDev(stats, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024)
36-
|| !checkStdDev(stats, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)) {
34+
if (!checkStdDev(results, 'lcp', MetricsStats.lcp, 50)
35+
|| !checkStdDev(results, 'cls', MetricsStats.cls, 0.1)
36+
|| !checkStdDev(results, 'cpu', MetricsStats.cpu, 1)
37+
|| !checkStdDev(results, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024)
38+
|| !checkStdDev(results, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)) {
3739
return false;
3840
}
3941

40-
const cpuUsage = stats.mean(MetricsStats.cpu)!;
42+
const cpuUsage = MetricsStats.mean(results, MetricsStats.cpu)!;
4143
if (cpuUsage > 0.85) {
4244
// Note: complexity on the "JankTest" is defined by the `minimum = ...,` setting in app.js - specifying the number of animated elements.
4345
console.warn(`✗ | Discarding results because CPU usage is too high and may be inaccurate: ${(cpuUsage * 100).toFixed(2)} %.`,

‎packages/replay/metrics/configs/dev/collect.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,17 @@ import { latestResultFile } from './env.js';
77
const collector = new MetricsCollector();
88
const result = await collector.execute({
99
name: 'dummy',
10-
a: new JankTestScenario(false),
11-
b: new JankTestScenario(true),
10+
scenarios: [
11+
new JankTestScenario('index.html'),
12+
new JankTestScenario('with-sentry.html'),
13+
new JankTestScenario('with-replay.html'),
14+
],
1215
runs: 1,
1316
tries: 1,
1417
async shouldAccept(results: Metrics[]): Promise<boolean> {
15-
const stats = new MetricsStats(results);
16-
printStats(stats);
18+
printStats(results);
1719

18-
const cpuUsage = stats.mean(MetricsStats.cpu)!;
20+
const cpuUsage = MetricsStats.mean(results, MetricsStats.cpu)!;
1921
if (cpuUsage > 0.9) {
2022
console.error(`CPU usage too high to be accurate: ${(cpuUsage * 100).toFixed(2)} %.`,
2123
'Consider simplifying the scenario or changing the CPU throttling factor.');

‎packages/replay/metrics/src/collector.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,11 @@ export class MetricsCollector {
5858
public async execute(testCase: TestCase): Promise<Result> {
5959
console.log(`Executing test case ${testCase.name}`);
6060
return consoleGroup(async () => {
61-
const aResults = await this._collect(testCase, 'A', testCase.a);
62-
const bResults = await this._collect(testCase, 'B', testCase.b);
63-
return new Result(testCase.name, this._options.cpuThrottling, networkConditions, aResults, bResults);
61+
const scenarioResults: Metrics[][] = [];
62+
for (let s = 0; s < testCase.scenarios.length; s++) {
63+
scenarioResults.push(await this._collect(testCase, s.toString(), testCase.scenarios[s]));
64+
}
65+
return new Result(testCase.name, this._options.cpuThrottling, networkConditions, scenarioResults);
6466
});
6567
}
6668

‎packages/replay/metrics/src/results/analyzer.ts

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { filesize } from 'filesize';
22

33
import { GitHash } from '../util/git.js';
4-
import { MetricsStats } from './metrics-stats.js';
4+
import { AnalyticsFunction, MetricsStats, NumberProvider } from './metrics-stats.js';
55
import { Result } from './result.js';
66
import { ResultsSet } from './results-set.js';
77

@@ -24,7 +24,7 @@ export class ResultsAnalyzer {
2424
for (const base of baseItems) {
2525
for (const item of items) {
2626
if (item.metric == base.metric) {
27-
item.other = base.value;
27+
item.others = base.values;
2828
otherHash = baseline[0];
2929
}
3030
}
@@ -40,21 +40,26 @@ export class ResultsAnalyzer {
4040
private _collect(): AnalyzerItem[] {
4141
const items = new Array<AnalyzerItem>();
4242

43-
const aStats = new MetricsStats(this._result.aResults);
44-
const bStats = new MetricsStats(this._result.bResults);
43+
const scenarioResults = this._result.scenarioResults;
4544

46-
const pushIfDefined = function (metric: AnalyzerItemMetric, unit: AnalyzerItemUnit, valueA?: number, valueB?: number): void {
47-
if (valueA == undefined || valueB == undefined) return;
48-
items.push({ metric: metric, value: new AnalyzerItemNumberValue(unit, valueA, valueB) })
45+
const pushIfDefined = function (metric: AnalyzerItemMetric, unit: AnalyzerItemUnit, source: NumberProvider, fn: AnalyticsFunction): void {
46+
const values = scenarioResults.map(items => fn(items, source));
47+
// only push if at least one value is defined
48+
if (values.findIndex(v => v != undefined) >= 0) {
49+
items.push({
50+
metric: metric,
51+
values: new AnalyzerItemNumberValues(unit, values)
52+
});
53+
}
4954
}
5055

51-
pushIfDefined(AnalyzerItemMetric.lcp, AnalyzerItemUnit.ms, aStats.mean(MetricsStats.lcp), bStats.mean(MetricsStats.lcp));
52-
pushIfDefined(AnalyzerItemMetric.cls, AnalyzerItemUnit.ms, aStats.mean(MetricsStats.cls), bStats.mean(MetricsStats.cls));
53-
pushIfDefined(AnalyzerItemMetric.cpu, AnalyzerItemUnit.ratio, aStats.mean(MetricsStats.cpu), bStats.mean(MetricsStats.cpu));
54-
pushIfDefined(AnalyzerItemMetric.memoryAvg, AnalyzerItemUnit.bytes, aStats.mean(MetricsStats.memoryMean), bStats.mean(MetricsStats.memoryMean));
55-
pushIfDefined(AnalyzerItemMetric.memoryMax, AnalyzerItemUnit.bytes, aStats.max(MetricsStats.memoryMax), bStats.max(MetricsStats.memoryMax));
56+
pushIfDefined(AnalyzerItemMetric.lcp, AnalyzerItemUnit.ms, MetricsStats.lcp, MetricsStats.mean);
57+
pushIfDefined(AnalyzerItemMetric.cls, AnalyzerItemUnit.ms, MetricsStats.cls, MetricsStats.mean);
58+
pushIfDefined(AnalyzerItemMetric.cpu, AnalyzerItemUnit.ratio, MetricsStats.cpu, MetricsStats.mean);
59+
pushIfDefined(AnalyzerItemMetric.memoryAvg, AnalyzerItemUnit.bytes, MetricsStats.memoryMean, MetricsStats.mean);
60+
pushIfDefined(AnalyzerItemMetric.memoryMax, AnalyzerItemUnit.bytes, MetricsStats.memoryMax, MetricsStats.max);
5661

57-
return items.filter((item) => item.value != undefined);
62+
return items;
5863
}
5964
}
6065

@@ -64,35 +69,42 @@ export enum AnalyzerItemUnit {
6469
bytes,
6570
}
6671

67-
export interface AnalyzerItemValue {
68-
readonly a: string;
69-
readonly b: string;
70-
readonly diff: string;
71-
readonly percent: string;
72+
export interface AnalyzerItemValues {
73+
value(index: number): string;
74+
diff(aIndex: number, bIndex: number): string;
75+
percent(aIndex: number, bIndex: number): string;
7276
}
7377

74-
class AnalyzerItemNumberValue implements AnalyzerItemValue {
75-
constructor(private _unit: AnalyzerItemUnit, private _a: number, private _b: number) { }
78+
const AnalyzerItemValueNotAvailable = 'n/a';
79+
80+
class AnalyzerItemNumberValues implements AnalyzerItemValues {
81+
constructor(private _unit: AnalyzerItemUnit, private _values: (number | undefined)[]) { }
7682

77-
public get a(): string {
78-
return this._withUnit(this._a);
83+
private _has(index: number): boolean {
84+
return index >= 0 && index < this._values.length && this._values[index] != undefined;
7985
}
8086

81-
public get b(): string {
82-
return this._withUnit(this._b);
87+
private _get(index: number): number {
88+
return this._values[index]!;
8389
}
8490

85-
public get diff(): string {
86-
const diff = this._b - this._a;
91+
public value(index: number): string {
92+
if (!this._has(index)) return AnalyzerItemValueNotAvailable;
93+
return this._withUnit(this._get(index));
94+
}
95+
96+
public diff(aIndex: number, bIndex: number): string {
97+
if (!this._has(aIndex) || !this._has(bIndex)) return AnalyzerItemValueNotAvailable;
98+
const diff = this._get(bIndex) - this._get(aIndex);
8799
const str = this._withUnit(diff, true);
88100
return diff > 0 ? `+${str}` : str;
89101
}
90102

91-
public get percent(): string {
92-
if (this._a == 0) return 'n/a';
93-
const diff = this._b / this._a * 100 - 100;
94-
const str = `${diff.toFixed(2)} %`;
95-
return diff > 0 ? `+${str}` : str;
103+
public percent(aIndex: number, bIndex: number): string {
104+
if (!this._has(aIndex) || !this._has(bIndex) || this._get(aIndex) == 0.0) return AnalyzerItemValueNotAvailable;
105+
const percent = this._get(bIndex) / this._get(aIndex) * 100 - 100;
106+
const str = `${percent.toFixed(2)} %`;
107+
return percent > 0 ? `+${str}` : str;
96108
}
97109

98110
private _withUnit(value: number, isDiff: boolean = false): string {
@@ -119,10 +131,10 @@ export interface AnalyzerItem {
119131
metric: AnalyzerItemMetric;
120132

121133
// Current (latest) result.
122-
value: AnalyzerItemValue;
134+
values: AnalyzerItemValues;
123135

124136
// Previous or baseline results, depending on the context.
125-
other?: AnalyzerItemValue;
137+
others?: AnalyzerItemValues;
126138
}
127139

128140
export interface Analysis {

‎packages/replay/metrics/src/results/metrics-stats.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,32 @@ import * as ss from 'simple-statistics'
33
import { Metrics } from '../collector';
44

55
export type NumberProvider = (metrics: Metrics) => number;
6+
export type AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => number | undefined;
67

78
export class MetricsStats {
8-
constructor(private _items: Metrics[]) { }
9-
109
static lcp: NumberProvider = metrics => metrics.vitals.lcp;
1110
static cls: NumberProvider = metrics => metrics.vitals.cls;
1211
static cpu: NumberProvider = metrics => metrics.cpu.average;
1312
static memoryMean: NumberProvider = metrics => ss.mean(Array.from(metrics.memory.snapshots.values()));
1413
static memoryMax: NumberProvider = metrics => ss.max(Array.from(metrics.memory.snapshots.values()));
1514

16-
public mean(dataProvider: NumberProvider): number | undefined {
17-
const numbers = this._filteredValues(dataProvider);
15+
static mean: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
16+
const numbers = MetricsStats._filteredValues(items.map(dataProvider));
1817
return numbers.length > 0 ? ss.mean(numbers) : undefined;
1918
}
2019

21-
public max(dataProvider: NumberProvider): number | undefined {
22-
const numbers = this._filteredValues(dataProvider);
20+
static max: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
21+
const numbers = MetricsStats._filteredValues(items.map(dataProvider));
2322
return numbers.length > 0 ? ss.max(numbers) : undefined;
2423
}
2524

26-
public stddev(dataProvider: NumberProvider): number | undefined {
27-
const numbers = this._filteredValues(dataProvider);
25+
static stddev: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
26+
const numbers = MetricsStats._filteredValues(items.map(dataProvider));
2827
return numbers.length > 0 ? ss.standardDeviation(numbers) : undefined;
2928
}
3029

3130
// See https://en.wikipedia.org/wiki/Interquartile_range#Outliers for details on filtering.
32-
private _filteredValues(dataProvider: NumberProvider): number[] {
33-
const numbers = this._items.map(dataProvider);
31+
private static _filteredValues(numbers: number[]): number[] {
3432
numbers.sort((a, b) => a - b)
3533

3634
const q1 = ss.quantileSorted(numbers, 0.25);

‎packages/replay/metrics/src/results/pr-comment.ts

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Git } from '../util/git.js';
2-
import { Analysis, AnalyzerItemMetric, ResultsAnalyzer } from './analyzer.js';
2+
import { Analysis, AnalyzerItemMetric, AnalyzerItemValues, ResultsAnalyzer } from './analyzer.js';
33
import { Result } from './result.js';
44
import { ResultSetItem } from './results-set.js';
55

@@ -44,54 +44,70 @@ export class PrCommentBuilder {
4444
}
4545

4646
public async addCurrentResult(analysis: Analysis, otherName: string): Promise<void> {
47-
// Decides whether to print the "Other" depending on it being set in the input data.
47+
// Decides whether to print the "Other" for comparison depending on it being set in the input data.
48+
const hasOther = analysis.otherHash != undefined;
4849
const maybeOther = function (content: () => string): string {
49-
if (analysis.otherHash == undefined) {
50-
return '';
51-
}
52-
return content();
50+
return hasOther ? content() : '';
5351
}
5452

53+
const currentHash = await Git.hash
54+
55+
this._buffer += `<h2>${this.title}</h2>`;
56+
if (!hasOther) {
57+
this._buffer += `Latest data for: ${currentHash}`;
58+
}
5559
this._buffer += `
56-
<h2>${this.title}</h2>
57-
<table>
58-
<thead>`;
59-
60-
const headerCols = '<th align="right">Plain</th><th align="right">+Replay</th><th align="right">Diff</th><th align="right">Ratio</th>';
61-
if (analysis.otherHash != undefined) {
62-
// If "other" is defined, add an aditional row of headers.
63-
this._buffer += `
60+
<table border="1">
61+
<thead>
6462
<tr>
6563
<th rowspan="2">&nbsp;</th>
66-
<th colspan="4" align="center">This PR (${await Git.hash})</th>
67-
<th colspan="4" align="center">${otherName} (${analysis.otherHash})</a></th>
64+
${maybeOther(() => `<th align="left">&nbsp;</th>`)}
65+
<th align="center">Plain</th>
66+
<th colspan="3" align="center">+Sentry</th>
67+
<th colspan="3" align="center">+Replay</th>
6868
</tr>
6969
<tr>
70-
${headerCols}
71-
${headerCols}
72-
</tr>`;
73-
} else {
74-
this._buffer += `
75-
<tr>
76-
<th>&nbsp;</th>
77-
${headerCols}
70+
${maybeOther(() => `<th align="left">Revision</th>`)}
71+
<th align="right">Value</th>
72+
<th align="right">Value</th>
73+
<th align="right">Diff</th>
74+
<th align="right">Ratio</th>
75+
<th align="right">Value</th>
76+
<th align="right">Diff</th>
77+
<th align="right">Ratio</th>
7878
</tr>`;
79+
80+
const valueColumns = function (values: AnalyzerItemValues): string {
81+
return `
82+
<td align="right">${values.value(0)}</td>
83+
<td align="right">${values.value(1)}</td>
84+
<td align="right"><strong>${values.diff(0, 1)}</strong></td>
85+
<td align="right"><strong>${values.percent(0, 1)}</strong></td>
86+
<td align="right">${values.value(2)}</td>
87+
<td align="right"><strong>${values.diff(1, 2)}</strong></td>
88+
<td align="right"><strong>${values.percent(1, 2)}</strong></td>
89+
`;
7990
}
8091

8192
for (const item of analysis.items) {
82-
this._buffer += `
93+
if (hasOther) {
94+
this._buffer += `
8395
<tr>
84-
<th align="right">${printableMetricName(item.metric)}</th>
85-
<td align="right">${item.value.a}</td>
86-
<td align="right">${item.value.b}</td>
87-
<td align="right"><strong>${item.value.diff}</strong></td>
88-
<td align="right"><strong>${item.value.percent}</strong></td>
89-
${maybeOther(() => `
90-
<td align="right">${item.other!.a}</td>
91-
<td align="right">${item.other!.b}</td>
92-
<td align="right"><strong>${item.other!.diff}</strong></td>
93-
<td align="right"><strong>${item.other!.percent}</strong></td>`)}
96+
<th rowspan="2" align="left">${printableMetricName(item.metric)}</th>
97+
<th align="left">This PR ${currentHash}</td>
98+
${valueColumns(item.values)}
99+
</tr>
100+
<tr>
101+
<th align="left">${otherName} ${analysis.otherHash}</td>
102+
${valueColumns(item.others!)}
94103
</tr>`
104+
} else {
105+
this._buffer += `
106+
<tr>
107+
<th align="left">${printableMetricName(item.metric)}</th>
108+
${valueColumns(item.values)}
109+
</tr>`
110+
}
95111
}
96112

97113
this._buffer += `
@@ -104,7 +120,7 @@ export class PrCommentBuilder {
104120
this._buffer += `
105121
<details>
106122
<summary><h3>${name}</h3></summary>
107-
<table>`;
123+
<table border="1">`;
108124

109125
// Each `resultFile` will be printed as a single row - with metrics as table columns.
110126
for (let i = 0; i < resultFiles.length; i++) {
@@ -124,7 +140,8 @@ export class PrCommentBuilder {
124140
// Add table row
125141
this._buffer += `<tr><th>${resultFile.hash}</th>`;
126142
for (const item of analysis.items) {
127-
this._buffer += `<td align="right">${item.value.diff}</td>`;
143+
// TODO maybe find a better way of showing this. After the change to multiple scenarios, this shows diff between "With Sentry" and "With Sentry + Replay"
144+
this._buffer += `<td align="right">${item.values.diff(1, 2)}</td>`;
128145
}
129146
this._buffer += '</tr>';
130147
}

0 commit comments

Comments
 (0)