feat: add Playwright-based RUM load generator for browser metrics#14
Conversation
Add a new browser-based load generator using Playwright to generate Real User Monitoring (RUM) metrics via the Dash0 Web SDK. This complements the existing Locust HTTP load generator. New features: - Playwright-based browser automation with headless support - 6 scenario types mirroring the Locust behaviors (dashboard, docking, crew, life-support, power, inventory) - Weighted random scenario selection - RUM data flush before browser close to ensure telemetry delivery - Docker Compose profile 'load-rum' for local development - Helm chart support with ENABLE_RUM_LOAD_GENERATOR env var Usage: # Docker Compose docker compose --profile load-rum up -d # Helmfile ENABLE_RUM_LOAD_GENERATOR=true helmfile apply
MaxAnderson95
left a comment
MaxAnderson95
left a comment
There was a problem hiding this comment.
Pull Request Review
Code Review
Reviewed 23 files for the new Playwright-based RUM load generator. The overall architecture is well-designed with good separation of concerns (browser pool, scenarios, utilities). The Helm chart and Docker Compose integrations look solid.
Summary
Critical Issues:
- Missing
package-lock.jsonwill cause Docker build to fail
Bugs:
- Page event listeners not re-attached after error recovery
- Silent error swallowing without actual logging
Minor:
- Weighted selection algorithm rebuilds array on each call (acceptable for a load generator)
Positive Observations
- Clean scenario abstraction with
BaseScenarioclass - Good graceful shutdown handling with SIGTERM/SIGINT
- Proper RUM data flushing before browser close
- Comprehensive Helm chart schema validation
- Good use of
shm_sizefor Playwright in containers
Confidence Score: 4.5/10
The missing lock file is a blocking issue that will prevent the Docker image from building. Once fixed, this PR should be ready to merge.
| COPY package.json package-lock.json* ./ | ||
|
|
||
| # Install dependencies | ||
| RUN npm ci |
There was a problem hiding this comment.
Critical Bug: npm ci requires a package-lock.json file to exist, but no lock file is present in the rum-load-generator/ directory. This will cause the Docker build to fail with an error like:
npm ERR! The `npm ci` command can only install with an existing package-lock.json
To fix this, you need to generate a lock file:
cd rum-load-generator && npm installThen commit the generated package-lock.json file.
| log('pool', instance.id, 'Recreating page after error'); | ||
| await instance.page.close().catch(() => {}); | ||
| instance.page = await instance.context.newPage(); |
There was a problem hiding this comment.
Bug: When the page is recreated after an error, the event listeners (console, pageerror) set up in launchBrowser() are not re-attached to the new page. This means browser console errors and page errors won't be logged after a recovery event.
Consider extracting the listener setup into a helper method:
| log('pool', instance.id, 'Recreating page after error'); | |
| await instance.page.close().catch(() => {}); | |
| instance.page = await instance.context.newPage(); | |
| log('pool', instance.id, 'Recreating page after error'); | |
| await instance.page.close().catch(() => {}); | |
| instance.page = await instance.context.newPage(); | |
| this.attachPageListeners(instance.page, instance.id); |
And add a new private method:
private attachPageListeners(page: Page, id: number): void {
page.on('console', (msg) => {
if (msg.type() === 'error') {
log('browser', id, `Console error: ${msg.text()}`);
}
});
page.on('pageerror', (error) => {
log('browser', id, `Page error: ${error.message}`);
});
}| } catch { | ||
| // Log but don't fail - some data may still have been sent | ||
| } |
There was a problem hiding this comment.
The comment says "Log but don't fail" but no logging actually occurs. Consider adding actual logging for debugging purposes:
| } catch { | |
| // Log but don't fail - some data may still have been sent | |
| } | |
| } catch (error) { | |
| // Log but don't fail - some data may still have been sent | |
| console.warn('RUM flush warning:', error instanceof Error ? error.message : String(error)); | |
| } |
Alternatively, use the project's log utility for consistency.
| // Build weighted list | ||
| const weightedList: string[] = []; | ||
| for (const [scenario, weight] of Object.entries(weights)) { | ||
| for (let i = 0; i < weight; i++) { | ||
| weightedList.push(scenario); | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor Performance Note: This weighted selection rebuilds an array of ~100 elements on every scenario selection. While acceptable for a load generator, a more efficient approach would be cumulative weight selection:
const entries = Object.entries(weights);
const totalWeight = entries.reduce((sum, [, w]) => sum + w, 0);
let random = Math.random() * totalWeight;
for (const [scenario, weight] of entries) {
random -= weight;
if (random <= 0) {
return scenarioMap[scenario];
}
}This is a minor issue and can be addressed in a follow-up if desired.
- Add package-lock.json required for npm ci in Docker build - Extract attachPageListeners() and call it when recreating pages after errors to ensure console/pageerror events are still captured - Add actual logging in rum-flush.ts catch block instead of silent swallowing - Optimize weighted scenario selection using cumulative weights instead of building large arrays
MaxAnderson95
left a comment
MaxAnderson95
left a comment
There was a problem hiding this comment.
Follow-up Pull Request Review
Code Review
This is a follow-up review after the previous review findings were addressed in commit 752f2b0.
Previous Review Status
All four issues from the previous review have been properly addressed:
package-lock.jsonadded- Page event listeners now re-attached via
attachPageListeners()method - RUM flush errors now properly logged
- Weighted scenario selection optimized with cumulative weights
New Finding
Critical - Playwright Version Mismatch:
- The Dockerfile base image is
mcr.microsoft.com/playwright:v1.40.0-jammy - The package-lock.json installs Playwright
1.57.0 - This mismatch will likely cause browser launch failures at runtime because Playwright 1.57.0 expects browser binaries that don't exist in the 1.40.0 base image
Minor Observations
- No input validation for parsed integers in config.ts (could result in NaN if env vars are malformed)
- Kubernetes deployment lacks liveness/readiness probes (acceptable for a load generator)
Positive Notes
- Clean architecture with browser pool, scenarios, and utilities separation
- Good error recovery pattern in browser loop
- Proper graceful shutdown with SIGTERM/SIGINT handling
- Well-structured Helm chart with comprehensive schema validation
- Good use of
/dev/shmvolume mount for Playwright in containers
Confidence Score: 6.0/10
The Playwright version mismatch is a significant concern that should be verified. If the Docker build succeeds but browsers fail to launch at runtime, this would be the cause. The fix is straightforward - either update the base image to match the installed version, or pin the npm package version.
| @@ -0,0 +1,31 @@ | |||
| FROM mcr.microsoft.com/playwright:v1.40.0-jammy | |||
There was a problem hiding this comment.
Potential Bug - Version Mismatch: The base image uses Playwright v1.40.0, but package-lock.json installs Playwright 1.57.0. This mismatch may cause browser launch failures at runtime.
The Playwright base image bundles browser binaries for its specific version. When npm installs 1.57.0, it expects browser versions that aren't present in the 1.40.0 image.
Options to fix:
- Update the base image to match the lockfile version:
| FROM mcr.microsoft.com/playwright:v1.40.0-jammy | |
| FROM mcr.microsoft.com/playwright:v1.57.0-jammy |
- Or pin the npm package to match the base image (in package.json, change
^1.40.0to1.40.0and regenerate the lockfile)
| "lint": "eslint src/" | ||
| }, | ||
| "dependencies": { | ||
| "playwright": "^1.40.0" |
There was a problem hiding this comment.
The caret (^) version range allows npm to install newer minor/patch versions. The lockfile shows 1.57.0 is installed, but the Dockerfile uses a 1.40.0 base image.
If you want to stay on 1.40.0, pin to an exact version:
| "playwright": "^1.40.0" | |
| "playwright": "1.40.0" |
Alternatively, update the Dockerfile base image to v1.57.0.
…e and npm package Update base Docker image to v1.57.0-jammy and pin npm package to exact version 1.57.0 to prevent version mismatch that could cause browser launch failures at runtime.
MaxAnderson95
left a comment
MaxAnderson95
left a comment
There was a problem hiding this comment.
Follow-up Pull Request Review
Code Review
This is a follow-up review after previous review findings were addressed in commits 752f2b0 and c4d294e.
Previous Review Status
All issues from previous reviews have been properly addressed:
| Issue | Status |
|---|---|
Missing package-lock.json |
Fixed in 752f2b0 |
| Page event listeners not re-attached | Fixed via attachPageListeners() method |
| Silent error swallowing in RUM flush | Fixed - now logs errors properly |
| Weighted selection algorithm inefficiency | Fixed with cumulative weights |
| Playwright version mismatch (1.40.0 vs 1.57.0) | Fixed in c4d294e - both aligned to 1.57.0 |
Remaining Observations
Minor - Input Validation (noted in previous review, acceptable for load generator):
- Config parsing uses
parseInt()without NaN validation. Malformed env vars likeBROWSER_COUNT=abcwould result inNaN. - This is acceptable for a load generator where misconfiguration is operator error.
Minor - Kubernetes Best Practices:
- No liveness/readiness probes on the RUM deployment (acceptable for a load generator)
- No
securityContextset (Playwright requires certain capabilities, so this is reasonable)
Positive Observations
- Clean architecture with browser pool, scenarios, and utilities separation
- Excellent error recovery pattern with page recreation and listener re-attachment
- Proper graceful shutdown with SIGTERM/SIGINT handling
- Well-structured Helm chart with comprehensive schema validation
- Good use of
/dev/shmvolume mount for Playwright in containers - Playwright version now properly aligned between Docker image and npm package
- Efficient cumulative weight selection algorithm
Summary
All critical and blocking issues from previous reviews have been resolved. The PR is well-implemented with good error handling, proper RUM data flushing, and clean code organization. The remaining observations are minor and acceptable for a load testing utility.
Confidence Score: 8.5/10
This PR is ready to merge. The implementation is solid, all review findings have been addressed, and the code follows good practices for a browser-based load generator.
| @@ -0,0 +1,31 @@ | |||
| FROM mcr.microsoft.com/playwright:v1.57.0-jammy | |||
There was a problem hiding this comment.
Previous review finding addressed: Playwright version mismatch has been fixed. The base image is now v1.57.0-jammy, matching the exact version pinned in package.json. This ensures browser binaries in the image are compatible with the installed Playwright package.
| "lint": "eslint src/" | ||
| }, | ||
| "dependencies": { | ||
| "playwright": "1.57.0" |
There was a problem hiding this comment.
Previous review finding addressed: The playwright dependency is now pinned to exact version 1.57.0 (no caret prefix), preventing npm from installing newer versions that could drift from the Docker base image.
| instance.page = await instance.context.newPage(); | ||
| this.attachPageListeners(instance.page, instance.id); |
There was a problem hiding this comment.
Previous review finding addressed: The attachPageListeners() method is now called after recreating the page, ensuring console and pageerror event handlers are properly re-attached after error recovery.
| } catch (error) { | ||
| // Log but don't fail - some data may still have been sent | ||
| log('rum-flush', 0, 'RUM flush warning', { error: error instanceof Error ? error.message : String(error) }); |
There was a problem hiding this comment.
Previous review finding addressed: The catch block now properly logs RUM flush warnings using the project's logging utility, rather than silently swallowing errors.
| // Use cumulative weight selection for efficiency | ||
| const entries = Object.entries(weights); | ||
| const totalWeight = entries.reduce((sum, [, weight]) => sum + weight, 0); |
There was a problem hiding this comment.
Previous review finding addressed: The weighted selection algorithm now uses efficient cumulative weights rather than building a large array on each call. This reduces memory allocations and improves performance.
1 participant
Summary
Changes
New RUM Load Generator (
rum-load-generator/)Docker Compose Integration
load-rumprofile for running browser-based load testingRUM_BROWSERS,RUM_ITERATIONSHelm Chart Updates
locustandrumconfiguration sections to values.yamlENABLE_RUM_LOAD_GENERATORenvironment variable support in helmfilerum-deployment.yamltemplate for Kubernetes deploymentReview Fixes (752f2b0)
package-lock.jsonrequired fornpm ciin Docker buildReview Fixes (c4d294e)
playwrightnpm dependency to exact version1.57.0to prevent future version driftUsage
Docker Compose
Helmfile