Skip to content

Commit e471837

Browse files
authored
ci: Move replay metrics into dedicated package (#7115)
1 parent 3324324 commit e471837

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+1315
-10762
lines changed

.github/workflows/build.yml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -802,8 +802,8 @@ jobs:
802802
run: |
803803
echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1
804804
805-
replay_metrics:
806-
name: Replay Metrics
805+
overhead_metrics:
806+
name: Overhead metrics
807807
needs: [job_get_metadata, job_build]
808808
runs-on: ubuntu-20.04
809809
timeout-minutes: 30
@@ -828,18 +828,14 @@ jobs:
828828
path: ${{ env.CACHED_BUILD_PATHS }}
829829
key: ${{ env.BUILD_CACHE_KEY }}
830830

831-
- name: Setup
832-
run: yarn install
833-
working-directory: packages/replay/metrics
834-
835831
- name: Collect
836832
run: yarn ci:collect
837-
working-directory: packages/replay/metrics
833+
working-directory: packages/overhead-metrics
838834

839835
- name: Process
840836
id: process
841837
run: yarn ci:process
842-
working-directory: packages/replay/metrics
838+
working-directory: packages/overhead-metrics
843839
# Don't run on forks - the PR comment cannot be added.
844840
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
845841
env:

.vscode/launch.json

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,6 @@
3737
"internalConsoleOptions": "openOnSessionStart",
3838
"outputCapture": "std"
3939
},
40-
{
41-
"type": "node",
42-
"name": "Debug replay metrics collection script",
43-
"request": "launch",
44-
"cwd": "${workspaceFolder}/packages/replay/metrics/",
45-
"program": "${workspaceFolder}/packages/replay/metrics/configs/dev/collect.ts",
46-
"preLaunchTask": "Build Replay metrics script",
47-
},
48-
{
49-
"type": "node",
50-
"name": "Debug replay metrics processing script",
51-
"request": "launch",
52-
"cwd": "${workspaceFolder}/packages/replay/metrics/",
53-
"program": "${workspaceFolder}/packages/replay/metrics/configs/dev/process.ts",
54-
"preLaunchTask": "Build Replay metrics script",
55-
},
5640
// Run rollup using the config file which is in the currently active tab.
5741
{
5842
"name": "Debug rollup (config from open file)",

.vscode/tasks.json

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,13 @@
11
{
22
// See https://go.microsoft.com/fwlink/?LinkId=733558 for documentation about `tasks.json` syntax
3-
"version": "2.0.0",
4-
"tasks": [
5-
{
6-
"label": "Prepare nextjs integration test app for VSCode debugger",
7-
"type": "npm",
8-
"script": "predebug",
9-
"path": "packages/nextjs/test/integration/",
10-
"detail": "Link the SDK (if not already linked) and build test app",
11-
},
12-
{
13-
"label": "Build Replay metrics script",
14-
"type": "npm",
15-
"script": "build",
16-
"path": "packages/replay/metrics",
17-
}
18-
]
3+
"version": "2.0.0",
4+
"tasks": [
5+
{
6+
"label": "Prepare nextjs integration test app for VSCode debugger",
7+
"type": "npm",
8+
"script": "predebug",
9+
"path": "packages/nextjs/test/integration/",
10+
"detail": "Link the SDK (if not already linked) and build test app"
11+
}
12+
]
1913
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"packages/hub",
4848
"packages/integration-tests",
4949
"packages/integrations",
50+
"packages/overhead-metrics",
5051
"packages/nextjs",
5152
"packages/node",
5253
"packages/node-integration-tests",

packages/browser/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"karma-typescript-es6-transform": "^4.0.0",
4141
"karma-webkit-launcher": "^1.0.2",
4242
"node-fetch": "^2.6.0",
43-
"playwright": "^1.27.1",
43+
"playwright": "^1.31.1",
4444
"sinon": "^7.3.2",
4545
"webpack": "^4.30.0"
4646
},

packages/integration-tests/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@
3232
},
3333
"dependencies": {
3434
"@babel/preset-typescript": "^7.16.7",
35-
"@playwright/test": "^1.29.2",
35+
"@playwright/test": "^1.31.1",
3636
"babel-loader": "^8.2.2",
3737
"html-webpack-plugin": "^5.5.0",
3838
"pako": "^2.1.0",
39-
"playwright": "^1.29.2",
39+
"playwright": "^1.31.1",
4040
"typescript": "^4.5.2",
4141
"webpack": "^5.52.0"
4242
},

packages/replay/metrics/.eslintrc.cjs renamed to packages/overhead-metrics/.eslintrc.cjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module.exports = {
2-
extends: ['../.eslintrc.js'],
2+
extends: ['../../.eslintrc.js'],
33
ignorePatterns: ['test-apps'],
44
overrides: [
55
{
@@ -8,6 +8,9 @@ module.exports = {
88
'no-console': 'off',
99
'@typescript-eslint/no-non-null-assertion': 'off',
1010
'import/no-unresolved': 'off',
11+
'@sentry-internal/sdk/no-optional-chaining': 'off',
12+
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
13+
'jsdoc/require-jsdoc': 'off',
1114
},
1215
},
1316
],

packages/replay/metrics/README.md renamed to packages/overhead-metrics/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
# Replay performance metrics
1+
# Overhead performance metrics
22

3-
Evaluates Replay impact on website performance by running a web app in Chromium via Playwright and collecting various metrics.
3+
Evaluates Sentry & Replay impact on website performance by running a web app in Chromium via Playwright and collecting various metrics.
44

5-
The general idea is to run a web app without Sentry Replay and then run the same app again with Sentry and another one with Sentry+Replay included.
5+
The general idea is to run a web app without Sentry, and then run the same app again with Sentry and another one with Sentry+Replay included.
66
For the three scenarios, we collect some metrics (CPU, memory, vitals) and later compare them and post as a comment in a PR.
77
Changes in the metrics, compared to previous runs from the main branch, should be evaluated on case-by-case basis when preparing and reviewing the PR.
88

packages/replay/metrics/configs/ci/collect.ts renamed to packages/overhead-metrics/configs/ci/collect.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import { Metrics, MetricsCollector } from '../../src/collector.js';
2-
import { MetricsStats, NumberProvider } from '../../src/results/metrics-stats.js';
1+
import type { Metrics } from '../../src/collector.js';
2+
import { MetricsCollector } from '../../src/collector.js';
3+
import type { NumberProvider } from '../../src/results/metrics-stats.js';
4+
import { MetricsStats } from '../../src/results/metrics-stats.js';
35
import { JankTestScenario } from '../../src/scenarios.js';
46
import { printStats } from '../../src/util/console.js';
57
import { latestResultFile } from './env.js';
@@ -10,10 +12,12 @@ function checkStdDev(results: Metrics[], name: string, provider: NumberProvider,
1012
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is undefined`);
1113
return false;
1214
} else if (value > max) {
13-
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is larger than ${max}. Actual value: ${value}`);
15+
console.warn(
16+
`✗ | Discarding results because StandardDeviation(${name}) is larger than ${max}. Actual value: ${value}`,
17+
);
1418
return false;
1519
} else {
16-
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`)
20+
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`);
1721
}
1822
return true;
1923
}
@@ -31,19 +35,23 @@ const result = await collector.execute({
3135
async shouldAccept(results: Metrics[]): Promise<boolean> {
3236
await printStats(results);
3337

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)) {
38+
if (
39+
!checkStdDev(results, 'lcp', MetricsStats.lcp, 50) ||
40+
!checkStdDev(results, 'cls', MetricsStats.cls, 0.1) ||
41+
!checkStdDev(results, 'cpu', MetricsStats.cpu, 1) ||
42+
!checkStdDev(results, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024) ||
43+
!checkStdDev(results, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)
44+
) {
3945
return false;
4046
}
4147

4248
const cpuUsage = MetricsStats.mean(results, MetricsStats.cpu)!;
4349
if (cpuUsage > 0.85) {
4450
// Note: complexity on the "JankTest" is defined by the `minimum = ...,` setting in app.js - specifying the number of animated elements.
45-
console.warn(`✗ | Discarding results because CPU usage is too high and may be inaccurate: ${(cpuUsage * 100).toFixed(2)} %.`,
46-
'Consider simplifying the scenario or changing the CPU throttling factor.');
51+
console.warn(
52+
`✗ | Discarding results because CPU usage is too high and may be inaccurate: ${(cpuUsage * 100).toFixed(2)} %.`,
53+
'Consider simplifying the scenario or changing the CPU throttling factor.',
54+
);
4755
return false;
4856
}
4957

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
export const previousResultsDir = 'out/previous-results';
22
export const baselineResultsDir = 'out/baseline-results';
33
export const latestResultFile = 'out/latest-result.json';
4-
export const artifactName = 'replay-sdk-metrics'
4+
export const artifactName = 'replay-sdk-metrics';

packages/replay/metrics/configs/ci/process.ts renamed to packages/overhead-metrics/configs/ci/process.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const baseBranch = await Git.baseBranch;
1515
await GitHub.downloadPreviousArtifact(baseBranch, baselineResultsDir, artifactName);
1616
await GitHub.downloadPreviousArtifact(branch, previousResultsDir, artifactName);
1717

18-
GitHub.writeOutput('artifactName', artifactName)
18+
GitHub.writeOutput('artifactName', artifactName);
1919
GitHub.writeOutput('artifactPath', path.resolve(previousResultsDir));
2020

2121
const previousResults = new ResultsSet(previousResultsDir);
@@ -27,15 +27,15 @@ if (baseBranch != branch) {
2727
await prComment.addAdditionalResultsSet(
2828
`Baseline results on branch: <code>${baseBranch}</code>`,
2929
// We skip the first one here because it's already included as `Baseline` column above in addCurrentResult().
30-
baseResults.items().slice(1, 10)
30+
baseResults.items().slice(1, 10),
3131
);
3232
} else {
3333
await prComment.addCurrentResult(await ResultsAnalyzer.analyze(latestResult, previousResults), 'Previous');
3434
}
3535

3636
await prComment.addAdditionalResultsSet(
3737
`Previous results on branch: <code>${branch}</code>`,
38-
previousResults.items().slice(0, 10)
38+
previousResults.items().slice(0, 10),
3939
);
4040

4141
await GitHub.addOrUpdateComment(prComment);

packages/replay/metrics/configs/dev/collect.ts renamed to packages/overhead-metrics/configs/dev/collect.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Metrics, MetricsCollector } from '../../src/collector.js';
1+
import type { Metrics } from '../../src/collector.js';
2+
import { MetricsCollector } from '../../src/collector.js';
23
import { MetricsStats } from '../../src/results/metrics-stats.js';
34
import { JankTestScenario } from '../../src/scenarios.js';
45
import { printStats } from '../../src/util/console.js';
@@ -19,8 +20,10 @@ const result = await collector.execute({
1920

2021
const cpuUsage = MetricsStats.mean(results, MetricsStats.cpu)!;
2122
if (cpuUsage > 0.9) {
22-
console.error(`CPU usage too high to be accurate: ${(cpuUsage * 100).toFixed(2)} %.`,
23-
'Consider simplifying the scenario or changing the CPU throttling factor.');
23+
console.error(
24+
`CPU usage too high to be accurate: ${(cpuUsage * 100).toFixed(2)} %.`,
25+
'Consider simplifying the scenario or changing the CPU throttling factor.',
26+
);
2427
return false;
2528
}
2629
return true;
Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,38 @@
1-
{
2-
"private": true,
3-
"name": "metrics",
4-
"main": "index.js",
5-
"author": "Sentry",
6-
"license": "MIT",
7-
"type": "module",
8-
"scripts": {
9-
"build": "tsc",
10-
"deps": "yarn --cwd ../ build:bundle && yarn --cwd ../../tracing/ build:bundle",
11-
"dev:collect": "ts-node-esm ./configs/dev/collect.ts",
12-
"dev:process": "ts-node-esm ./configs/dev/process.ts",
13-
"ci:collect": "ts-node-esm ./configs/ci/collect.ts",
14-
"ci:process": "ts-node-esm ./configs/ci/process.ts"
15-
},
16-
"dependencies": {
17-
"@octokit/rest": "^19.0.5",
18-
"@types/node": "^18.11.17",
19-
"axios": "^1.2.2",
20-
"extract-zip": "^2.0.1",
21-
"filesize": "^10.0.6",
22-
"p-timeout": "^6.0.0",
23-
"playwright": "^1.29.1",
24-
"playwright-core": "^1.29.1",
25-
"simple-git": "^3.16.0",
26-
"simple-statistics": "^7.8.0",
27-
"typescript": "^4.9.4"
28-
},
29-
"devDependencies": {
30-
"ts-node": "^10.9.1"
31-
}
32-
}
1+
{
2+
"private": true,
3+
"version": "7.39.0",
4+
"name": "@sentry-internal/overhead-metrics",
5+
"main": "index.js",
6+
"author": "Sentry",
7+
"license": "MIT",
8+
"type": "module",
9+
"scripts": {
10+
"build": "tsc",
11+
"dev:collect": "ts-node-esm ./configs/dev/collect.ts",
12+
"dev:process": "ts-node-esm ./configs/dev/process.ts",
13+
"ci:collect": "ts-node-esm ./configs/ci/collect.ts",
14+
"ci:process": "ts-node-esm ./configs/ci/process.ts",
15+
"fix": "run-s fix:eslint fix:prettier",
16+
"fix:eslint": "eslint . --format stylish --fix",
17+
"fix:prettier": "prettier --write \"{src,test-apps,configs}/**/*.{ts,js,html,css}\"",
18+
"lint": "run-s lint:prettier lint:eslint",
19+
"lint:eslint": "eslint . --format stylish",
20+
"lint:prettier": "prettier --check \"{src,test-apps,configs}/**/*.{ts,js,html,css}\""
21+
},
22+
"dependencies": {
23+
"@octokit/rest": "^19.0.5",
24+
"@types/node": "^18.11.17",
25+
"axios": "^1.2.2",
26+
"extract-zip": "^2.0.1",
27+
"filesize": "^10.0.6",
28+
"p-timeout": "^6.0.0",
29+
"playwright": "^1.31.1",
30+
"playwright-core": "^1.29.1",
31+
"simple-git": "^3.16.0",
32+
"simple-statistics": "^7.8.0",
33+
"typescript": "^4.9.4"
34+
},
35+
"devDependencies": {
36+
"ts-node": "^10.9.1"
37+
}
38+
}

0 commit comments

Comments
 (0)