feat: identify the logged-in user from the main process (setUser) - #13
Merged
Conversation
Adds `setUser` / `getUser` / `clearUser`. Until now the SDK only knew the
device, through the anonymous id, so a logged-in session could not be
attributed to a person and Electron UV could only ever be anonymous UV.
The names match `flashcatRum.setUser()` in `@flashcatcloud/browser-rum`,
which is what customers already call one file away in the same
application. Upstream's `@datadog/electron-sdk` names it `setUserInfo`, a
name Datadog's own browser SDK has since dropped, and we do not track
that fork — consistency inside the product wins over parity with a fork
we are not following.
Scope is set/get/clear. No `extraInfo`, no account APIs: nothing needs
them yet.
`usr.anonymous_id` is untouched, and `usr.id` is still never backfilled
with it. The two coexist so unique users can be counted off
`COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))` across a
login. `clearUser` removes `usr.id` rather than blanking it, because
`NULLIF(usr_id, '')` treats an absent field and an empty string as
different rows. The existing `Object.keys(usr) === ['anonymous_id']`
guard is kept and extended rather than relaxed.
Only `id`, `name` and `email` are copied out of the caller's object,
which is what makes `setUser({ anonymous_id })` structurally impossible;
the hook also writes `anonymous_id` last so the guarantee does not rest
on the sanitizer alone. Upstream's `filterReservedKeys` excludes only the
standard fields from its `extraInfo` bag, so `addUserExtraInfo({
anonymous_id })` there overwrites the device id — recorded in a comment
so a later `extraInfo` reserves the key.
Two questions, two stores. `get()` answers "who is logged in now" from a
plain field, so `clearUser()` takes effect in the millisecond it happens.
Assembly asks `find(startTime)`, backed by a `DiskValueHistory` like
`ViewContext`'s, because a main-process event is not always assembled in
the moment it describes: a native crash is parsed on the next startup
carrying the crash's own timestamp. `set()` closes the previous entry and
opens the next from a single clock read, the millisecond race
`ViewCollection.createNewView` avoids the same way.
Renderer events are stamped in `Assembly.assembleRendererRumEvent`, which
needs no Browser SDK change; the bridge's `getUser()` and the identity
push exist for what a renderer uploads itself. The main process wins and
**replaces** rather than merges: `combine` merges per key and skips
`undefined`, so merging `{ id, name }` over `{ id, name, email }` would
emit one person's id beside another's email. The deciding reason is
`clearUser()` — a stale renderer identity surviving a logout would leave
the user's name and email on everything that window kept reporting.
`usr.anonymous_id` survives the replacement, and with no user set nothing
is touched, so renderer-only `setUser` callers are unaffected.
Two things dev verification caught, neither visible from unit tests:
- **Views resolve at emit time, not at their start time.** A view is an
interval, re-reported as it grows, and the backend takes the session's
identity from the last view row (`lastView.UserID`). The main process
emits one synthetic view per session and `setUser` cannot run before
`init`, so resolving at the view start kept `t_sessions.usr_id` empty
for the whole session while the errors beside it carried the identity.
- **A restored history is closed at startup.** Quitting is not logging
out, so the previous run's entry is still open on disk; restoring it
active attributed this process's events to whoever used the machine
last. Closing at exactly `now` was not enough — `find` treats `endTime`
as inclusive and the first view is created in the same millisecond as
`init`, so a second run stamped its opening view with the first run's
user. Closed one millisecond earlier, which is also the truthful bound.
Testing: `yarn test:unit` 657, `yarn test:e2e` 46, `yarn test:integration`
24. Every behaviour above has a guard that fails without it — 24 reverts
applied one at a time, all caught. `yarn typecheck`, `format:check`,
`test:e2e:typecheck` clean; `lint` still reports only the two pre-existing
errors on `publish`.
Verified against dev (appid `mKESnRV4wGs5nwcbTwotmW`): `t_sessions` shows
`usr_id` and `usr_anonymous_id` populated together, `t_errors` carries id,
name and email, and a second run on the same profile no longer inherits
the first run's identity.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bridge was rewritten under this branch, so the conflicts are all in the same place: the configuration channel this branch adds a user to. `IdentityUpdate` is gone. The push now carries the whole `BridgeConfig` rather than a session id, so a user field added to the configuration reaches renderers on its own — the second payload this branch had to extend no longer exists, and `pushConfig` needed nothing. The preload gained an `apply()` that takes a configuration wholesale, so the user is set there with everything else instead of by hand beside the session id, and the `identityPushed` flag folds into the `configPushed` one. `BridgeHandler`'s renderer set is gone too — it pushes to every live `webContents` — so the tests move from registering senders through a configuration request to declaring which renderers are alive. The case covering a configuration request without a sender goes with it: the handler no longer reads the sender, so it could not fail.
`commonContext` and `Assembly` reach `UserContext` for the identity in force, and it imports `electron` at module load for the path its history file lives at. Nothing in either suite touches that history, but the import alone is enough: without an Electron binary the module throws on load and takes both suites with it. That is what CI has, and why these two were the only ones failing there while the other thirty-five passed — they were the only ones reaching `electron` without mocking it. Reproduced by removing `node_modules/electron/path.txt`, which is what the module checks: the same error, and gone with the mock.
Fiona2016
marked this pull request as ready for review
August 7, 2026 12:08
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.
Adds
setUser/getUser/clearUserto the main process. Until now the SDK only knew the device — throughusr.anonymous_id(PR#11) — so a logged-in session could not be attributed to a person and Electron UV could only ever be anonymous UV.Branched from
publishafter #11 merged. Does not overlap #12 semantically: the four files both touch (src/index.ts,e2e/app/src/main.ts,e2e/app/src/preload.ts,e2e/lib/mainPage.ts) are edited in different regions.The API
Why
setUserand not upstream'ssetUserInfo.@flashcatcloud/browser-rum— the SDK the renderer of the same application uses, one file away — exposessetUser/getUser/clearUserand has nosetUserInfoat all. Upstream@datadog/electron-sdk0.7.0 usessetUserInfo, a name Datadog's own browser SDK has since dropped, and we do not track that fork. Consistency inside the product beats parity with a fork we are not following.Scope is set/get/clear. No
extraInfo, no account APIs — nothing needs them yet.The invariants this had to not break
usr.anonymous_idis untouched by all three. The two identifiers coexist so unique users are counted offCOALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, '')), which is stable across a login.usr.idis still never backfilled. Before the firstsetUserit is absent, not the anonymous id. The existingObject.keys(usr) === ['anonymous_id']test is kept and extended, not relaxed.clearUserremovesusr.id, never blanks it.NULLIF(usr_id, '')treats an absent field and an empty string as different rows.setUsercannot reachanonymous_id. Onlyid/name/emailare copied out of the caller's object, and the hook writesanonymous_idlast so the guarantee does not rest on the sanitizer alone.An invalid call is rejected whole rather than half-applied, and does not notify renderers — a partly-applied identity is harder to notice than none.
Two questions, two stores
get()answers "who is logged in now" from a plain field, soclearUser()takes effect in the millisecond it happens. Event assembly instead asksfind(startTime), backed by aDiskValueHistorylikeViewContext's, because a main-process event is not always assembled in the moment it describes — a native crash is parsed on the next startup carrying the crash's own timestamp, andaddErroraccepts a caller-suppliedstartTime. Resolving those at assembly time would hand one user's crash to another.set()closes the previous entry and opens the next from a single clock read — the millisecond raceViewCollection.createNewViewavoids the same way.Renderers: replacement, not a merge
Renderer events are stamped in
Assembly.assembleRendererRumEvent, which needs no Browser SDK change — the Browser SDK's bridge contract has no user getter.DatadogEventBridge.getUser()and thedatadog:bridge-identitypush exist for what a renderer uploads itself (Session Replay segments), and are what a future Browser SDK would read.The main process wins, and it replaces the identity rather than merging it.
combinemerges per key and skipsundefined, so merging a main-process{ id, name }over a renderer's{ id, name, email }would emit one person's id and name beside another's email — an identity belonging to nobody, and worse than either source alone. The deciding reason for precedence isclearUser(): a stale renderer identity surviving a logout would leave the user's name and email on everything that window kept reporting, so a logout has to be enforceable from one place.usr.anonymous_idsurvives the replacement, and when no user is set nothing is touched — applications that only callflashcatRum.setUser()in their renderers are unaffected.Two things only the dev run caught
Both passed every unit test before they were found.
1. Views must resolve at emit time, not at their start time. A view is an interval, re-reported as it grows (
_dd.document_version), and the backend takes the session's identity from the last view row (buildSessionViewUpdatesreadslastView.UserID). The main process emits exactly one synthetic view per session, andsetUsercannot run beforeinit— so resolving that view at its start time leftt_sessions.usr_idempty for the whole session while the error events beside it carried the full identity. Observed in Doris before the fix.2. A restored history has to be closed at startup. Quitting is not logging out, so a previous run normally leaves its entry open on disk; restoring it still-active attributed this process's events to whoever used the machine last. Closing it at exactly
nowwas not enough —findtreatsendTimeas inclusive and the first main-process view is created in the same millisecond asinit, so a second run stamped its opening view with the first run's user. Closed one millisecond earlier, which is also the truthful bound: the previous run ended strictly before this one began.Testing
yarn test:unit— 657 (600 onpublish)yarn test:e2e— 46 (37 onpublish)yarn test:integration— 24, unchangedyarn typecheck,yarn format:check,yarn test:e2e:typecheckclean.yarn lintstill reports only the two pre-existing errors onpublish(OperationCollection.tsindexed-object-style; theplayground/src/renderer.tsparse error that needs the playground installed).Mutation-verified. Every behaviour above was reverted one at a time — 24 reverts, each run against its guard spec, all 24 caught. Two guards were rewritten after the harness showed them passing against a broken implementation: the clock-race test did not exercise the race (no integer timestamp fell in the gap it created), and the restored-history test masked the inclusive boundary with a
tick().Verified against dev (appid
mKESnRV4wGs5nwcbTwotmW, staging intake):t_sessionscarriesusr_idandusr_anonymous_idtogethert_errorscarriesusr_id,usr_name,usr_emailalongsideusr_anonymous_idsetUser,usr_idis empty whileusr_anonymous_idis populateduserDataprofile no longer inherits the first run's identityNot in scope
addUserExtraInfoand the four account APIs upstream ships.getUser()— that is a Browser SDK change.🤖 Generated with Claude Code