[RUM-15529] Integrate Session Replay#131
Open
cdn34dd wants to merge 11 commits into
Open
Conversation
…transport strategies - Extract abstract base classes from BatchProducer and BatchConsumer, moving the concrete JSON-NDJSON / JSON-array-POST logic into GenericBatchProducer and GenericBatchConsumer under a new `generic/` sub-package. - Introduce BatchFactory to own producer/consumer creation, so BatchManager stays decoupled from concrete implementations. Future tracks can add a peer package and a dispatch case in BatchFactory without touching the scheduling or the file-rotation logic.
This comment has been minimized.
This comment has been minimized.
Implement end-to-end session replay for Electron: BrowserRecord events from renderer processes are buffered into Segments with per-session ZLIB compression, then persisted to disk via a dedicated batch pipeline and uploaded as multipart/form-data to the replay intake.
- Add "records" to getCapabilities() so the browser RUM SDK detects session replay support and starts emitting BrowserRecord events through the bridge. -Replace the synchronous ipcRenderer.sendSync(CONFIG_CHANNEL) with an async retry loop using ipcRenderer.invoke. The bridge now initialises immediately with safe defaults (privacy: mask, hosts: [location.hostname]) and polls for up to 5s (50 × 100ms) for the Electron SDK's CONFIG_CHANNEL handler to become available. This decouples the preload lifecycle from SDK init order — previously the config was silently dropped if init() had not been called before the BrowserWindow was created.
Aligns the main-process handler with the dd-trace preload patch, which replaced ipcRenderer.sendSync with ipcRenderer.invoke. The previous synchronous on/sendSync pairing required the SDK to be initialised before any BrowserWindow was created; the new invoke/handle pairing lets the preload retry asynchronously until the handler is available.
cdn34dd
force-pushed
the
carlosnogueira/RUM-15529/integrate-session-replay
branch
from
47d9edb to
3783730
Compare
Collaborator
Author
|
@codex review |
![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: 378373028c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 378373028c
ℹ️ 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".
bcaudan
reviewed
Jun 4, 2026
Comment on lines
+73
to
+93
| const rendererViewId = (event.data as { view?: { id?: string } }).view?.id; | ||
| const replayStats = | ||
| event.data.type === 'view' && rendererViewId ? this.getViewReplayStats?.(rendererViewId) : undefined; | ||
|
|
||
| const mainProcessAttributes = { | ||
| session: { id: session?.id }, | ||
| session: { | ||
| id: session?.id, | ||
| ...(replayStats ? { has_replay: true } : {}), | ||
| }, | ||
| application: { id: application?.id }, | ||
| container: { view: { id: view?.id }, source: 'electron' }, | ||
| ...(replayStats | ||
| ? { | ||
| _dd: { | ||
| replay_stats: { | ||
| segments_count: replayStats.segments_count, | ||
| segments_total_raw_size: replayStats.segments_total_raw_size, | ||
| }, | ||
| }, | ||
| } | ||
| : {}), |
Collaborator
There was a problem hiding this comment.
💬 suggestion: we should add tests somewhere for all that
Comment on lines
+36
to
+37
| private segmentIndexPerView = new Map<string, number>(); | ||
| private viewReplayStats = new Map<string, ViewReplayStats>(); |
Collaborator
There was a problem hiding this comment.
❗ warning: We should prevent these maps to grow indefinitely
Comment on lines
+61
to
+65
| if (event.lifecycle === LifecycleKind.SESSION_EXPIRED) { | ||
| this.flush('session_renew'); | ||
| } else if (event.lifecycle === LifecycleKind.SESSION_RENEW) { | ||
| this.nextCreationReason = 'session_renew'; | ||
| } |
Collaborator
There was a problem hiding this comment.
💬 suggestion: From Claude:
SESSION_EXPIRED flush uses wrong CreationReason -- causes premature deflate reset
flush('session_renew') resets the deflate stream at session expiry, but the new session may not start for minutes (it starts at next user activity). This means the fresh deflate context is created too early, and when SESSION_RENEW fires later it resets again, potentially producing a malformed first segment.
Fix:
if (event.lifecycle === LifecycleKind.SESSION_EXPIRED) {
this.flush('segment_duration_limit'); // flush pending; session is ending
} else if (event.lifecycle === LifecycleKind.SESSION_RENEW) {
this.deflate = new StreamingDeflate(); // fresh context for new session
this.nextCreationReason = 'session_renew';
}wdyt?
Collaborator
Author
There was a problem hiding this comment.
I've addressed the changes to this code in a previous comment, but regarding: this.flush('segment_duration_limit') I don't think that is correct, as that would label the new session's first segment as if it were triggered by a timer. I believe that init is the right value here.
Collaborator
There was a problem hiding this comment.
That seems a bit weird to use init here and also on addRecord 🤔
What was your reference implemenation?
- Move replay stats injection out of Assembly into a dedicated registerReplayContext hook; hooks now receive source and rendererViewId so any hook can act on renderer view events without Assembly owning replay-specific logic - Rename GenericBatch* → StandardBatch* and relocate to standard/; clarifies the distinction between the append-and-rotate strategy and the replay-specific one - Extract BatchConsumer abstract base class with shared file-scan, read, upload, and delete logic; StandardBatchConsumer and ReplayBatchConsumer each implement only buildRequest() - Delete BatchFactory and types.ts; inline configs into their respective classes and introduce batchConfig.types.ts for the BatchManager config - Add test coverage: BatchConsumer base behaviour, Assembly hook parameter forwarding, and registerReplayContext skip/inject logic
Clarify BatchProducer responsibilities between abstract and concrete classes: - Remove batchSize from BatchProducerConfig and the abstract class; StandardBatchProducer introduces StandardBatchProducerConfig for it - Move size-batching state and methods (currentBatchFile, currentBatchSize, getCurrentBatchPath, rotateBatch, flush override) from abstract to StandardBatchProducer as private members - Add protected initialize() to the abstract class to replace the duplicated ensureTrackDirectoryExists + rotateOrphanedBatches calls in each create() - ReplayBatchProducer.writeData now delegates the tmp→log rename to the shared renameBatchFile instead of reimplementing it inline - Fix lint errors: drop unused parameter in BatchConsumer.spec test stub,use mockProducerCreate directly to avoid unbound-method warning
- Tighten RawReplayEvent: remove unused track fields, type data as BrowserRecord, make view required and source explicit; bridge guards against missing view - Move StreamingDeflate to src/tools/ (shared, not replay-specific) and wrap all stream callbacks with monitor for error observability - Replace globalThis.setTimeout/clearTimeout with monitored browser-core equivalents; wrap compressSegment .then callback with monitor - Fix session lifecycle: move deflate reset and per-session map clearing to SESSION_RENEW; drop the non-spec session_renew creation reason in favour of init - Convert CreationReason from string union to const object matching the full rum-events-format schema enum - Raise SEGMENT_BYTES_LIMIT from 60 KB to 10 MB - browser IPC constraint does not apply in the Electron main process - Make estimatedSize O(1) by accumulating size incrementally in addRecord
- Chain pendingFlush with Promise.all instead of overwriting it: without this, a SESSION_EXPIRED flush followed by a new-session flush before the first compression completes causes stop() to miss the expired session's last segment on quit - Append a per-instance sequence counter to batch filenames (batch-<ts>-<seq>.tmp) so two segments flushed within the same millisecond no longer collide — the timestamp-only name could cause the second rename to silently overwrite the first on POSIX or fail on Windows
cdn34dd
force-pushed
the
carlosnogueira/RUM-15529/integrate-session-replay
branch
from
4626888 to
926b91c
Compare
- Instead of maintaining fragile unified-diff patch files across all workspaces, introduce scripts/patch-dd-trace-preload.cjs which finds the dd-trace preload in local node_modules (or the monorepo root) and replaces it on each install. The replacement preload (scripts/dd-trace-electron-preload.js) is identical to upstream except getCapabilities() returns '["records"]' to enable session replay. - dd-trace is now a plain npm dependency; each workspace runs the patch script via postinstall so independent yarn install calls stay patched automatically.
cdn34dd
force-pushed
the
carlosnogueira/RUM-15529/integrate-session-replay
branch
from
926b91c to
5182d1b
Compare
bcaudan
reviewed
Jun 18, 2026
| "LICENSE-3rdparty.csv" | ||
| ], | ||
| "scripts": { | ||
| "postinstall": "node bin/patch-dd-trace-preload.cjs", |
Collaborator
There was a problem hiding this comment.
❓ question: Do we rely on the postinstall to be executed in customers environemnt?
| export interface InternalContext { | ||
| session_id: string; | ||
| } | ||
| let segmentCollection: ReplayCollection | undefined; |
Collaborator
There was a problem hiding this comment.
🥜 nitpick: what about using consistent naming?
Suggested change
| let segmentCollection: ReplayCollection | undefined; | |
| let replayCollection: ReplayCollection | undefined; |
Comment on lines
+43
to
+44
| segmentCollection = new ReplayCollection(eventManager, config, sessionManager); | ||
| registerReplayContext(hooks, (viewId) => segmentCollection?.getViewReplayStats(viewId)); |
Collaborator
There was a problem hiding this comment.
💬 suggestion: what about passing the hooks to ReplayCollection and register the context there?
Comment on lines
+62
to
+81
| /** | ||
| * Registers a one-time `before-quit` handler that flushes pending transport | ||
| * data before allowing the process to exit. `preventDefault` defers the quit | ||
| * while the async flush runs; a 5-second fallback ensures the app never hangs. | ||
| * Using `once` means the handler removes itself — when `_flushTransport` calls | ||
| * `app.quit()` the second quit propagates without re-entering this handler. | ||
| */ | ||
| function setupBeforeQuitHandler(): void { | ||
| app.once( | ||
| 'before-quit', | ||
| monitor((event: Electron.Event) => { | ||
| event.preventDefault(); | ||
| const fallback = setTimeout(() => app.quit(), 5000); | ||
| void _flushTransport().finally(() => { | ||
| clearTimeout(fallback); | ||
| app.quit(); | ||
| }); | ||
| }) | ||
| ); | ||
| } |
Collaborator
There was a problem hiding this comment.
💬 suggestion: A couple of edge cases that we could easily handle:
- if a second quit is triggered while the flush is in progress (e.g. user hits Cmd+Q again, or the OS sends a quit signal), the handler has already been consumed and the app exits mid-flush
- If the fallback fires first:
- app.quit() is called → app starts quitting
- _flushTransport eventually settles → .finally() runs → clearTimeout(fallback) (no-op, already fired) → app.quit() again
With something like:
const onBeforeQuit = (event: Electron.Event) => {
event.preventDefault();
let done = false;
const doQuit = () => {
if (!done) {
done = true;
app.removeListener('before-quit', onBeforeQuit);
app.quit();
}
};
setTimeout(doQuit, 5000);
void _flushTransport().finally(doQuit);
};
app.on('before-quit', onBeforeQuit);| } | ||
|
|
||
| return { | ||
| session: { has_replay: true } as object, |
Collaborator
There was a problem hiding this comment.
💬 suggestion: is it needed?
Suggested change
| session: { has_replay: true } as object, | |
| session: { has_replay: true }, |
Comment on lines
8
to
+9
| sessionSampleRate: 100, | ||
| sessionReplaySampleRate: 100, |
Collaborator
There was a problem hiding this comment.
💬 suggestion: those two are not used in bridge mode
Suggested change
| sessionSampleRate: 100, | |
| sessionReplaySampleRate: 100, |
| * mock intake receives it with correct metadata. | ||
| */ | ||
|
|
||
| test.describe('session replay', () => { |
Collaborator
There was a problem hiding this comment.
💬 suggestion: we should add a scenario to exercise the mask configuration
| track: typeof EventTrack.REPLAY; | ||
| format: typeof EventFormat.REPLAY; | ||
| data: unknown; | ||
| view?: { id: string }; |
Collaborator
There was a problem hiding this comment.
I'd say either we rely on the contract and we'll see if it fails at some point or we enforce the contract and send some telemetry when it is not respected.
Comment on lines
+61
to
+65
| if (event.lifecycle === LifecycleKind.SESSION_EXPIRED) { | ||
| this.flush('session_renew'); | ||
| } else if (event.lifecycle === LifecycleKind.SESSION_RENEW) { | ||
| this.nextCreationReason = 'session_renew'; | ||
| } |
Collaborator
There was a problem hiding this comment.
That seems a bit weird to use init here and also on addRecord 🤔
What was your reference implemenation?
| * | ||
| * Segments are flushed when: | ||
| * - Duration limit (5s) is reached | ||
| * - Estimated byte size limit (60KB) is exceeded |
Collaborator
There was a problem hiding this comment.
💬 suggestion: outdated comment
Suggested change
| * - Estimated byte size limit (60KB) is exceeded | |
| * - Estimated byte size limit (10MB) is exceeded |
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
2 participants
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
Implement session replay for the Electron SDK. Browser RUM SDK records DOM activity in renderer processes and sends those records through the
DatadogEventBridgeto the main process where these are buffered and compressed them into segments, and later sent to the Datadog session replay intake, enabling replay playback in the DD UI for Electron apps.Example session: https://mobile-integration.datadoghq.com/rum/replay/sessions/0b8c44f0-e8f7-4c0b-9ea8-0b0ec84d1993?applicationId=056b6201-48cb-4acc-9fbc-48507b8e3e12&seed=fe0ea26d-27d2-4108-9f30-296ff56af2bd&ts=1780479119880
Changes
Transport layer refactor (prerequisite)
BatchProducerandBatchConsumerare now abstract base classes, moving the concrete JSON-NDJSON / JSON-array-POST logic intoGenericBatchProducerandGenericBatchConsumerundersrc/transport/batch/generic/. A newBatchFactoryowns producer/consumer creation and dispatches on track type, allowing future transports to plug in without touching the scheduling or file-rotation logic.Session replay pipeline
src/domain/replay/Segment.ts— buffersBrowserRecordobjects into a segment, tracking timestamps, record count, and full-snapshot flag.flush()produces the exact wire format the backend expects (records array merged with metadata, trailing\n).src/domain/replay/StreamingDeflate.ts— maintains a single stateful ZLIB stream across all segments in a session, producing output byte-for-byte compatible with Pako (the browser SDK's compressor). Implements the cumulative Adler-32 checksum required for backend stitching.src/domain/replay/ReplayCollection.ts— listens forRawReplayEvents on the event bus, buffers records into segments, handles view changes and session lifecycle (flush on view change, session expiry, size/duration limits), compresses, and emitsServerReplayEvents for the transport layer.src/transport/batch/replay/ReplayBatchProducer.ts— writes one atomic file per compressed segment (JSON metadata line + base64 compressed body,.tmp→.logrename for atomicity).src/transport/batch/replay/ReplayBatchConsumer.ts— reads the two-line format and POSTs multipart/form-data to the session replay intake endpoint.Assembly enrichment
Assemblynow accepts agetViewReplayStatscallback and injectssession.has_replay: trueand_dd.replay_statsinto outgoing RUM view events, so the DD UI shows the replay indicator on views that have recorded data.Bridge / IPC
BridgeHandler'sCONFIG_CHANNELhandler switched fromipcMain.on+sendSynctoipcMain.handle+invoke, decoupling the preload's config fetch from SDK init order."records"togetCapabilities()— without this the browser RUM SDK never emitsBrowserRecordevents — and to replace the blockingsendSyncconfig fetch with an async retry loop that tolerates late SDK initialisation.Graceful shutdown
app.once('before-quit', ...)flushes the in-flight replay segment to disk before process exit (5 s safety timeout), ensuring the last ~5 s of recording is not lost on orderly shutdown.Useful links
https://datadoghq.atlassian.net/wiki/spaces/PANA/pages/2419392855/RUM+Session+Replay+Recorder+architecture
https://datadoghq.atlassian.net/wiki/spaces/RUMP/pages/2491842579/RFC+-+Session+Replay+Writer+Reader
Checklist