Skip to content

Commit 10dc496

Browse files
vaindvaind
committed
metrics: check stddev when collecting results
1 parent 6df0698 commit 10dc496

File tree

6 files changed

+81
-61
lines changed

6 files changed

+81
-61
lines changed

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,38 @@
11
import { Metrics, MetricsCollector } from '../../src/collector.js';
2+
import { MetricsStats, NumberProvider } from '../../src/results/metrics-stats.js';
23
import { JankTestScenario } from '../../src/scenarios.js';
34
import { latestResultFile } from './env.js';
45

6+
function checkStdDev(stats: MetricsStats, name: string, provider: NumberProvider, max: number): boolean {
7+
const value = stats.stddev(provider);
8+
if (value == undefined) {
9+
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is undefined`);
10+
return false;
11+
} else if (value > max) {
12+
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is larger than ${max}. Actual value: ${value}`);
13+
return false;
14+
} else {
15+
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`)
16+
}
17+
return true;
18+
}
19+
520
const collector = new MetricsCollector({ headless: true });
621
const result = await collector.execute({
7-
name: 'dummy',
22+
name: 'jank',
823
a: new JankTestScenario(false),
924
b: new JankTestScenario(true),
10-
runs: 1,
11-
tries: 1,
12-
async test(_aResults: Metrics[], _bResults: Metrics[]) {
13-
return true;
25+
runs: 10,
26+
tries: 10,
27+
async shouldAccept(results: Metrics[]): Promise<boolean> {
28+
const stats = new MetricsStats(results);
29+
return true
30+
&& checkStdDev(stats, 'lcp', MetricsStats.lcp, 10)
31+
&& checkStdDev(stats, 'cls', MetricsStats.cls, 10)
32+
&& checkStdDev(stats, 'cpu', MetricsStats.cpu, 10)
33+
&& checkStdDev(stats, 'memory-mean', MetricsStats.memoryMean, 10000)
34+
&& checkStdDev(stats, 'memory-max', MetricsStats.memoryMax, 10000);
35+
;
1436
},
1537
});
1638

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const result = await collector.execute({
99
b: new JankTestScenario(true),
1010
runs: 1,
1111
tries: 1,
12-
async test(_aResults: Metrics[], _bResults: Metrics[]) {
12+
async shouldAccept(_results: Metrics[]): Promise<boolean> {
1313
return true;
1414
},
1515
});

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

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { JsHeapUsage, JsHeapUsageSampler, JsHeapUsageSerialized } from './perf/m
66
import { PerfMetricsSampler } from './perf/sampler.js';
77
import { Result } from './results/result.js';
88
import { Scenario, TestCase } from './scenarios.js';
9+
import { consoleGroup } from './util/console.js';
910
import { WebVitals, WebVitalsCollector } from './vitals/index.js';
1011

1112
const cpuThrottling = 4;
@@ -55,37 +56,38 @@ export class MetricsCollector {
5556

5657
public async execute(testCase: TestCase): Promise<Result> {
5758
console.log(`Executing test case ${testCase.name}`);
58-
console.group();
59-
for (let i = 1; i <= testCase.tries; i++) {
60-
const aResults = await this._collect('A', testCase.a, testCase.runs);
61-
const bResults = await this._collect('B', testCase.b, testCase.runs);
62-
if (await testCase.test(aResults, bResults)) {
63-
console.groupEnd();
64-
console.log(`Test case ${testCase.name} passed on try ${i}/${testCase.tries}`);
65-
return new Result(testCase.name, cpuThrottling, networkConditions, aResults, bResults);
66-
} else if (i != testCase.tries) {
67-
console.log(`Test case ${testCase.name} failed on try ${i}/${testCase.tries}`);
68-
} else {
69-
console.groupEnd();
70-
console.error(`Test case ${testCase.name} failed`);
71-
}
72-
}
73-
throw `Test case execution ${testCase.name} failed after ${testCase.tries} tries.`;
59+
return consoleGroup(async () => {
60+
const aResults = await this._collect(testCase, 'A', testCase.a);
61+
const bResults = await this._collect(testCase, 'B', testCase.b);
62+
return new Result(testCase.name, cpuThrottling, networkConditions, aResults, bResults);
63+
});
7464
}
7565

76-
private async _collect(name: string, scenario: Scenario, runs: number): Promise<Metrics[]> {
77-
const label = `Scenario ${name} data collection (total ${runs} runs)`;
78-
console.time(label);
79-
const results: Metrics[] = [];
80-
for (let run = 0; run < runs; run++) {
81-
const innerLabel = `Scenario ${name} data collection, run ${run}/${runs}`;
82-
console.time(innerLabel);
83-
results.push(await this._run(scenario));
84-
console.timeEnd(innerLabel);
66+
private async _collect(testCase: TestCase, name: string, scenario: Scenario): Promise<Metrics[]> {
67+
const label = `Scenario ${name} data collection (total ${testCase.runs} runs)`;
68+
for (let try_ = 1; try_ <= testCase.tries; try_++) {
69+
console.time(label);
70+
const results: Metrics[] = [];
71+
for (let run = 1; run <= testCase.runs; run++) {
72+
const innerLabel = `Scenario ${name} data collection, run ${run}/${testCase.runs}`;
73+
console.time(innerLabel);
74+
results.push(await this._run(scenario));
75+
console.timeEnd(innerLabel);
76+
}
77+
console.timeEnd(label);
78+
assert.strictEqual(results.length, testCase.runs);
79+
if (await testCase.shouldAccept(results)) {
80+
console.log(`Test case ${testCase.name}, scenario ${name} passed on try ${try_}/${testCase.tries}`);
81+
return results;
82+
} else if (try_ != testCase.tries) {
83+
console.log(`Test case ${testCase.name} failed on try ${try_}/${testCase.tries}, retrying`);
84+
} else {
85+
throw `Test case ${testCase.name}, scenario ${name} failed after ${testCase.tries} tries.`;
86+
}
8587
}
86-
console.timeEnd(label);
87-
assert.strictEqual(results.length, runs);
88-
return results;
88+
// Unreachable code, if configured properly:
89+
console.assert(testCase.tries >= 1);
90+
return [];
8991
}
9092

9193
private async _run(scenario: Scenario): Promise<Metrics> {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ export class ResultsAnalyzer {
4848
items.push({ metric: metric, value: new AnalyzerItemNumberValue(unit, valueA, valueB) })
4949
}
5050

51-
pushIfDefined(AnalyzerItemMetric.lcp, AnalyzerItemUnit.ms, aStats.lcp, bStats.lcp);
52-
pushIfDefined(AnalyzerItemMetric.cls, AnalyzerItemUnit.ms, aStats.cls, bStats.cls);
53-
pushIfDefined(AnalyzerItemMetric.cpu, AnalyzerItemUnit.ratio, aStats.cpu, bStats.cpu);
54-
pushIfDefined(AnalyzerItemMetric.memoryAvg, AnalyzerItemUnit.bytes, aStats.memoryMean, bStats.memoryMean);
55-
pushIfDefined(AnalyzerItemMetric.memoryMax, AnalyzerItemUnit.bytes, aStats.memoryMax, bStats.memoryMax);
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));
5656

5757
return items.filter((item) => item.value != undefined);
5858
}

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,24 @@ export type NumberProvider = (metrics: Metrics) => number;
77
export class MetricsStats {
88
constructor(private _items: Metrics[]) { }
99

10+
static lcp: NumberProvider = metrics => metrics.vitals.lcp;
11+
static cls: NumberProvider = metrics => metrics.vitals.cls;
12+
static cpu: NumberProvider = metrics => metrics.cpu.average;
13+
static memoryMean: NumberProvider = metrics => ss.mean(Array.from(metrics.memory.snapshots.values()));
14+
static memoryMax: NumberProvider = metrics => ss.max(Array.from(metrics.memory.snapshots.values()));
15+
1016
public mean(dataProvider: NumberProvider): number | undefined {
1117
const numbers = this._items.map(dataProvider);
1218
return numbers.length > 0 ? ss.mean(numbers) : undefined;
1319
}
1420

15-
public get lcp(): number | undefined {
16-
return this.mean((metrics) => metrics.vitals.lcp);
17-
}
18-
19-
public get cls(): number | undefined {
20-
return this.mean((metrics) => metrics.vitals.cls);
21-
}
22-
23-
public get cpu(): number | undefined {
24-
return this.mean((metrics) => metrics.cpu.average);
25-
}
26-
27-
public get memoryMean(): number | undefined {
28-
return this.mean((metrics) => ss.mean(Array.from(metrics.memory.snapshots.values())));
21+
public max(dataProvider: NumberProvider): number | undefined {
22+
const numbers = this._items.map(dataProvider);
23+
return numbers.length > 0 ? ss.max(numbers) : undefined;
2924
}
3025

31-
public get memoryMax(): number | undefined {
32-
const numbers = this._items.map((metrics) => ss.max(Array.from(metrics.memory.snapshots.values())));
33-
return numbers.length > 0 ? ss.max(numbers) : undefined;
26+
public stddev(dataProvider: NumberProvider): number | undefined {
27+
const numbers = this._items.map(dataProvider);
28+
return numbers.length > 0 ? ss.standardDeviation(numbers) : undefined;
3429
}
3530
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ export interface TestCase {
1818
runs: number;
1919
tries: number;
2020

21-
// Test function that will be executed and given scenarios A and B result sets.
22-
// Each has exactly `runs` number of items.
23-
test(aResults: Metrics[], bResults: Metrics[]): Promise<boolean>;
21+
// Test function that will be executed and given a scenarios result set with exactly `runs` number of items.
22+
// Should returns true if this "try" should be accepted and collected.
23+
// If false is returned, `Collector` will retry up to `tries` number of times.
24+
shouldAccept(results: Metrics[]): Promise<boolean>;
2425
}
2526

2627
// A simple scenario that just loads the given URL.
@@ -37,9 +38,9 @@ export class JankTestScenario implements Scenario {
3738
public constructor(private _withSentry: boolean) { }
3839

3940
public async run(_: playwright.Browser, page: playwright.Page): Promise<void> {
40-
let url = path.resolve(`./test-apps/jank/${ this._withSentry ? 'with-sentry' : 'index' }.html`);
41+
let url = path.resolve(`./test-apps/jank/${this._withSentry ? 'with-sentry' : 'index'}.html`);
4142
assert(fs.existsSync(url));
42-
url = `file:///${ url.replace('\\', '/')}`;
43+
url = `file:///${url.replace('\\', '/')}`;
4344
console.log('Navigating to ', url);
4445
await page.goto(url, { waitUntil: 'load', timeout: 60000 });
4546
await new Promise(resolve => setTimeout(resolve, 5000));

0 commit comments

Comments
 (0)