-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ci(replay): Overhead measurement #6611
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
Changes from all commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
6fda61a
chore: replay metrics collection - initial script setup
vaind b333fbe
chore: replay metrics LCP collection
vaind a65e18c
cls vital collection
vaind be39bc5
replay: fid metric
vaind d57e56c
replay metrics - cpu stats
vaind cda998c
metrics memory profiler
vaind bcac89c
replay metrics - refactor scenarios
vaind 68300fc
chore: formatting
vaind 628a877
chore: refactoring
vaind 9f37006
define test cases for metrics collection
vaind 2c4aa3c
refactor: split up data collection
vaind 945845e
write collected metrics to file
vaind b0ec7e6
add metrics processing script
vaind d3f93d8
metrics: reading json result file
vaind 67a6dd8
replay matrics: jank test app copy
vaind c019cc6
fix JSON serialization
vaind 5658c6c
yank-test scenario
vaind 64f446f
collect jank test-app metrics with sentry included
vaind 601d67a
feat: analyze collected metrics
vaind 0df3970
metrics: ci configs, git & github utils (wip)
vaind 468f6b2
ci: collect sdk metrics
vaind f7873b3
metrics collection timeout
vaind 4df8bce
fix metrics collection on linux
vaind e38bbcc
await artifacts download
vaind 49b4131
fix gh authentication
vaind 940c072
metrics: add PR comment
vaind c8a71b7
improve pr comment
vaind 78c6a94
metrics: switch from puppeteer to playwright
vaind cef616c
fix PR commenting
vaind 9ee3943
first self-review
vaind 6133883
fixes
vaind d4d7f3b
chore - metrics linter issues
vaind 416ac54
more linter fixes
vaind d8297ec
metrics: improve PR comment contents
vaind 6397225
metrics: improve comment
vaind 2bfaa58
metrics: check stddev when collecting results
vaind 432591e
metrics: filter outliers
vaind 686a313
metrics: fix CPU usage collection
vaind 743221a
tune cpu usage
vaind cedef00
perf sampler error handling
vaind 7d1b2b4
metrics collection timeout issues
vaind ec925ab
metrics: fail on error logs
vaind be830af
move metrics CI job to build.yaml to get the full build cache
vaind 4d0a36c
tune metrics collection
vaind 3afa90b
ci metrics collection issues
vaind e713fab
tune CI metrics configs
vaind 2078d63
more CI tuning
vaind 0f2d1ac
docs
vaind d9b8244
show increase as ratio too
vaind b772b3e
fix: don't post PR comments on forks
vaind c3fcad9
feat: separate metrics measurements for sentry and sentry+replay
vaind bedeecf
fix and improve metrics analyzer console output
vaind 77fa886
review changes
vaind ef05d06
metrics collector: improve close/dispose error handling
vaind 355c4dc
collect metrics only if `ci-overhead-measurements` label is set on a PR
vaind File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,4 +47,4 @@ tmp.js | |
|
||
# eslint | ||
.eslintcache | ||
eslintcache/* | ||
**/eslintcache/* |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ build/ | |
demo/build/ | ||
# TODO: Check if we can re-introduce linting in demo | ||
demo | ||
metrics |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
module.exports = { | ||
extends: ['../.eslintrc.js'], | ||
ignorePatterns: ['test-apps'], | ||
overrides: [ | ||
{ | ||
files: ['*.ts'], | ||
rules: { | ||
'no-console': 'off', | ||
'@typescript-eslint/no-non-null-assertion': 'off', | ||
'import/no-unresolved': 'off', | ||
}, | ||
}, | ||
], | ||
}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
out |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# Replay performance metrics | ||
|
||
Evaluates 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. | ||
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. | ||
|
||
## Resources | ||
|
||
* https://github.com/addyosmani/puppeteer-webperf |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Replay metrics configuration & entrypoints (scripts) | ||
|
||
* [dev](dev) contains scripts launched during local development | ||
* [ci](ci) contains scripts launched in CI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { Metrics, MetricsCollector } from '../../src/collector.js'; | ||
import { MetricsStats, NumberProvider } from '../../src/results/metrics-stats.js'; | ||
import { JankTestScenario } from '../../src/scenarios.js'; | ||
import { printStats } from '../../src/util/console.js'; | ||
import { latestResultFile } from './env.js'; | ||
|
||
function checkStdDev(results: Metrics[], name: string, provider: NumberProvider, max: number): boolean { | ||
const value = MetricsStats.stddev(results, provider); | ||
if (value == undefined) { | ||
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}`); | ||
return false; | ||
} else { | ||
console.log(`✓ | StandardDeviation(${name}) is ${value} (<= ${max})`) | ||
} | ||
return true; | ||
} | ||
|
||
const collector = new MetricsCollector({ headless: true, cpuThrottling: 2 }); | ||
const result = await collector.execute({ | ||
name: 'jank', | ||
scenarios: [ | ||
new JankTestScenario('index.html'), | ||
new JankTestScenario('with-sentry.html'), | ||
new JankTestScenario('with-replay.html'), | ||
], | ||
runs: 10, | ||
tries: 10, | ||
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)) { | ||
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.'); | ||
return false; | ||
} | ||
|
||
return true; | ||
}, | ||
}); | ||
|
||
result.writeToFile(latestResultFile); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +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' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import path from 'path'; | ||
|
||
import { ResultsAnalyzer } from '../../src/results/analyzer.js'; | ||
import { PrCommentBuilder } from '../../src/results/pr-comment.js'; | ||
import { Result } from '../../src/results/result.js'; | ||
import { ResultsSet } from '../../src/results/results-set.js'; | ||
import { Git } from '../../src/util/git.js'; | ||
import { GitHub } from '../../src/util/github.js'; | ||
import { artifactName, baselineResultsDir, latestResultFile, previousResultsDir } from './env.js'; | ||
|
||
const latestResult = Result.readFromFile(latestResultFile); | ||
const branch = await Git.branch; | ||
const baseBranch = await Git.baseBranch; | ||
|
||
await GitHub.downloadPreviousArtifact(baseBranch, baselineResultsDir, artifactName); | ||
await GitHub.downloadPreviousArtifact(branch, previousResultsDir, artifactName); | ||
|
||
GitHub.writeOutput('artifactName', artifactName) | ||
GitHub.writeOutput('artifactPath', path.resolve(previousResultsDir)); | ||
|
||
const previousResults = new ResultsSet(previousResultsDir); | ||
|
||
const prComment = new PrCommentBuilder(); | ||
if (baseBranch != branch) { | ||
const baseResults = new ResultsSet(baselineResultsDir); | ||
await prComment.addCurrentResult(await ResultsAnalyzer.analyze(latestResult, baseResults), 'Baseline'); | ||
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) | ||
); | ||
} 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) | ||
); | ||
|
||
await GitHub.addOrUpdateComment(prComment); | ||
|
||
// Copy the latest test run results to the archived result dir. | ||
await previousResults.add(latestResultFile, true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { Metrics, 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'; | ||
import { latestResultFile } from './env.js'; | ||
|
||
const collector = new MetricsCollector(); | ||
const result = await collector.execute({ | ||
name: 'dummy', | ||
scenarios: [ | ||
new JankTestScenario('index.html'), | ||
new JankTestScenario('with-sentry.html'), | ||
new JankTestScenario('with-replay.html'), | ||
], | ||
runs: 1, | ||
tries: 1, | ||
async shouldAccept(results: Metrics[]): Promise<boolean> { | ||
printStats(results); | ||
|
||
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.'); | ||
return false; | ||
} | ||
return true; | ||
}, | ||
}); | ||
|
||
result.writeToFile(latestResultFile); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export const outDir = 'out/results-dev'; | ||
export const latestResultFile = 'out/latest-result.json'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { ResultsAnalyzer } from '../../src/results/analyzer.js'; | ||
import { Result } from '../../src/results/result.js'; | ||
import { ResultsSet } from '../../src/results/results-set.js'; | ||
import { printAnalysis } from '../../src/util/console.js'; | ||
import { latestResultFile, outDir } from './env.js'; | ||
|
||
const resultsSet = new ResultsSet(outDir); | ||
const latestResult = Result.readFromFile(latestResultFile); | ||
|
||
const analysis = await ResultsAnalyzer.analyze(latestResult, resultsSet); | ||
printAnalysis(analysis); | ||
|
||
await resultsSet.add(latestResultFile, true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"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.15.1", | ||
"simple-statistics": "^7.8.0", | ||
"typescript": "^4.9.4" | ||
}, | ||
"devDependencies": { | ||
"ts-node": "^10.9.1" | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.