Skip to content

ci: Move replay metrics into dedicated package #7115

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

Merged
merged 2 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -802,8 +802,8 @@ jobs:
run: |
echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1

replay_metrics:
name: Replay Metrics
overhead_metrics:
name: Overhead metrics
needs: [job_get_metadata, job_build]
runs-on: ubuntu-20.04
timeout-minutes: 30
Expand All @@ -828,18 +828,14 @@ jobs:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}

- name: Setup
run: yarn install
working-directory: packages/replay/metrics

- name: Collect
run: yarn ci:collect
working-directory: packages/replay/metrics
working-directory: packages/overhead-metrics

- name: Process
id: process
run: yarn ci:process
working-directory: packages/replay/metrics
working-directory: packages/overhead-metrics
# Don't run on forks - the PR comment cannot be added.
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
env:
Expand Down
16 changes: 0 additions & 16 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,6 @@
"internalConsoleOptions": "openOnSessionStart",
"outputCapture": "std"
},
{
Copy link
Member

Choose a reason for hiding this comment

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

l: I never used these launch scripts so my logaf is practically nonexistent but is there a particular reason to remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was more thinking why we would need them 😅

"type": "node",
"name": "Debug replay metrics collection script",
"request": "launch",
"cwd": "${workspaceFolder}/packages/replay/metrics/",
"program": "${workspaceFolder}/packages/replay/metrics/configs/dev/collect.ts",
"preLaunchTask": "Build Replay metrics script",
},
{
"type": "node",
"name": "Debug replay metrics processing script",
"request": "launch",
"cwd": "${workspaceFolder}/packages/replay/metrics/",
"program": "${workspaceFolder}/packages/replay/metrics/configs/dev/process.ts",
"preLaunchTask": "Build Replay metrics script",
},
// Run rollup using the config file which is in the currently active tab.
{
"name": "Debug rollup (config from open file)",
Expand Down
26 changes: 10 additions & 16 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
{
// See https://go.microsoft.com/fwlink/?LinkId=733558 for documentation about `tasks.json` syntax
"version": "2.0.0",
"tasks": [
{
"label": "Prepare nextjs integration test app for VSCode debugger",
"type": "npm",
"script": "predebug",
"path": "packages/nextjs/test/integration/",
"detail": "Link the SDK (if not already linked) and build test app",
},
{
"label": "Build Replay metrics script",
"type": "npm",
"script": "build",
"path": "packages/replay/metrics",
}
]
"version": "2.0.0",
"tasks": [
{
"label": "Prepare nextjs integration test app for VSCode debugger",
"type": "npm",
"script": "predebug",
"path": "packages/nextjs/test/integration/",
"detail": "Link the SDK (if not already linked) and build test app"
}
]
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"packages/hub",
"packages/integration-tests",
"packages/integrations",
"packages/overhead-metrics",
"packages/nextjs",
"packages/node",
"packages/node-integration-tests",
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"karma-typescript-es6-transform": "^4.0.0",
"karma-webkit-launcher": "^1.0.2",
"node-fetch": "^2.6.0",
"playwright": "^1.27.1",
"playwright": "^1.31.1",
"sinon": "^7.3.2",
"webpack": "^4.30.0"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@
},
"dependencies": {
"@babel/preset-typescript": "^7.16.7",
"@playwright/test": "^1.29.2",
"@playwright/test": "^1.31.1",
"babel-loader": "^8.2.2",
"html-webpack-plugin": "^5.5.0",
"pako": "^2.1.0",
"playwright": "^1.29.2",
"playwright": "^1.31.1",
"typescript": "^4.5.2",
"webpack": "^5.52.0"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module.exports = {
extends: ['../.eslintrc.js'],
extends: ['../../.eslintrc.js'],
ignorePatterns: ['test-apps'],
overrides: [
{
Expand All @@ -8,6 +8,9 @@ module.exports = {
'no-console': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'import/no-unresolved': 'off',
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
'jsdoc/require-jsdoc': 'off',
},
},
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Replay performance metrics
# Overhead performance metrics

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

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.
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.
For the three scenarios, we collect some metrics (CPU, memory, vitals) and later compare them and post as a comment in a PR.
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.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Metrics, MetricsCollector } from '../../src/collector.js';
import { MetricsStats, NumberProvider } from '../../src/results/metrics-stats.js';
import type { Metrics } from '../../src/collector.js';
import { MetricsCollector } from '../../src/collector.js';
import type { NumberProvider } from '../../src/results/metrics-stats.js';
import { MetricsStats } from '../../src/results/metrics-stats.js';
import { JankTestScenario } from '../../src/scenarios.js';
import { printStats } from '../../src/util/console.js';
import { latestResultFile } from './env.js';
Expand All @@ -10,10 +12,12 @@ function checkStdDev(results: Metrics[], name: string, provider: NumberProvider,
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is undefined`);
return false;
} else if (value > max) {
console.warn(`✗ | Discarding results because StandardDeviation(${name}) is larger than ${max}. Actual value: ${value}`);
console.warn(
`✗ | Discarding results because StandardDeviation(${name}) is larger than ${max}. Actual value: ${value}`,
);
return false;
} else {
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`)
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`);
}
return true;
}
Expand All @@ -31,19 +35,23 @@ const result = await collector.execute({
async shouldAccept(results: Metrics[]): Promise<boolean> {
await printStats(results);

if (!checkStdDev(results, 'lcp', MetricsStats.lcp, 50)
|| !checkStdDev(results, 'cls', MetricsStats.cls, 0.1)
|| !checkStdDev(results, 'cpu', MetricsStats.cpu, 1)
|| !checkStdDev(results, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024)
|| !checkStdDev(results, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)) {
if (
!checkStdDev(results, 'lcp', MetricsStats.lcp, 50) ||
!checkStdDev(results, 'cls', MetricsStats.cls, 0.1) ||
!checkStdDev(results, 'cpu', MetricsStats.cpu, 1) ||
!checkStdDev(results, 'memory-mean', MetricsStats.memoryMean, 1000 * 1024) ||
!checkStdDev(results, 'memory-max', MetricsStats.memoryMax, 1000 * 1024)
) {
return false;
}

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export const previousResultsDir = 'out/previous-results';
export const baselineResultsDir = 'out/baseline-results';
export const latestResultFile = 'out/latest-result.json';
export const artifactName = 'replay-sdk-metrics'
export const artifactName = 'replay-sdk-metrics';
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const baseBranch = await Git.baseBranch;
await GitHub.downloadPreviousArtifact(baseBranch, baselineResultsDir, artifactName);
await GitHub.downloadPreviousArtifact(branch, previousResultsDir, artifactName);

GitHub.writeOutput('artifactName', artifactName)
GitHub.writeOutput('artifactName', artifactName);
GitHub.writeOutput('artifactPath', path.resolve(previousResultsDir));

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

await prComment.addAdditionalResultsSet(
`Previous results on branch: <code>${branch}</code>`,
previousResults.items().slice(0, 10)
previousResults.items().slice(0, 10),
);

await GitHub.addOrUpdateComment(prComment);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Metrics, MetricsCollector } from '../../src/collector.js';
import type { Metrics } from '../../src/collector.js';
import { MetricsCollector } from '../../src/collector.js';
import { MetricsStats } from '../../src/results/metrics-stats.js';
import { JankTestScenario } from '../../src/scenarios.js';
import { printStats } from '../../src/util/console.js';
Expand All @@ -19,8 +20,10 @@ const result = await collector.execute({

const cpuUsage = MetricsStats.mean(results, MetricsStats.cpu)!;
if (cpuUsage > 0.9) {
console.error(`CPU usage too high to be accurate: ${(cpuUsage * 100).toFixed(2)} %.`,
'Consider simplifying the scenario or changing the CPU throttling factor.');
console.error(
`CPU usage too high to be accurate: ${(cpuUsage * 100).toFixed(2)} %.`,
'Consider simplifying the scenario or changing the CPU throttling factor.',
);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
{
"private": true,
"name": "metrics",
"main": "index.js",
"author": "Sentry",
"license": "MIT",
"type": "module",
"scripts": {
"build": "tsc",
"deps": "yarn --cwd ../ build:bundle && yarn --cwd ../../tracing/ build:bundle",
"dev:collect": "ts-node-esm ./configs/dev/collect.ts",
"dev:process": "ts-node-esm ./configs/dev/process.ts",
"ci:collect": "ts-node-esm ./configs/ci/collect.ts",
"ci:process": "ts-node-esm ./configs/ci/process.ts"
},
"dependencies": {
"@octokit/rest": "^19.0.5",
"@types/node": "^18.11.17",
"axios": "^1.2.2",
"extract-zip": "^2.0.1",
"filesize": "^10.0.6",
"p-timeout": "^6.0.0",
"playwright": "^1.29.1",
"playwright-core": "^1.29.1",
"simple-git": "^3.16.0",
"simple-statistics": "^7.8.0",
"typescript": "^4.9.4"
},
"devDependencies": {
"ts-node": "^10.9.1"
}
}
{
"private": true,
"version": "7.39.0",
"name": "@sentry-internal/overhead-metrics",
"main": "index.js",
"author": "Sentry",
"license": "MIT",
"type": "module",
"scripts": {
"build": "tsc",
"dev:collect": "ts-node-esm ./configs/dev/collect.ts",
"dev:process": "ts-node-esm ./configs/dev/process.ts",
"ci:collect": "ts-node-esm ./configs/ci/collect.ts",
"ci:process": "ts-node-esm ./configs/ci/process.ts",
"fix": "run-s fix:eslint fix:prettier",
"fix:eslint": "eslint . --format stylish --fix",
"fix:prettier": "prettier --write \"{src,test-apps,configs}/**/*.{ts,js,html,css}\"",
"lint": "run-s lint:prettier lint:eslint",
"lint:eslint": "eslint . --format stylish",
"lint:prettier": "prettier --check \"{src,test-apps,configs}/**/*.{ts,js,html,css}\""
},
"dependencies": {
"@octokit/rest": "^19.0.5",
"@types/node": "^18.11.17",
"axios": "^1.2.2",
"extract-zip": "^2.0.1",
"filesize": "^10.0.6",
"p-timeout": "^6.0.0",
"playwright": "^1.31.1",
"playwright-core": "^1.29.1",
"simple-git": "^3.16.0",
"simple-statistics": "^7.8.0",
"typescript": "^4.9.4"
},
"devDependencies": {
"ts-node": "^10.9.1"
}
}
Loading