-
Notifications
You must be signed in to change notification settings - Fork 162
[RUM-12600] Continuous Benchmarking #3927
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
base: main
Are you sure you want to change the base?
Conversation
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 0558c91 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
f8d98a3 to
036f257
Compare
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
| if (request.headers['accept-encoding']?.includes('deflate')) { | ||
| uploadBytes += postData.length | ||
| } else { | ||
| uploadBytes += new TextEncoder().encode(postData).length | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ question: What's the logic behind this? The accept-encoding headers tells the server that it can respond with encoded data. FMU this has nothing to do with postData?
Also yeah one limitation of the CDP is that it doesn't expose the request as binary... but I'm not sure encoding the postData to UTF-8 is a good strategy either? 🤔
This is why I went with the proxy route BTW.
benchmark/helpers.ts
Outdated
| document.addEventListener("readystatechange", (event) => { | ||
| if (document.readyState === "interactive") { | ||
| loadSDK() | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ question: I'm curious why we need this? We fixed an issue where the SDK is injected a bit too early #3153 . Did you observe another issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! In the /performances approach, we use import(bundle_url).then(...), but the issue is that the lazy-loaded module URL resolved by Webpack is incorrect.
I wanted to switch to the regular async setup, but Playwright’s addInitScript runs before any DOM element are accessible. (e.g. document.documentElement is undefined). That’s why I wait until document.readyState === "interactive".
| test.describe('benchmark', () => { | ||
| createBenchmarkTest('heavy').run(async (page, takeMeasurements) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought: I think there are more idiomatic ways to declare common test logic in playwright using fixtures. But personally I don't mind your approach, as I find it more explicit than using playwright magic.
| { | ||
| "compilerOptions": { | ||
| "baseUrl": ".", | ||
| "strict": true, | ||
| "skipLibCheck": true, | ||
| "experimentalDecorators": true, | ||
| "esModuleInterop": true, | ||
| "moduleResolution": "node", | ||
| "resolveJsonModule": true, | ||
| "target": "ES2022", | ||
| "module": "ES2020", | ||
| "types": ["node", "ajv"], | ||
| "allowJs": true, | ||
| "noEmit": true | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 suggestion: extend from the base tsconfig and remove redundant options.
eec95f4 to
17d8314
Compare
17d8314 to
25f6b65
Compare
Motivation
We want to monitor the impact of performance-intensive features like Replay and Profiling.
This PR introduces benchmark tests to measure their effect.
We track:
/performances, except it only collects the total consumption.For now, we run a single scenario,
heavy.scenario.ts, which executes a sample app from this PR.Current results:

Changes
Benchmark tests in
/benchmarkTest instructions
1- Run the sample app (from browser-sdk-test-playground)
2- Run the benchmark (from browser-sdk)
Checklist