feat(rum): keep collecting Session Replay in the page when a host bridge is present - #18
Merged
Merged
Conversation
…dge is present Add a `sessionReplayDirectUpload` init option. When the SDK runs inside a host application that injects `DatadogEventBridge` (an Electron renderer process, a mobile WebView), upstream hands Session Replay over to the host, and drops it entirely when the host does not declare the `records` capability. Our Electron SDK never takes it over, so Session Replay is simply lost there. The option makes the page keep the regular recorder and upload the segments itself, over the same intake connection a plain web page uses. The intake needs no change: the payload, the endpoint builder (including `proxy`) and the authentication are the ones a web page already uses. Three condition changes, marked `FLASHCAT FORK (n/3)`: 1. `recorderApi.ts` - the bridge check moved from `makeRecorderApi()` to `onRumStart()`, since it now depends on a configuration option that is not known when `makeRecorderApi()` runs. `makeRecorderApi()` keeps only the browser support check. 2. `startRecording.ts` - records go to `startSegmentCollection` instead of `startRecordBridge`. 3. `rumSessionManager.ts` - the session is `SAMPLED` so views carry `session.has_replay`, which is what the read side keys on. Segments are joined to a session by `(account, session id)`, and nothing overwrites the session id of a segment uploaded from the page. The stub session manager therefore reads the id the host actually uses through two new optional bridge methods, `getSessionId()` and `getAnonymousId()`, falling back to the placeholder id when a host built against an older SDK does not implement them. `getAnonymousId()` also fixes the anonymous id being always empty in a host application, which made unique user counts always zero. `sessionReplaySampleRate` still defaults to 0, so it must be set explicitly for anything to be recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… host bridge Two follow-ups from the first end-to-end round on Electron. Replay segments were all rejected with 401 and nothing reached storage. `overrideInitConfigurationForBridge()` replaces `clientToken` with the literal `'empty'` and `applicationId` with a placeholder, on the assumption that a page hosting a bridge never sends a request of its own. `sessionReplayDirectUpload` breaks that assumption, so both are now kept when the option is set. Verified by comparison during the round: same URL and payload, `dd-api-key=empty` gives 401, the real token gives 400, and a well formed segment with the real token and the real application id gives 202 and lands with an `acct_id`/`session_id` matching its session row. RUM events are unaffected, as they never go over HTTP. Giving the stub session an `anonymousId` also activated the `usr.id` backfill in `userContext.ts`, and every renderer process event started carrying `usr.id = anonymous_id`. That backfill is a Flashcat addition that the web needs, because the web baseline counts only `usr.id` and would otherwise miss every logged-out visitor. A host application does not need it: its baseline already prefers the anonymous id, which is stable across logins, so the copy only imports the web double-counting bug where one device counts as two users around a login. It is now skipped when a bridge provides the anonymous id, keyed on `canUseEventBridge()` rather than on the option, since the id is host-owned whenever a bridge is present. Web behavior is byte for byte unchanged, and a regression test pins it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The sessionReplayDirectUpload markers were numbered out of three while a fourth site already existed, and the userContext and rumSessionManager identity changes were tagged with them even though they belong to the host-provided identifier work and are reachable without the option. Number the four option sites 1/4..4/4 and point the identity comments at DatadogEventBridge instead.
The implementation folds an empty answer into "not implemented" with `||`, and host applications rely on that to say "no session right now" — but the interface did not say so, leaving the next reader free to tighten it to `??` and hand the rest of the SDK an empty session id.
A host application that owns the session id answers `''` through the bridge while it has none — a session that timed out and has not been renewed yet. The bridge wrapper folded that empty answer into `undefined`, the same value it returns for a host that does not implement `getSessionId()` at all, and the session manager stub then fell back to `STUB_SESSION_ID`. RUM events survive that: the host overrides their session id, or drops them. But Session Replay segments are uploaded by this page directly, so they reached the intake carrying a placeholder that every application built on the host shares. Records collected during the gap were both lost to the real session and mixed into a fake one shared across applications. The two answers are now distinct. `undefined` still means "this host does not answer for the session id" and still falls back to the placeholder. `''` means "this host has no session right now", and the stub reports no tracked session for it: `computeSegmentContext` then finds no session and segment creation stops, which is the behaviour segmentCollection already documents for a session that ends. Reusing the id the host held a moment ago would be just as wrong — it would attribute records to a session that has ended — so nothing is cached. The stub also watches the host session. The bridge is pull-only, so reading it again is the only way to notice a transition; that is how the regular session manager works too, polling its own store every STORAGE_POLL_DELAY and turning what it sees into expire/renew notifications. Mirroring it lets everything already subscribed to SESSION_EXPIRED / SESSION_RENEWED behave: the recorder flushes its pending segment and stops instead of holding it open across the end of the host session, and restarts on a fresh view — and so a fresh full snapshot, without which the next segment would be unplayable — once the host has a session again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fiona2016
marked this pull request as ready for review
August 6, 2026 12:39
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
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.
What
Adds a
sessionReplayDirectUploadinit option to@flashcatcloud/browser-rum, so that a page embedded in a host application keeps collecting Session Replay and uploads it itself, instead of handing it over to the host.Draft: do not merge, do not publish.
Baseline: this branch is cut from
publish(97bcf9b7, v0.0.6), which is this fork's mainline — notmain.mainis stuck 75 commits behind at the fork point; npm@flashcatcloud/browser-core@0.0.6matchespublish. An earlier revision of this work was mistakenly based onmain(PR #17, now closed).Why
When the SDK runs inside a host application that injects a
DatadogEventBridge(an Electron renderer process, a mobile WebView), upstream assumes the host takes over Session Replay. If the host does not declare therecordscapability, the whole recorder API is replaced by a no-op shell and no replay is collected at all.Our Electron SDK (
@flashcatcloud/electron-sdk) injects the bridge from its preload script but returns an empty capability list, so customers who had working Session Replay lose it as soon as they adopt the Electron SDK.The upstream design — the host receives raw records over
bridge.send('record', ...)and does the segmentation, compression and multipart upload itself — is a large amount of work and has no Electron implementation upstream to this day. This change takes the other route: let the page stay on the path it already uses on the web.The intake side needs no change. The payload, the endpoint builder (including
proxy, which applies to thereplaytrack exactly like torum) and the authentication (dd-api-key= the same client token) are the ones a plain web page already uses. This was verified end to end on dev: segments uploaded from an application of typeelectron, both direct and through addforwardproxy, return202, land in S3 and int_replay_segments, and the read endpoint hands back a presigned URL.The four change points
They are the only behavioral changes, each marked
FLASHCAT FORK (n/4)in the code so the fork delta stays easy to spot if an upstream patch ever has to be picked. Points 1-3 turn the recorder back on; point 4 is what makes the resulting uploads actually accepted, and was added after the first end-to-end round on Electron.packages/rum/src/boot/recorderApi.ts— the recorder is no longer disabled up front when a bridge is present without therecordscapability. That check moved frommakeRecorderApi()intoonRumStart(), because it now depends on a configuration option, and the configuration does not exist yet whenmakeRecorderApi()runs at module evaluation.makeRecorderApi()keeps only the browser support check; the resulting behavior for a host that does not opt in is unchanged (the strategy stays the pre-start one, soisRecording()isfalse,getReplayStats()andgetSessionReplayLink()areundefined, and no worker is ever started).packages/rum/src/boot/startRecording.ts— records go tostartSegmentCollection(segment + upload) instead ofstartRecordBridge(hand over to the host).packages/rum-core/src/domain/rumSessionManager.ts— the stub session is markedSAMPLED, so view events carrysession.has_replay. That flag is what the read side keys on to expose the replay; without it the segments would be uploaded but never reachable.packages/rum-core/src/boot/preStartRum.ts—overrideInitConfigurationForBridge()no longer replacesclientTokenwith the literal'empty'andapplicationIdwith a placeholder when the option is set. See below; without this every segment is rejected with 401.Plus one correctness fix that falls out of point 3, in
packages/rum-core/src/domain/contexts/userContext.ts— also below.Why point 4 is required, not cosmetic
overrideInitConfigurationForBridge()swaps the real credentials for placeholders:That is sound upstream, because the premise is that a page hosting a bridge never sends a request of its own. This PR breaks exactly that premise: the page starts sending replay requests again, and it was sending them with the string
'empty'as its credential.The first end-to-end round pinned it down by comparison — same URL, same payload, only the key changed:
dd-api-key=emptyacct_id/session_idmatching its session rowSo the join logic was fine all along and only the credential was blocking. The application id matters too, not just the token:
computeSegmentContext()builds segment metadata fromconfiguration.applicationId, andt_replay_segments.application_idis a NOT NULL column.Keeping both is safe for everything else. RUM events under a bridge go through
bridge.send()and never touch HTTP, so the credentials are irrelevant to them. And the renderer'sapplication.idis overwritten by the Electron main process inassembleRendererRumEventanyway, so restoring the real one introduces no inconsistency.sessionSampleRate: 100is left exactly as upstream has it.The
usr.idfixuserContext.tscontains a Flashcat-only line (not upstream):Giving the stub session an
anonymousIdin point 3 activated it — the first round measured every renderer process event carryingusr.id = anonymous_id, 6 of 6.That is unwanted here. A host application's unique-user baseline already prefers the anonymous id (
COALESCE(usr_anonymous_id, usr_id)), and that id is stable across logins, so the copy buys nothing — while importing the web's double-counting bug, where one device counts as two users before and after a login.fc-sdk-electrondeliberately does not backfill on the main process side, and the renderer now matches.Web behavior is deliberately untouched, since this file is shared by every web customer and that line is why web unique users are countable at all: the web baseline counts only
usr.id, so without the backfill every logged-out visitor is missed. The skip is therefore conditional.The condition is
canUseEventBridge(), not the new option. The anonymous id becomes host-owned as soon as a bridge supplies it, whether or not direct upload is enabled, so gating on the option would leave the same double-counting in place for a host that providesgetAnonymousId()without opting into replay. Bridge presence is what actually delimits "this id is ours" from "this id is the host's". It is evaluated once, since a bridge is injected before any page script runs.Both sides are pinned by tests: the web backfill (which had no test before) and the bridge case where
usr.idstays absent whileusr.anonymous_idis set.Session id and anonymous id
This is the part the approach stands or falls on.
With a bridge present, the page uses
startRumSessionManagerStub(), whose session id is the hardcoded placeholder00000000-aaaa-0000-aaaa-000000000000. That is fine upstream because the host overwritessession.idon every RUM event it forwards — which our Electron main process does. But nobody overwrites the session id of a replay segment, since the segment is uploaded by the page directly. Segments are joined to a session by(account, session id), so a placeholder id means the replay is stored and never associated with a real session.The stub therefore reads the identifiers the host actually owns, through two new optional methods on the bridge:
Both are feature-detected: a host built against an older SDK does not implement them, and the page falls back to the placeholder session id and to no anonymous id, rather than crashing. The session id is read on every
findTrackedSession()call, because the host renews it over time.getAnonymousId()also fixes a second problem: the stub session had noanonymousIdfield at all, sousr.anonymous_idwas always empty in a host application and unique user counts were always zero.The host side of the contract is implemented and verified in flashcatcloud/fc-sdk-electron#11.
Configuration
sessionReplaySampleRatedefaults to 0, and this option does not change that. EnablingsessionReplayDirectUploadalone records nothing. The sample rate is applied to the stub session as well, so it has to be set explicitly. This is called out in the option's TSDoc.The option has no effect outside a host application: without a bridge the page already records and uploads Session Replay.
Tests
New specs cover each branch:
recorderApi.spec.ts— the recorder starts when the bridge does not support records but the option is set (the existing "should not start recording" case is untouched and still passes).startRecording.spec.ts— with a bridge present and the option set, segments are sent over the SDK's own request and nothing goes throughbridge.send.rumSessionManager.spec.ts— the session isSAMPLEDwith the option set,OFFwhensessionReplaySampleRateis 0; the bridge-provided session id and anonymous id are used, the placeholder is used when the bridge does not implement the methods or returns an empty string, and the session id is re-read on each call.preStartRum.spec.ts— credentials are still replaced by placeholders by default, and kept when the option is set.userContext.spec.ts— the web backfill is pinned (it had no coverage before), and under a bridgeusr.idis not backfilled whileusr.anonymous_idis still set, with a customer-providedusr.idleft untouched.mockEventBridgegained optionalsessionId/anonymousId; omitting them leaves the methods off the mock entirely, which is what emulates an older host.No existing assertion was relaxed or removed.
configuration.spec.tswas updated only to include the new key in the exhaustiveRequired<RumInitConfiguration>object (the option is not reported to telemetry).Checks
Green on this baseline, with no assertion relaxed or removed:
yarn test:unit— 2669/2683 passing, 0 failures (baselinepublishis 2656/2670, 0 failures; the delta is exactly the 13 new specs).yarn typecheck— 0 errors.yarn lint— 0 problems.prettier --check— clean on every touched file.One local-setup note for reviewers:
packages/corehas to be built once (tsc -p packages/core/tsconfig.cjs.json && tsc -p packages/core/tsconfig.esm.json) beforeyarn lint/yarn typecheckresolve@flashcatcloud/browser-core. Without it you get ~246 spuriousimport/no-unresolvederrors and 4 type errors indeveloper-extension/. This is a build-state artifact, not a code issue.Follow-up: no collection while the host has no session
Review caught a hole in the identifier handling above. A host that owns the session id answers
''through the bridge while it has none — a session that timed out and has not been renewed.getEventBridge().getSessionId()folded that empty answer intoundefined, the same value it returns for a host that does not implement the getter at all, and the stub then fell back toSTUB_SESSION_ID.RUM events survive that: the host overrides their session id, or drops them. Session Replay segments do not — this PR has the page upload them directly, so they reached the intake carrying a placeholder that is a single constant shared by every application built on the host. Records collected during the gap were both lost to the real session and mixed into a fake one shared across applications. The window is real: only a click counts as end-user activity in the Electron host, so scrolling or typing for 15 minutes expires the session while the recorder keeps running.
What changed
eventBridge.ts—getSessionId()now returnsundefinedonly when the host does not implement the getter, and''when the host implements it and has no session. Any other falsy answer from an implementing host is normalised to''. The interface doc says so.rumSessionManager.ts—findTrackedSession()returnsundefinedfor''. Nothing is cached: reusing the id the host held a moment ago would attribute records to a session that has ended, which is the same bug with a different id.rumSessionManager.ts— the stub now watches the host session and turns transitions intoSESSION_EXPIRED/SESSION_RENEWED. It owns astop(), whichstartRumregisters as a cleanup task.Why this semantic
findTrackedSession()returningundefinedis already this SDK's "no session" state, and the replay pipeline already knows what to do with it —segmentCollection.tsdocuments it verbatim: "If the RUM session ends and no session id is available when creating a new segment, records will be ignored, until the session is renewed and a new session id is available." So segment creation pauses and resumes on its own. No new concept, no new code path.The watch exists because that alone is not enough. Without a
SESSION_EXPIRED, the recorder keeps a segment open across the end of the host session (up toSEGMENT_DURATION_LIMIT), and after a renewal the next segment starts from an incremental record with no full snapshot — unplayable. Both are whatpostStartStrategyalready handles for a regular session, so the stub feeds it the same two events.Polling is not a new mechanism either: the bridge is pull-only, and the regular session manager learns of its own expiry the same way, polling its store every
STORAGE_POLL_DELAYand turning the transitions into expire/renew notifications. The stub mirrors that, at the same interval, and only when the host actually answers for the session id — an older host never starts a timer.Tests
13 further specs, each verified by mutation (revert the behaviour, the spec fails):
getSessionId()folds''back intoundefinedfindTrackedSession()falls back to the placeholder for''eventBridge.spec.ts— the three answers are distinct, and a falsy answer from an implementing host normalises to''.rumSessionManager.spec.ts— no tracked session while the host reports none, never the placeholder, and a tracked session again on renewal;SESSION_EXPIREDon expiry,SESSION_RENEWEDon renewal, both when the host swaps one session for another without reporting a gap, the manager already answering with the new session when it notifies, nothing notified while the host session is unchanged, no watch for a host that does not implement the getter, and no notifications afterstop().The rest of the chain was already covered and is untouched:
computeSegmentContextreturnsundefinedwhen the session is not tracked, andaddRecordignores records with no context.yarn test:unit— 2682/2696 passing, 0 failures (was 2669/2683; the delta is exactly the 13 new specs).yarn typecheck,yarn lint,prettier --checkall clean.🤖 Generated with Claude Code