✅ Add E2E coverage for WebSocket collection before init() - #4939
Draft
bdibon wants to merge 7 commits into
Draft
Conversation
Lift the fake WebSocket inlined in the WebSocket observable spec into a shared core test helper, so upcoming WebSocket tickets drive instrumentation through one test double instead of growing their own copies. The utility follows the XHR mock's shape: it swaps the `WebSocket` global and registers its own cleanup, including resetting the observable singleton. The observable spec's test cases and assertions are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WebSocket joins fetch, XHR, console and runtime errors as a buffered data source, so a socket constructed before init() is recorded rather than lost. The subscription is unconditional: the WebSocket opt-in is only known at init(), and consulting it before instrumenting would miss every connection opened before then — which is the point of the feature. The opt-in is left to the consumer of the source. The full WebSocketContext union crosses the buffer unchanged, with no coalescing in the core buffering layer. Nothing consumes the new source yet, so there is no customer-visible change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WebSocket collection now consumes the buffered data observable instead of subscribing to the WebSocket observable directly, so connections opened before init() are replayed and reported as complete resource events. The opt-in gate (trackResources plus betaTrackWebSockets or the TRACK_WEBSOCKETS experimental feature) moves into the collection entry point, since it is not knowable until init(). RUM startup now calls it unconditionally; the returned stop handle is a no-op when the gate is closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collection layer only observes what webSocketObservable emits, so asserting that application handlers and payloads survive belongs with the instrumentation. Handler passthrough was already covered there; add the missing send-payload case and drop the collection-level duplicate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page setup built the SDK bundle load and the init() call together, with no way to run application script in between. That window — SDK loaded and instrumenting, but not configured yet — is exactly what the buffered data sources serve, and it was untestable end to end. `createTest().withPreInitScript(js)` now emits that script as `window.DD_PRE_INIT`, called from the init site of the bundle, npm and async setups. The script may return a promise, in which case init() waits for it to settle, so an asynchronous exchange can complete entirely before the SDK starts. The bundle setup now emits every SDK script tag before any init call, so the pre-init script runs with all of them instrumenting the page. Setups that don't own their init call site reject the option rather than silently ignoring it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WebSocket early data collection had unit coverage on both sides of the buffer boundary, but nothing exercised the pre-start → post-start strategy swap in a browser. The headline behaviour — a socket opened before DD_RUM.init() still reports — was unverified end to end. Two scenarios open a socket in the pre-init window: one exchanges a message and stays open until the test closes it, the other also closes before init() so the entire connection lifecycle exists only in the buffer. Both assert the message counts and that the timings are measured from the real constructor call rather than from init(). Verified red: both fail against the collection code from before the buffered source was consumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bundles Sizes Evolution
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 0595f6d | Docs | Datadog PR Page | Give us feedback! |
5 tasks
bdibon
force-pushed
the
boris.dibon/websocket-early-collection
branch
from
August 11, 2026 16:31
0386e91 to
fa87695
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Motivation
WebSocket early data collection has unit coverage on both sides of the buffer boundary, but nothing exercised the real pre-start → post-start strategy swap in a browser. The headline behaviour of the feature — a socket opened before
DD_RUM.init()still reports — was therefore unverified end to end.The blocker was in the test framework, not the test: the E2E page setup builds the SDK bundle load and the
init()call together into the page<head>, with no injection point between them. There was no way to run application script after the SDK loads but beforeinit(), which is exactly the window this feature serves.Follow-up to the WebSocket early-collection work, which put this out of scope on the explicit condition that it be done separately.
Changes
1. A pre-
init()script injection point in the E2E framework (first commit)createTest().withPreInitScript(js)runs application code in the window where the SDK is loaded and instrumenting the page, but not yet configured. The script is emitted as a memoisedwindow.DD_PRE_INITand called from the init call site of the bundle, npm and async setups.The script may return a promise, in which case
init()waits for it to settle. Without this,init()would run in the same tick as the script and only the synchronous part (theWebSocketconstructor) would land in the buffer — the handshake, the message exchange and the close would all take the live path, and the test would silently prove much less than it appears to.Two consequences worth a reviewer's attention:
<script src>tag before any init script. PreviouslyDD_LOGS.init()ran before the RUM bundle was fetched, which would put the pre-init script in a window where RUM is not yet instrumenting. This is the only change that affects tests not using the new option, hence the full-suite run below.appSetup,microfrontendSetup,salesforceSetup) throw if the option is set, rather than silently ignoring it and letting a future test pass for the wrong reason.The injection point is generic, not WebSocket-specific: fetch and XHR have the same untested pre-init window, so this unblocks equivalent coverage for them.
2. Two scenarios (second commit)
init(), closed by the test afterwards.init(), so the entire connection lifecycle exists only in the buffer and the connection no longer exists when the SDK starts.Both assert the pre-
init()message counts and that timings are measured from the real constructor call rather than frominit(), by comparingresource.websocket.start_time/end_timeagainst a marker recorded at the top ofinit().WebSocketPage.testBody()is deliberately untouched — the new pre-init script runs in the<head>before the body exists, so it publishes a handle onwindowinstead of reusing the DOM fixture.Test instructions
To confirm the tests are not vacuous, check out the WebSocket collection code from before it consumed the buffered data source and re-run — both new tests fail with no websocket resource event reaching the intake, while the other eight
before inittests in the suite still pass:git checkout 29abcf7d0 -- packages/browser-rum-core/src/boot/startRum.ts \ packages/browser-rum-core/src/domain/resource/webSocketCollection.ts CI=true yarn test:e2e -g "before init\(\)" # 2 failed, 8 passed git checkout HEAD -- packages/browser-rum-core/src/boot/startRum.ts \ packages/browser-rum-core/src/domain/resource/webSocketCollection.tsVerified locally:
yarn typecheck/yarn lint/yarn formatyarn test:unitCI=true yarn test:e2e(full suite, all 3 setups)CI=true yarn test:e2e -g "before init\(\)" --repeat-each=6CI=true yarn test:e2e -g "websocket" --repeat-each=3 --workers=10Note on flakiness: in the one full-suite local run, the two new tests were flaky-but-passing — as were 17 unrelated tests in the same overloaded 53-minute run, so those timeouts can't be confidently attributed to this change. A real race was found and closed regardless:
page.goto()resolves on the load event, which can now precede the deliberately-delayedinit(), so the scenarios wait explicitly onwindow.RUM_INIT_TIMEandwindow.preInitWebSocket.Checklist
🤖 Generated with Claude Code