test(sfu): add SFU capacity load test and client instrumentation - #63
test(sfu): add SFU capacity load test and client instrumentation#63Harxhit wants to merge 1 commit into
Conversation
- add load-test script and results CSV for SFU capacity testing - expose window globals for test timing (__csSocket, __csRoomId, __csLiveAt, __csJoinedAt, __csFirstFrameAt) - wire socket connect/disconnect lifecycle in Broadcaster; use connectSocket in ViewerPage - unprotect broadcaster/viewer/dashboard routes for load testing - log socket disconnect reason and details at error level Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/router/index.tsx">
<violation number="1" location="frontend/src/router/index.tsx:19">
P1: These routes are now siblings of `ProtectedRoute`, so React Router renders them without authentication or socket initialization. Move all three routes back inside the protected route.</violation>
</file>
<file name="load-test/sfu-capacity.js">
<violation number="1" location="load-test/sfu-capacity.js:13">
P0: This commits a reusable account password to the repository and makes the load test use that account in every environment. Read both values from `CROWDSTREAM_TEST_EMAIL` and `CROWDSTREAM_TEST_PASSWORD` instead of storing credentials in source.</violation>
<violation number="2" location="load-test/sfu-capacity.js:659">
P2: When `maxViewers` is not divisible by `batchSize`, the test stops below the requested viewer count and reports an incomplete capacity run. Launch a final batch sized to the remaining viewers, or otherwise loop until the actual count reaches `MAX_VIEWERS`.</violation>
<violation number="3" location="load-test/sfu-capacity.js:725">
P2: When signaling succeeds but no video frame arrives, the report records no failure because this count checks only `viewer.joined`. Count viewers without a finite `firstFrameLatencyMs` as failures so the capacity result reflects missing media.</violation>
</file>
<file name="backend/src/utils/socket.util.ts">
<violation number="1" location="backend/src/utils/socket.util.ts:144">
P2: Routine disconnections are now logged at `error` level. Disconnects are a normal event (user closes tab, network blip) and will be written to `logs/error.log` per the winston transport in `logging.ts` and counted by any error-based monitoring, flooding them with non-errors and masking real failures. Keep this at `info`, or add a separate debug/warn level if you need it prominent during the load test.</violation>
</file>
<file name="frontend/src/pages/ViewerPage.tsx">
<violation number="1" location="frontend/src/pages/ViewerPage.tsx:24">
P2: connectSocket() has side effects (opening the socket connection, attaching event listeners, mutating window.__csSocket) but is now called during the render body. Render-phase side effects violate React's purity contract and can open a connection for a render that is later discarded or re-run (e.g. under StrictMode/Suspense). Move the connection into the component's useEffect and keep the render body pure.</violation>
</file>
<file name="load-test/sfu-capacity-results.csv">
<violation number="1" location="load-test/sfu-capacity-results.csv:1">
P2: P50 equals P99 in every row for both join and first-frame latency, so these columns convey no actual latency distribution. Because the run used one viewer per batch, each 'percentile' is computed over a single sample and the P50/P99 labels are misleading to anyone reading the capacity results. Rerun with a meaningful batch size (or report per-viewer samples) so P50/P99 differ and the spread is visible.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const TEST_EMAIL = "harsxit04@gmail.com" | ||
| const TEST_PASSWORD = "@Harshit1308" |
There was a problem hiding this comment.
P0: This commits a reusable account password to the repository and makes the load test use that account in every environment. Read both values from CROWDSTREAM_TEST_EMAIL and CROWDSTREAM_TEST_PASSWORD instead of storing credentials in source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At load-test/sfu-capacity.js, line 13:
<comment>This commits a reusable account password to the repository and makes the load test use that account in every environment. Read both values from `CROWDSTREAM_TEST_EMAIL` and `CROWDSTREAM_TEST_PASSWORD` instead of storing credentials in source.</comment>
<file context>
@@ -0,0 +1,854 @@
+const BASE_URL = arg("baseUrl", "http://localhost");
+// const TOKEN = arg("token", null);
+
+const TEST_EMAIL = "harsxit04@gmail.com"
+const TEST_PASSWORD = "@Harshit1308"
+
</file context>
| const TEST_EMAIL = "harsxit04@gmail.com" | |
| const TEST_PASSWORD = "@Harshit1308" | |
| const TEST_EMAIL = process.env.CROWDSTREAM_TEST_EMAIL | |
| const TEST_PASSWORD = process.env.CROWDSTREAM_TEST_PASSWORD |
| <Route path="/signin" element={<SignInPage />} /> | ||
| <Route path="/signup" element={<SignUpPage />} /> | ||
|
|
||
| <Route path="/broadcaster" element={<BroadcasterPage />} /> |
There was a problem hiding this comment.
P1: These routes are now siblings of ProtectedRoute, so React Router renders them without authentication or socket initialization. Move all three routes back inside the protected route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/router/index.tsx, line 19:
<comment>These routes are now siblings of `ProtectedRoute`, so React Router renders them without authentication or socket initialization. Move all three routes back inside the protected route.</comment>
<file context>
@@ -16,10 +16,10 @@ export default function Router() {
<Route path="/signin" element={<SignInPage />} />
<Route path="/signup" element={<SignUpPage />} />
+ <Route path="/broadcaster" element={<BroadcasterPage />} />
+ <Route path="/viewer" element={<ViewerPage />} />
+ <Route path="/dashboard" element={<DashboardPage />} />
</file context>
|
|
||
| for ( | ||
| let target = BATCH_SIZE; | ||
| target <= MAX_VIEWERS; |
There was a problem hiding this comment.
P2: When maxViewers is not divisible by batchSize, the test stops below the requested viewer count and reports an incomplete capacity run. Launch a final batch sized to the remaining viewers, or otherwise loop until the actual count reaches MAX_VIEWERS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At load-test/sfu-capacity.js, line 659:
<comment>When `maxViewers` is not divisible by `batchSize`, the test stops below the requested viewer count and reports an incomplete capacity run. Launch a final batch sized to the remaining viewers, or otherwise loop until the actual count reaches `MAX_VIEWERS`.</comment>
<file context>
@@ -0,0 +1,854 @@
+
+ for (
+ let target = BATCH_SIZE;
+ target <= MAX_VIEWERS;
+ target += BATCH_SIZE
+ ) {
</file context>
| const failed = | ||
| batch.filter( | ||
| (viewer) => | ||
| !viewer.joined |
There was a problem hiding this comment.
P2: When signaling succeeds but no video frame arrives, the report records no failure because this count checks only viewer.joined. Count viewers without a finite firstFrameLatencyMs as failures so the capacity result reflects missing media.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At load-test/sfu-capacity.js, line 725:
<comment>When signaling succeeds but no video frame arrives, the report records no failure because this count checks only `viewer.joined`. Count viewers without a finite `firstFrameLatencyMs` as failures so the capacity result reflects missing media.</comment>
<file context>
@@ -0,0 +1,854 @@
+ const failed =
+ batch.filter(
+ (viewer) =>
+ !viewer.joined
+ ).length;
+
</file context>
| socket.on("disconnect", async (reason) => { | ||
| logger.info(`User disconnected ${socket.id} beacuse of ${reason}`) | ||
| socket.on("disconnect", async (reason, details) => { | ||
| logger.error(`User disconnected, ${socket.id} reason: ${reason} details: ${details}`) |
There was a problem hiding this comment.
P2: Routine disconnections are now logged at error level. Disconnects are a normal event (user closes tab, network blip) and will be written to logs/error.log per the winston transport in logging.ts and counted by any error-based monitoring, flooding them with non-errors and masking real failures. Keep this at info, or add a separate debug/warn level if you need it prominent during the load test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/utils/socket.util.ts, line 144:
<comment>Routine disconnections are now logged at `error` level. Disconnects are a normal event (user closes tab, network blip) and will be written to `logs/error.log` per the winston transport in `logging.ts` and counted by any error-based monitoring, flooding them with non-errors and masking real failures. Keep this at `info`, or add a separate debug/warn level if you need it prominent during the load test.</comment>
<file context>
@@ -140,8 +140,8 @@ io.on("connection", (socket) => {
- socket.on("disconnect", async (reason) => {
- logger.info(`User disconnected ${socket.id} beacuse of ${reason}`)
+ socket.on("disconnect", async (reason, details) => {
+ logger.error(`User disconnected, ${socket.id} reason: ${reason} details: ${details}`)
handleDisconnect(socket)
await stopFfmpegRecording(socket.id)
</file context>
| logger.error(`User disconnected, ${socket.id} reason: ${reason} details: ${details}`) | |
| logger.info(`User disconnected, ${socket.id} reason: ${reason} details: ${details}`) |
|
|
||
| export default function ViewerPage() { | ||
| const socket = getSocket() | ||
| const socket = connectSocket() |
There was a problem hiding this comment.
P2: connectSocket() has side effects (opening the socket connection, attaching event listeners, mutating window.__csSocket) but is now called during the render body. Render-phase side effects violate React's purity contract and can open a connection for a render that is later discarded or re-run (e.g. under StrictMode/Suspense). Move the connection into the component's useEffect and keep the render body pure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/pages/ViewerPage.tsx, line 24:
<comment>connectSocket() has side effects (opening the socket connection, attaching event listeners, mutating window.__csSocket) but is now called during the render body. Render-phase side effects violate React's purity contract and can open a connection for a render that is later discarded or re-run (e.g. under StrictMode/Suspense). Move the connection into the component's useEffect and keep the render body pure.</comment>
<file context>
@@ -20,7 +21,7 @@ interface Log {
export default function ViewerPage() {
- const socket = getSocket()
+ const socket = connectSocket()
const [searchParams] = useSearchParams();
</file context>
| @@ -0,0 +1,101 @@ | |||
| timestamp,viewers,joinP50,joinP99,firstFrameP50,firstFrameP99,failures | |||
There was a problem hiding this comment.
P2: P50 equals P99 in every row for both join and first-frame latency, so these columns convey no actual latency distribution. Because the run used one viewer per batch, each 'percentile' is computed over a single sample and the P50/P99 labels are misleading to anyone reading the capacity results. Rerun with a meaningful batch size (or report per-viewer samples) so P50/P99 differ and the spread is visible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At load-test/sfu-capacity-results.csv, line 1:
<comment>P50 equals P99 in every row for both join and first-frame latency, so these columns convey no actual latency distribution. Because the run used one viewer per batch, each 'percentile' is computed over a single sample and the P50/P99 labels are misleading to anyone reading the capacity results. Rerun with a meaningful batch size (or report per-viewer samples) so P50/P99 differ and the spread is visible.</comment>
<file context>
@@ -0,0 +1,101 @@
+timestamp,viewers,joinP50,joinP99,firstFrameP50,firstFrameP99,failures
+2026-08-19T03:33:16.173Z,1,341,341,362,362,0
+2026-08-19T03:33:20.123Z,2,346,346,388,388,0
</file context>
1 participant
Summary by cubic
Adds an SFU capacity load test that drives a real media session and measures join and first-frame latency as viewers are added in batches. Results show latency and failures climbing past ~65 concurrent viewers.
Test setup and instrumentation
load-test/sfu-capacity-results.csv.__csRoomId,__csLiveAt,__csJoinedAt, and__csFirstFrameAtonwindowso the test can time room creation, join, and first frame./broadcaster,/viewer, and/dashboardare no longer wrapped inProtectedRoute, so they load without authentication in every environment.getSockettoconnectSocket.Written for commit 24b6eae. Summary will update on new commits.