♻️ Route fetch, XHR, and console through bufferedDataObservable#4470
♻️ Route fetch, XHR, and console through bufferedDataObservable#4470thomas-lebeau merged 7 commits intov7from
Conversation
thomas-lebeau
force-pushed
the
thomas.lebeau/unify-buffered-observables
branch
from
e98e9d5 to
629186b
Compare
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: f7feeaa | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
thomas-lebeau
force-pushed
the
thomas.lebeau/unify-buffered-observables
branch
from
629186b to
496df82
Compare
- Add FETCH, XHR, and CONSOLE types to BufferedData discriminated union
- Subscribe to fetch/xhr/console observables in startBufferingData()
- Update Logs and RUM consumers to receive network and console events
via bufferedDataObservable instead of subscribing directly
- Remove early instrumentation (subscribe(noop)) from preStart phases
- Remove trackConsoleError (replaced by console handling in errorCollection)
- Convert fetchObservable/xhrObservable/consoleObservable from
BufferedObservable to plain Observable (buffering now centralized)
- Normalize BufferedData shape to use { type, data } consistently
thomas-lebeau
force-pushed
the
thomas.lebeau/unify-buffered-observables
branch
from
496df82 to
27309cc
Compare
…romRawError - Remove redundant fingerprint, causes, and dd_context tests from consoleCollection.spec.ts since error field construction is already tested in createErrorFieldFromRawError.spec.ts - Add actual cause entries in createErrorFieldFromRawError tests to cover the cases previously tested in consoleCollection - Rename remaining console test to clarify its purpose
- Verify responseBody is undefined when bodyUsed is true - Verify responseBody is undefined when bodyDisturbed is true
Due to FIFO subscriber ordering on singleton observables, the bufferedDataObservable subscriber and the direct subscriber fire in different orders depending on the SDK lifecycle phase. assignRequestIndex ensures the first caller wins, keeping REQUEST_STARTED and REQUEST_COMPLETED requestIndex values consistent.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3d96224bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) | ||
| }) | ||
| subscribe(BufferedDataType.RUNTIME_ERROR, mockable(trackRuntimeError)()) | ||
| subscribe(BufferedDataType.FETCH, initFetchObservable()) |
There was a problem hiding this comment.
startBufferingData() now subscribes to initFetchObservable() immediately, which makes this the first (and long-lived) subscriber. In createFetchObservable(), instrumentation is only installed when globalObject.fetch exists at first subscribe time; if a fetch polyfill is loaded later (common in older browsers), the early subscription never retries and fetch events are never collected for the rest of the session. Before this commit, the first fetch subscription happened during init(), so late-loaded polyfills could still be instrumented.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid but narrow — this only affects browsers without native fetch that load a polyfill after SDK load but before init().
Sincerely yours, Claude
| }) | ||
| subscribe(BufferedDataType.RUNTIME_ERROR, mockable(trackRuntimeError)()) | ||
| subscribe(BufferedDataType.FETCH, initFetchObservable()) | ||
| subscribe(BufferedDataType.XHR, initXhrObservable({ allowUntrustedEvents: true })) |
There was a problem hiding this comment.
Initializing XHR buffering with allowUntrustedEvents: true hard-codes the global XHR observable to accept untrusted events, because initXhrObservable() is singleton-based and keeps the first configuration. This bypasses user configuration where allowUntrustedEvents is false, so synthetic loadend events can now be processed and may generate incorrect or attacker-controlled request completions that were previously filtered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
User config isn't available pre-init(), so we default to allowUntrustedEvents: true. This is low risk while covering edge cases with some XHR polyfills.
Sincerely yours, Claude
- Build notification objects inline instead of mutating context in fetch error handler - Replace shallowClone with spread operator in XHR observable notification
…fered-observables
…fered-observables
2 participants
Motivation
Centralize the buffering of fetch, XHR, and console observables into
bufferedDataObservableinstead of each observable managing its own buffer. This simplifies the pre-start data flow and removes the need for early instrumentation (subscribe(noop)) and product-specific wrappers liketrackConsoleError.Changes
FETCH,XHR, andCONSOLEtypes to theBufferedDatadiscriminated union with a consistent{ type, data }shapestartBufferingData()so buffering is centralizedbufferedDataObservableinstead of subscribing directly to each observablefetchObservable/xhrObservable/consoleObservablefromBufferedObservableto plainObservable(buffering now centralized)subscribe(noop)) from pre-start phasestrackConsoleError(replaced by console handling inerrorCollection)Test instructions
yarn test:unitto verify all unit tests passChecklist