Skip to content

Commit 061c923

Browse files
vaindvaind
committed
metrics collection timeout issues
1 parent 4448f24 commit 061c923

File tree

8 files changed

+100
-55
lines changed

8 files changed

+100
-55
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Metrics, MetricsCollector } from '../../src/collector.js';
22
import { MetricsStats, NumberProvider } from '../../src/results/metrics-stats.js';
33
import { JankTestScenario } from '../../src/scenarios.js';
4+
import { printStats } from '../../src/util/console.js';
45
import { latestResultFile } from './env.js';
56

67
function checkStdDev(stats: MetricsStats, name: string, provider: NumberProvider, max: number): boolean {
@@ -26,17 +27,19 @@ const result = await collector.execute({
2627
tries: 10,
2728
async shouldAccept(results: Metrics[]): Promise<boolean> {
2829
const stats = new MetricsStats(results);
30+
printStats(stats);
31+
2932
if (!checkStdDev(stats, 'lcp', MetricsStats.lcp, 30)
3033
|| !checkStdDev(stats, 'cls', MetricsStats.cls, 0.1)
3134
|| !checkStdDev(stats, 'cpu', MetricsStats.cpu, 10)
32-
|| !checkStdDev(stats, 'memory-mean', MetricsStats.memoryMean, 30 * 1024)
33-
|| !checkStdDev(stats, 'memory-max', MetricsStats.memoryMax, 100 * 1024)) {
35+
|| !checkStdDev(stats, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024)
36+
|| !checkStdDev(stats, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)) {
3437
return false;
3538
}
3639

3740
const cpuUsage = stats.mean(MetricsStats.cpu)!;
38-
if (cpuUsage > 0.75) {
39-
console.error(`CPU usage too high and may be inaccurate: ${(cpuUsage * 100).toFixed(2)} %.`,
41+
if (cpuUsage > 0.85) {
42+
console.warn(`✗ | Discarding results because CPU usage is too high and may be inaccurate: ${(cpuUsage * 100).toFixed(2)} %.`,
4043
'Consider simplifying the scenario or changing the CPU throttling factor.');
4144
return false;
4245
}

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
1-
import { AnalyzerItemMetric, ResultsAnalyzer } from '../../src/results/analyzer.js';
1+
import { ResultsAnalyzer } from '../../src/results/analyzer.js';
22
import { Result } from '../../src/results/result.js';
33
import { ResultsSet } from '../../src/results/results-set.js';
4+
import { printAnalysis } from '../../src/util/console.js';
45
import { latestResultFile, outDir } from './env.js';
56

67
const resultsSet = new ResultsSet(outDir);
78
const latestResult = Result.readFromFile(latestResultFile);
89

910
const analysis = await ResultsAnalyzer.analyze(latestResult, resultsSet);
10-
11-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
12-
const table: { [k: string]: any } = {};
13-
for (const item of analysis.items) {
14-
table[AnalyzerItemMetric[item.metric]] = {
15-
value: item.value.diff,
16-
...((item.other == undefined) ? {} : {
17-
previous: item.other.diff
18-
})
19-
};
20-
}
21-
console.table(table);
11+
printAnalysis(analysis);
2212

2313
await resultsSet.add(latestResultFile, true);

‎packages/replay/metrics/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"axios": "^1.2.2",
2020
"extract-zip": "^2.0.1",
2121
"filesize": "^10.0.6",
22+
"p-timeout": "^6.0.0",
2223
"playwright": "^1.29.1",
2324
"playwright-core": "^1.29.1",
2425
"simple-git": "^3.15.1",

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

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import assert from 'assert';
1+
import pTimeout from 'p-timeout';
22
import * as playwright from 'playwright';
33

44
import { CpuUsage, CpuUsageSampler, CpuUsageSerialized } from './perf/cpu.js';
@@ -71,12 +71,16 @@ export class MetricsCollector {
7171
for (let run = 1; run <= testCase.runs; run++) {
7272
const innerLabel = `Scenario ${name} data collection, run ${run}/${testCase.runs}`;
7373
console.time(innerLabel);
74-
results.push(await this._run(scenario));
74+
try {
75+
results.push(await this._run(scenario));
76+
} catch (e) {
77+
console.warn(`${innerLabel} failed with ${e}`);
78+
break;
79+
}
7580
console.timeEnd(innerLabel);
7681
}
7782
console.timeEnd(label);
78-
assert.strictEqual(results.length, testCase.runs);
79-
if (await testCase.shouldAccept(results)) {
83+
if ((results.length == testCase.runs) && await testCase.shouldAccept(results)) {
8084
console.log(`Test case ${testCase.name}, scenario ${name} passed on try ${try_}/${testCase.tries}`);
8185
return results;
8286
} else if (try_ != testCase.tries) {
@@ -93,40 +97,44 @@ export class MetricsCollector {
9397
private async _run(scenario: Scenario): Promise<Metrics> {
9498
const disposeCallbacks: (() => Promise<void>)[] = [];
9599
try {
96-
const browser = await playwright.chromium.launch({
97-
headless: this._options.headless,
98-
100+
return await pTimeout((async () => {
101+
const browser = await playwright.chromium.launch({
102+
headless: this._options.headless,
103+
});
104+
disposeCallbacks.push(() => browser.close());
105+
const page = await browser.newPage();
106+
disposeCallbacks.push(() => page.close());
107+
108+
const cdp = await page.context().newCDPSession(page);
109+
110+
// Simulate throttling.
111+
await cdp.send('Network.emulateNetworkConditions', {
112+
offline: false,
113+
latency: PredefinedNetworkConditions[networkConditions].latency,
114+
uploadThroughput: PredefinedNetworkConditions[networkConditions].upload,
115+
downloadThroughput: PredefinedNetworkConditions[networkConditions].download,
116+
});
117+
await cdp.send('Emulation.setCPUThrottlingRate', { rate: cpuThrottling });
118+
119+
// Collect CPU and memory info 10 times per second.
120+
const perfSampler = await PerfMetricsSampler.create(cdp, 100);
121+
disposeCallbacks.push(async () => perfSampler.stop());
122+
const cpuSampler = new CpuUsageSampler(perfSampler);
123+
const memSampler = new JsHeapUsageSampler(perfSampler);
124+
125+
const vitalsCollector = await WebVitalsCollector.create(page);
126+
await scenario.run(browser, page);
127+
128+
// NOTE: FID needs some interaction to actually show a value
129+
const vitals = await vitalsCollector.collect();
130+
131+
return new Metrics(vitals, cpuSampler.getData(), memSampler.getData());
132+
})(), {
133+
milliseconds: 60 * 1000,
99134
});
100-
disposeCallbacks.push(async () => browser.close());
101-
const page = await browser.newPage();
102-
103-
const cdp = await page.context().newCDPSession(page);
104-
105-
// Simulate throttling.
106-
await cdp.send('Network.emulateNetworkConditions', {
107-
offline: false,
108-
latency: PredefinedNetworkConditions[networkConditions].latency,
109-
uploadThroughput: PredefinedNetworkConditions[networkConditions].upload,
110-
downloadThroughput: PredefinedNetworkConditions[networkConditions].download,
111-
});
112-
await cdp.send('Emulation.setCPUThrottlingRate', { rate: cpuThrottling });
113-
114-
// Collect CPU and memory info 10 times per second.
115-
const perfSampler = await PerfMetricsSampler.create(cdp, 100);
116-
disposeCallbacks.push(async () => perfSampler.stop());
117-
const cpuSampler = new CpuUsageSampler(perfSampler);
118-
const memSampler = new JsHeapUsageSampler(perfSampler);
119-
120-
const vitalsCollector = await WebVitalsCollector.create(page);
121-
122-
await scenario.run(browser, page);
123-
124-
// NOTE: FID needs some interaction to actually show a value
125-
const vitals = await vitalsCollector.collect();
126-
127-
return new Metrics(vitals, cpuSampler.getData(), memSampler.getData());
128135
} finally {
129-
disposeCallbacks.reverse().forEach((cb) => cb().catch(console.log));
136+
console.log('Disposing of browser and resources');
137+
disposeCallbacks.reverse().forEach((cb) => cb().catch(() => { /* silent */ }));
130138
}
131139
}
132140
}

‎packages/replay/metrics/src/perf/sampler.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export class PerfMetrics {
4444
export class PerfMetricsSampler {
4545
private _consumers: PerfMetricsConsumer[] = [];
4646
private _timer!: NodeJS.Timer;
47+
private _errorPrinted: boolean = false;
4748

4849
private constructor(private _cdp: playwright.CDPSession) { }
4950

@@ -72,6 +73,13 @@ export class PerfMetricsSampler {
7273
this._cdp.send('Performance.getMetrics').then(response => {
7374
const metrics = new PerfMetrics(response.metrics);
7475
this._consumers.forEach(cb => cb(metrics).catch(console.error));
75-
}, console.error);
76+
}, (e) => {
77+
// This happens if the browser closed unexpectedly. No reason to try again.
78+
if (!this._errorPrinted) {
79+
this._errorPrinted = true;
80+
console.log(e);
81+
this.stop();
82+
}
83+
});
7684
}
7785
}

‎packages/replay/metrics/src/util/console.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
1+
import { filesize } from 'filesize';
2+
3+
import { Analysis, AnalyzerItemMetric } from '../results/analyzer.js';
4+
import { MetricsStats } from '../results/metrics-stats.js';
15

26
export async function consoleGroup<T>(code: () => Promise<T>): Promise<T> {
37
console.group();
48
return code().finally(console.groupEnd);
59
}
10+
11+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
12+
type PrintableTable = { [k: string]: any };
13+
14+
export function printStats(stats: MetricsStats): void {
15+
console.table({
16+
lcp: `${stats.mean(MetricsStats.lcp)?.toFixed(2)} %`,
17+
cls: `${stats.mean(MetricsStats.cls)?.toFixed(2)} %`,
18+
cpu: `${((stats.mean(MetricsStats.cpu) || 0) * 100).toFixed(2)} %`,
19+
memoryMean: filesize(stats.mean(MetricsStats.memoryMean)),
20+
memoryMax: filesize(stats.max(MetricsStats.memoryMax)),
21+
});
22+
}
23+
24+
export function printAnalysis(analysis: Analysis): void {
25+
const table: PrintableTable = {};
26+
for (const item of analysis.items) {
27+
table[AnalyzerItemMetric[item.metric]] = {
28+
value: item.value.diff,
29+
...((item.other == undefined) ? {} : {
30+
previous: item.other.diff
31+
})
32+
};
33+
}
34+
console.table(table);
35+
}

‎packages/replay/metrics/test-apps/jank/app.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ document.addEventListener("DOMContentLoaded", function() {
2525
incrementor = 10,
2626
distance = 3,
2727
frame,
28-
minimum = 30,
28+
minimum = 50,
2929
subtract = document.querySelector('.subtract'),
3030
add = document.querySelector('.add');
3131

‎packages/replay/metrics/yarn.lock‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,11 @@ once@^1.3.1, once@^1.4.0:
337337
dependencies:
338338
wrappy "1"
339339

340+
p-timeout@^6.0.0:
341+
version "6.0.0"
342+
resolved "https://registry.yarnpkg.com/p-timeout/-/p-timeout-6.0.0.tgz#84c210f5500da1af4c31ab2768d794e5e081dd91"
343+
integrity sha512-5iS61MOdUMemWH9CORQRxVXTp9g5K8rPnI9uQpo97aWgsH3vVXKjkIhDi+OgIDmN3Ly9+AZ2fZV01Wut1yzfKA==
344+
340345
pend@~1.2.0:
341346
version "1.2.0"
342347
resolved "https://registry.yarnpkg.com/pend/-/pend-1.2.0.tgz#7a57eb550a6783f9115331fcf4663d5c8e007a50"

0 commit comments

Comments
 (0)