From 06bb967f7a9e407dd614e56ee60ae63f593d9ec5 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 4 Sep 2026 07:27:00 -0700 Subject: [PATCH 1/2] feat(rum): apply a rate that leaves zero to the running session A session sample rate published above 0 now ends the session of a visitor whose session was drawn AT 0, so collection starts at their next interaction instead of waiting for that session to rotate -- up to four hours. This is the case where waiting shows an operator who has just switched collection on nothing at all, and nothing at all is indistinguishable from a broken integration. It joins the two changes that already did not wait: a stricter privacy level, and a rate of 0. Written against the rate the session was DRAWN at rather than against whether it is being collected, which is what keeps it honest. Re-drawing every session that is not collected would spare the winners and re-roll the losers, so a fleet drawn at 20 and moved to 50 would come out at 60. A rate of 0 is the one value with no winners to spare -- nothing was collected and no coin was flipped -- so re-drawing everyone lands exactly on the new rate. A rate rising from one real value to another therefore still waits. Answering that question needed a record a sampled-out session never had. Such a session is given no id, so its draw was not recorded at all and the rate it was drawn at fell back to init -- which reads a session that lost a draw at 30 as one drawn at 0 and re-draws it, the bias above. Its draw is now recorded in the same single entry as a collected session's, under an id no session can hold, so the two cannot read each other's. That sentinel shares one id across every sampled-out session, so the id check that makes a stale record inert for a collected session does nothing here. What replaces it is that the page which draws now owns the slot: reportDraw hands over every draw rather than only the ones worth keeping, so a draw that lands on the init values clears the record instead of leaving the previous session's behind to answer for it. Resolving a rate runs the site's beforeSampling callback, so it is asked only where the answer settles whether the session ends, not once per announcement for every visitor. --- CHANGELOG.md | 25 +++ .../src/domain/configuration/configuration.ts | 24 ++- .../src/domain/rumSessionManager.spec.ts | 171 ++++++++++++++-- .../rum-core/src/domain/rumSessionManager.ts | 182 +++++++++++++----- 4 files changed, 321 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c63ede6e4..696358c579 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,31 @@ --- +## Unreleased + +- āœØ A session sample rate published from the console that rises above 0 now ends the running + session of a visitor whose session was drawn at 0, so collection starts at their next interaction + instead of waiting for that session to end on its own ā€” up to four hours. This is the case where + waiting shows an operator who has just switched collection on nothing at all, which is + indistinguishable from a broken integration. It joins the two changes that already did not wait: + a stricter Session Replay privacy level, and a rate of 0. Nothing here happens without + `remoteConfigurationEnabled: true`. +- šŸ“ Only a session drawn AT 0 is re-drawn, not every session that is not being collected. Those + are different populations: a visitor who lost a draw at 30 had a coin flipped for them, and + re-rolling the losers while the winners keep their sessions would put the real rate above the + published one. While a rate of 0 is in force nothing is collected and no coin is flipped, so + re-drawing everyone lands exactly on the new rate. A rate rising from one real value to another + therefore still waits for the next session, as before. +- šŸ“ The rate a sampled-out session was drawn at is now recorded alongside the one a collected + session was drawn at, in the same single `localStorage` entry this SDK already keeps for the + draw. No new entry, no extra request. Without it a page that did not perform the draw ā€” the + second page of a visit, or another tab ā€” could not tell the two populations above apart. +- šŸ“ What you will see on the day you lift a rate off 0: visitors who were invisible start + appearing within seconds of loading a page rather than at their next session, so collected volume + climbs the same day rather than the next. That is the change taking effect, not a defect. + +--- + ## v0.2.2 - šŸ› The settings cache no longer grows by one entry per release of your site. Entries are keyed by diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 9ea4ca9ea7..f449070d4f 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -93,14 +93,22 @@ export interface RumInitConfiguration extends InitConfiguration { * values passed here, so they can be changed without releasing a new version of this site. * * A change applies to sessions started after it arrives, and a session already under way is never - * re-decided in place. Two changes do not wait for that session to end on its own, because their - * effect on it can be told without drawing again: a stricter `defaultPrivacyLevel`, and a session - * sample rate of 0. Both apply only while the visitor is being collected ā€” one who is not records - * nothing and sends nothing, so neither has anything to act on there. Either ends the current - * session, and the visitor's next action starts a new one under the new settings; the old session - * is collected to its end as it was begun, so no recording is left masked in one half and plain - * in the other. Every other change waits for the next session, a loosening privacy level and a - * rate rising to 100 included ā€” for "collect this visitor now" there is `setForcedSession()`. + * re-decided in place. Three changes do not wait for that session to end on its own, because + * their effect on it can be told without drawing again: a stricter `defaultPrivacyLevel`, and a + * session sample rate of 0, both while the visitor is being collected ā€” one who is not records + * nothing and sends nothing, so neither has anything to act on there ā€” and a rate above 0 for a + * visitor whose session was drawn AT 0, who was never in a draw at all and now could be. Any of + * the three ends the current session, and the visitor's next action starts a new one under the + * new settings; the old session is collected to its end as it was begun, so no recording is left + * masked in one half and plain in the other. + * + * Every other change waits for the next session, a loosening privacy level included, and so does + * a rate rising from one real value to another: only a second draw could say whether a session + * drawn at 30 should have been kept at 80, and drawing twice turns a rate p into pĀ². Re-drawing + * only the visitors who are not collected would spare the winners and re-roll the losers, which + * lifts the real rate above the published one. A rate of 0 is the one value with no winners to + * spare, which is why leaving it is decidable and leaving 30 is not. For "collect this one + * visitor now" at any rate, there is `setForcedSession()`. * * How soon "does not wait" is depends on when this client next hears of the change, and it hears * only at page load and at each new session. A visitor who keeps loading pages hears within diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 40878e2db0..186514a200 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -853,7 +853,51 @@ describe('rum session manager', () => { return getSessionState(SESSION_STORE_KEY).isExpired === '1' } - describe('the two changes it can decide on its own', () => { + describe('the three changes it can decide on its own', () => { + it('ends a session drawn at zero when the rate rises above it', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Nothing was collected and no coin was flipped, so re-drawing this visitor lands exactly + // on the new rate ā€” and until it happens an operator who has just switched collection on + // sees nothing at all, which is indistinguishable from broken. + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session drawn at zero even when the new rate is a partial one', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session drawn on an init rate of zero when the first settings deliver a rate', () => { + // Nothing in storage yet, so this session was drawn on the init values ā€” and a draw landing + // exactly on them records nothing, which is why zero can only be read back off init here. + // This is the application that never collects until the console says so. + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('collects the session that follows a rate lifted off zero', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + }) + it('ends a session being collected when the rate goes to zero', () => { storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) startWith({ sessionSampleRate: 100 }) @@ -946,25 +990,17 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeFalse() }) - it('leaves a session that is not being collected alone when the rate goes to a hundred', () => { - storeRemote({ version: 1, sessionSampleRate: 0 }) + it('leaves a session that lost a draw at a real rate alone when the rate rises', () => { + // The regression this exists to catch: re-drawing every session that is not collected, + // while leaving the collected ones alone, spares the winners and re-rolls the losers ā€” a + // fleet drawn at 30 and moved to 80 would come out well above 80. Only a session drawn at + // zero has no winner beside it to spare. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 30 }) startWith({ sessionSampleRate: 0 }) expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) - // The one rate whose outcome could be asserted and deliberately is not: `setForcedSession` - // already covers "collect this visitor now", raising volume unannounced is the one - // direction that surprises, and nothing about it is urgent. - deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) - - expect(expireSessionSpy).not.toHaveBeenCalled() - expect(isSessionEnded()).toBeFalse() - }) - - it('leaves a session that is not collected alone when the rate merely rises', () => { - storeRemote({ version: 1, sessionSampleRate: 0 }) - startWith({ sessionSampleRate: 0 }) - - deliver({ version: 2, sessionSampleRate: 30 }) + deliver({ version: 2, sessionSampleRate: 80 }) expect(expireSessionSpy).not.toHaveBeenCalled() expect(isSessionEnded()).toBeFalse() @@ -984,9 +1020,9 @@ describe('rum session manager', () => { }) it('does not end one sampled-out session after another as settings keep arriving', () => { - // A session that is not collected is given no id, so no record of its draw is kept and the - // level it was drawn under cannot be read back. Ending it would not change that, so acting - // on the comparison would end every session this visitor is ever given. + // Nothing is recorded for this visitor, so a stricter level has nothing to catch however + // many times it is announced. The rate stays at zero throughout, so the one rule that does + // act on a sampled-out session finds nothing to act on either. storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) startWith({ sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) @@ -1074,6 +1110,79 @@ describe('rum session manager', () => { }) describe('what it compares', () => { + it('does not answer for a sampled-out session with the record of the one it replaced', () => { + // A page that draws owns the record slot. Having drawn on the init values it has nothing to + // record, and leaving the previous session's record there would let it answer for this one: + // every sampled-out session is recorded under the same id, so unlike a collected session it + // cannot tell that the record describes somebody else. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 0 }) + const firstPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + firstPage.stop() + stopSessionManager() + + // The settings entry is gone ā€” swept as belonging to a release nobody runs any more ā€” so + // the draw that follows uses the init rate and has nothing to record. It loses too, so it + // is a sampled-out session that did not write the record it would be read under. + localStorage.removeItem(STORE_KEY) + expireCookie() + const secondPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + secondPage.stop() + stopSessionManager() + + // A third page restores that session instead of drawing one, so the record is the only + // thing it can read the draw off ā€” and the only record left would be the first session's. + startWith({ sessionSampleRate: 50 }) + expireSessionSpy.calls.reset() + + // Read off the first session's record this one looks drawn at zero and is re-drawn; read + // off init, which is what it was actually drawn at, it lost a draw at fifty and stays. + deliver({ version: 2, sessionSampleRate: 80 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('reads the rate a sampled-out session was drawn at back through storage', () => { + // The case that decides whether any of this reaches a real visitor: they were drawn at zero + // on the page before, and the page acting on the change never performed that draw. A + // sampled-out session is given no id, so its draw is recorded under one no session can + // hold ā€” without that record this page falls back to the init rate and answers wrongly. + storeRemote({ version: 1, sessionSampleRate: 0 }) + const firstPage = startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + firstPage.stop() + stopSessionManager() + + // A second page load restores the same session without drawing anything of its own. Init + // says 50 here on purpose: falling back to it would read this session as one that lost a + // draw and leave it alone, which is the answer the record exists to correct. + startWith({ sessionSampleRate: 50 }) + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('does not consult beforeSampling when no rate could decide anything', () => { + // Resolving the rate runs the site's own code, and an announcement is not a draw. It is + // asked only where the answer is what settles whether the session ends ā€” never once per + // announcement for every visitor. + const beforeSampling = jasmine.createSpy('beforeSampling').and.returnValue(undefined) + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 30 }) + startWith({ sessionSampleRate: 0, beforeSampling }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + beforeSampling.calls.reset() + + // This visitor lost a draw at thirty, so no rate the console publishes says anything about + // the session they are on, and there is nothing to ask. + deliver({ version: 2, sessionSampleRate: 80 }) + + expect(beforeSampling).not.toHaveBeenCalled() + }) + it('never draws again to reach its decision', () => { storeRemote({ version: 1, sessionSampleRate: 100 }) startWith({ sessionSampleRate: 100 }) @@ -1141,6 +1250,28 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeFalse() }) + it('stops re-drawing once the session that follows has lost a draw at the new rate', () => { + // The loop this could have become: the replacement is sampled out too, and if it were read + // as another session drawn at zero every further announcement would end it again. It was + // drawn at the new rate, and that is what its record says. + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + expect(isSessionEnded()).toBeTrue() + + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + expireSessionSpy.calls.reset() + + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + it('stops tightening the privacy level once the session is drawn under it', () => { storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 9e742043ee..90f6196876 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -99,6 +99,35 @@ export const enum SessionReplayState { FORCED, } +/** + * FLASHCAT FORK - the id the draw of a session that lost its lottery is recorded under. + * + * A session that is not collected is given no id ā€” see `sessionStore` ā€” so it has nothing to key a + * record on, and until this existed its draw was simply not recorded. That left the one question + * this SDK has to answer before it may re-draw such a visitor unanswerable: was this session drawn + * at a rate of 0, or did it lose a draw at some other rate? Getting that wrong in the second + * direction re-rolls losers while sparing winners, which quietly lifts a fleet's real sampling + * rate towards 100% ā€” see `endSessionIfSettingsAreDecisive`. + * + * Not a UUID, and not a value `generateUUID` can produce, so a record written here can never be + * mistaken for a real session's. The two are told apart by the id alone, which is what lets both + * share the single record slot: a collected session looks its own id up and a sampled-out one + * looks this up, and neither can read the other's. + * + * What it gives up, and why that is affordable: every sampled-out session matches this same id, so + * the id check that makes a stale record inert for a collected session does nothing here. What + * keeps a stale one from being read instead is that the page which draws owns the slot ā€” it writes + * its draw or clears the slot, in the same stack that created the session ā€” so the record always + * describes the most recent draw, and the most recent draw is what created the session being read. + * The gap left is the one this design already has for collected sessions and states two comments + * down: a tab polling storage between the session store's write and the record's would read the + * previous draw. A collected session falls back to init there; a sampled-out one reads the + * previous sampled-out draw's rate instead, which differs from its own only if the console moved + * the rate between two consecutive sessions of one visitor, and costs that visitor one extra + * re-draw when it does. + */ +const NOT_TRACKED_DRAW_ID = 'not-tracked' + export function startRumSessionManager( configuration: RumConfiguration, lifeCycle: LifeCycle, @@ -182,15 +211,32 @@ export function startRumSessionManager( const drawn = pendingDraw pendingDraw = undefined const sessionEntity = sessionManager.findSession() - if (!sessionEntity?.id) { + if (!sessionEntity) { return } + // A session that lost its draw has no id to be recorded under, so it is recorded under an id no + // session can hold. It has to be recorded at all for the same reason a collected one does ā€” the + // rate it was drawn at is not something a later page can work out, and here it decides whether + // a console change away from 0 may re-draw this visitor at once. Which of the two is read back + // follows from the session itself, so no record can be read for a session it does not describe. + const drawId = sessionEntity.id || NOT_TRACKED_DRAW_ID if (drawn) { - writeDrawRecord(configuration, { id: sessionEntity.id, ...drawn }) - drawnHistory.add(drawn, startTime) + // The page that draws owns the slot, and says so either way. A record is only worth keeping + // when it says something the init values do not ā€” but leaving the previous one in place + // instead would let it outlive the session it described, and a sampled-out session cannot + // spot that the way a collected one does: it matches on an id every sampled-out session + // shares. So a draw that has nothing to record clears the slot rather than passing over it. + // The cost is one `removeItem` per session drawn on a site that enabled none of this, which + // is a handful per visit. + if (isWorthRecording(configuration, drawn)) { + writeDrawRecord(configuration, { id: drawId, ...drawn }) + drawnHistory.add(drawn, startTime) + } else { + forgetDrawRecord(configuration) + } return } - const stored = readDrawRecord(configuration, sessionEntity.id) + const stored = readDrawRecord(configuration, drawId) if (stored) { drawnHistory.add(stored, startTime) } @@ -213,27 +259,37 @@ export function startRumSessionManager( // pages fetches the change within seconds and then carries on under the old decision for the rest // of their visit. // - // Two changes are not made to wait, and what makes exactly those two special is that their + // Three changes are not made to wait, and what makes exactly those three special is that their // outcome for the running session can be asserted without drawing again: // // - a stricter default privacy level: every further second recorded is a second of plaintext // uploaded, and masking cannot reach back for it. This is the one whose cost is not // recoverable, and the reason the rest of this exists; - // - a session sample rate of 0: nothing is meant to be collected any more, and this is the - // emergency stop the console offers ā€” one that took four hours would not be one. + // - a session sample rate of 0 for a session being collected: nothing is meant to be collected + // any more, and this is the emergency stop the console offers ā€” one that took four hours + // would not be one; + // - a rate above 0 for a session that was drawn AT 0: this visitor was never in a draw at all, + // and now could be. Without it an application whose rate only ever comes from the console + // shows an operator who has just switched collection on precisely nothing, for as long as + // the sessions already running take to rotate ā€” and nothing at all is indistinguishable from + // broken. // - // Both are about a session that is being collected, which is why that is the first thing checked. - // A visitor who is not being collected records nothing and uploads nothing, so neither rule has - // anything to act on for them. + // The first two are about a session that is being collected, and the third only ever about one + // that is not, which is why each rule checks that for itself. // // No rate other than 0 says anything about whether THIS session should have been kept ā€” only a - // second draw could, and drawing twice silently turns a rate p into pĀ². A rate of 100 could be - // asserted about a session that is not collected, and deliberately is not acted on: `setForcedSession` - // already exists for "collect this visitor now", it is the one direction that raises volume - // unannounced, and nothing about it is urgent. So everything else waits for the next session, a - // loosening privacy level included. Loosening waits on purpose: the delay is what leaves an - // operator room to undo a mistake, and what it costs meanwhile is more of the data already being - // collected. + // second draw could, and drawing twice silently turns a rate p into pĀ². That is also why the + // third rule is written against the rate the session was DRAWN at rather than against whether it + // is being collected: re-drawing every session that is not collected, while leaving the collected + // ones alone, spares the winners and re-rolls the losers, so a fleet drawn at 20 and moved to 50 + // would come out at 60. A rate of 0 is the one value with no winners to spare ā€” nothing was + // collected, no coin was flipped ā€” so re-drawing everyone lands exactly on the new rate. And it + // costs nothing to end such a session: it has no id, no events and no history, so it does not + // exist in the data and ending it leaves no seam. + // + // Everything else waits for the next session, a loosening privacy level included. Loosening waits + // on purpose: the delay is what leaves an operator room to undo a mistake, and what it costs + // meanwhile is more of the data already being collected. // // The action is to end the session and let the next activity start a new one ā€” never to flip the // running one, which would leave a replay masked in its first half and plain in its second, or @@ -250,24 +306,37 @@ export function startRumSessionManager( // Any other tab of the same visitor that does load a page ends the session they share. function endSessionIfSettingsAreDecisive() { const session = sessionManager.findSession() - if (!session || !isTypeTracked(session.trackingType)) { - // Nothing here that ending would change. Whatever starts this visitor's next session draws - // on the settings just stored, which is the ordinary path and already gives them effect. - // - // It also could not be decided if we wanted to: a session that is not collected is given no - // id, so no record is kept of what it was drawn under. The comparison below would fall - // through to the init value on every announcement and keep answering "tighter", ending one - // empty session after another for as long as the visitor stayed. + if (!session) { return } const remote = readRemoteConfig(configuration.remoteConfig) + // What this session was created under, which is not the previously stored settings: settings + // are stored while a session runs, and the session was drawn under whatever was stored before + // that. No record means the draw used the init values ā€” `reportDraw` records every draw that + // did not, so a draw with nothing recorded is a draw that used them. + const drawn = drawnHistory.find() + + if (!isTypeTracked(session.trackingType)) { + // Nothing forced can reach this comparison as a zero: a forced draw is recorded at 100 and is + // collected besides, so the record already answers the question the tracked branch has to ask + // `forcedSession` about below. + const drawnSampleRate = drawn?.sessionSampleRate ?? configuration.sessionSampleRate + if (drawnSampleRate !== 0) { + return + } + // Asked only now, and only here, because resolving runs the site's `beforeSampling`: this + // announcement is not a draw, and the callback should be run no more often than a decision + // actually turns on its answer. + if (resolveSampleRates(configuration, remote).sessionSampleRate > 0) { + sessionManager.expire() + } + return + } - // What this session is masking pages with right now, which is not the previously stored - // settings: settings are stored while a session runs, and the session was drawn under whatever - // was stored before that. No record means the draw used the init value, and so does the - // recorder ā€” see `startRecording`, which falls back the same way. - const drawnPrivacyLevel = drawnHistory.find()?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + // What this session is masking pages with right now ā€” the recorder falls back to the init value + // the same way when there is no record, see `startRecording`. + const drawnPrivacyLevel = drawn?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel const nextPrivacyLevel = remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel if (PRIVACY_LEVEL_STRICTNESS[nextPrivacyLevel] > PRIVACY_LEVEL_STRICTNESS[drawnPrivacyLevel]) { sessionManager.expire() @@ -283,8 +352,7 @@ export function startRumSessionManager( return } - const { sessionSampleRate } = resolveSampleRates(configuration, remote) - if (sessionSampleRate === 0) { + if (resolveSampleRates(configuration, remote).sessionSampleRate === 0) { sessionManager.expire() } } @@ -454,9 +522,11 @@ function computeSessionState( configuration: RumConfiguration, rawTrackingType?: string, forcedSession?: boolean, - // FLASHCAT FORK - called when a draw actually happens (never for a restored session) and lands - // on something other than the init values, with the rates the draw used and the remote version - // they came from. + // FLASHCAT FORK - called whenever a draw actually happens and never for a restored session, with + // the rates the draw used and the remote version they came from. Reporting every draw, including + // one that landed on the init values, is what lets the caller tell "this page drew" from "this + // page adopted a session somebody else drew" ā€” see `trackDraw`, where only the first may write to + // the record slot. onDraw?: (drawn: DrawnConfiguration) => void ) { let trackingType: RumTrackingType @@ -549,12 +619,9 @@ const PRIVACY_LEVEL_STRICTNESS: { [level in DefaultPrivacyLevel]: number } = { * report the same shape and differ only in the rates: forcing pins them, an ordinary draw uses * what the console and the application settled on. * - * What decides whether a draw is worth recording is the draw itself, not which feature produced it: - * a draw that used exactly what init passed is already described by the events, so recording it - * would buy nothing and cost a storage write on every site that turned none of this on. Everything - * else is recorded ā€” including a `beforeSampling` override or a forced session on a site with - * remote configuration switched off, where the rates used and the rates init passed are precisely - * the values that differ. + * Whether the draw is worth keeping is `isWorthRecording`'s question, asked one layer up, because + * the answer there decides between writing the record and clearing it ā€” and only a caller that + * hears about every draw can clear one. */ function reportDraw( configuration: RumConfiguration, @@ -566,23 +633,32 @@ function reportDraw( if (!onDraw) { return } - const drawn: DrawnConfiguration = { + onDraw({ version: remote.version, sessionSampleRate, sessionReplaySampleRate, traceSampleRate: remote.traceSampleRate ?? initTraceRule(configuration), defaultPrivacyLevel: remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel, - } - if ( - drawn.version === undefined && - drawn.sessionSampleRate === configuration.sessionSampleRate && - drawn.sessionReplaySampleRate === configuration.sessionReplaySampleRate && - drawn.traceSampleRate === initTraceRule(configuration) && - drawn.defaultPrivacyLevel === configuration.defaultPrivacyLevel - ) { - return - } - onDraw(drawn) + }) +} + +/** + * FLASHCAT FORK - whether a draw says anything the init values do not. + * + * One that does not is already described by the events, so keeping it would buy nothing and cost a + * storage write on every site that turned none of this on. Asked about the draw rather than about + * which feature produced it: a `beforeSampling` override or a forced session on a site with remote + * configuration switched off is precisely the case where the rates used and the rates init passed + * are the values that differ. + */ +function isWorthRecording(configuration: RumConfiguration, drawn: DrawnConfiguration) { + return ( + drawn.version !== undefined || + drawn.sessionSampleRate !== configuration.sessionSampleRate || + drawn.sessionReplaySampleRate !== configuration.sessionReplaySampleRate || + drawn.traceSampleRate !== initTraceRule(configuration) || + drawn.defaultPrivacyLevel !== configuration.defaultPrivacyLevel + ) } /** From 175bf28f1669c7ae1e7e8af3d0bd4c4079d038da Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Sep 2026 00:21:37 -0700 Subject: [PATCH 2/2] fix(rum): decide on a sampled-out session from the draw record in storage A rate leaving 0 ends the running session only if that session was drawn at 0, and the rate it was drawn at was read off the in-memory copy taken when the session was adopted. That copy can outlive the session: the session store tells sessions apart by id and tracking type, and two sampled-out sessions have neither an id nor a different type, so a tab whose storage poll misses the expired state between them never sees another tab end the first and draw the second. It keeps the first session's rate and, on the next delivered settings, may end a session that already lost a draw at the current rate. Read the rate off storage at the moment of the decision instead. The page that draws writes its record in the same stack that creates the session, so storage always describes the current draw, and it is the only thing the two tabs share. The tracked branch keeps the in-memory copy: a collected session carries an id, so its replacement is seen. --- CHANGELOG.md | 5 ++- .../src/domain/rumSessionManager.spec.ts | 37 ++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 42 +++++++++++-------- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 696358c579..170b09cac8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,10 @@ - šŸ“ The rate a sampled-out session was drawn at is now recorded alongside the one a collected session was drawn at, in the same single `localStorage` entry this SDK already keeps for the draw. No new entry, no extra request. Without it a page that did not perform the draw ā€” the - second page of a visit, or another tab ā€” could not tell the two populations above apart. + second page of a visit, or another tab ā€” could not tell the two populations above apart. The + decision reads that record straight off storage rather than off what the page last saw of the + draw: two sampled-out sessions look alike to the session store, so a tab can miss another tab + ending one and drawing the next, and storage is the one place the current draw is always found. - šŸ“ What you will see on the day you lift a rate off 0: visitors who were invisible start appearing within seconds of loading a page rather than at their next session, so collected volume climbs the same day rather than the next. That is the change taking effect, not a defect. diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 186514a200..8cf789409e 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -1165,6 +1165,43 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeTrue() }) + it('reads the rate off storage rather than off the draw this page last saw', () => { + // Two sampled-out sessions look alike to the session store ā€” no id, the same tracking type ā€” + // so a tab that misses the expired state between them never learns the session was + // replaced: nothing expires and nothing renews here, and what this page last read of the + // draw stays as it was. Storage is the one thing the tab that drew the replacement shares + // with this one, so it is what has to be read when the decision is made. + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 50 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Another tab hears a rate of 30, ends the session drawn at zero and draws the next one, + // which loses ā€” all between two of this page's storage polls. + storeRemote({ version: 2, sessionSampleRate: 30 }) + setCookie(SESSION_STORE_KEY, `rum=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) + localStorage.setItem( + DRAW_KEY, + JSON.stringify({ + id: 'not-tracked', + version: 2, + sessionSampleRate: 30, + sessionReplaySampleRate: 50, + traceSampleRate: 100, + defaultPrivacyLevel: 'mask', + }) + ) + clock.tick(STORAGE_POLL_DELAY) + expect(expireSessionSpy).not.toHaveBeenCalled() + + // This page's own request answers with settings newer still. Read off the draw it last saw + // the session looks drawn at zero and is ended; read off storage it lost a draw at thirty + // and is left alone. + deliver({ version: 3, sessionSampleRate: 80 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + it('does not consult beforeSampling when no rate could decide anything', () => { // Resolving the rate runs the site's own code, and an announcement is not a draw. It is // asked only where the answer is what settles whether the session ends ā€” never once per diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 90f6196876..7617bfb0e4 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -119,12 +119,11 @@ export const enum SessionReplayState { * keeps a stale one from being read instead is that the page which draws owns the slot ā€” it writes * its draw or clears the slot, in the same stack that created the session ā€” so the record always * describes the most recent draw, and the most recent draw is what created the session being read. - * The gap left is the one this design already has for collected sessions and states two comments - * down: a tab polling storage between the session store's write and the record's would read the - * previous draw. A collected session falls back to init there; a sampled-out one reads the - * previous sampled-out draw's rate instead, which differs from its own only if the console moved - * the rate between two consecutive sessions of one visitor, and costs that visitor one extra - * re-draw when it does. + * The one thing the record decides for a sampled-out session ā€” whether a rate leaving 0 may end + * it ā€” is read off storage at the moment of that decision rather than off the copy `trackDraw` + * took when the session was adopted. See `endSessionIfSettingsAreDecisive` for why the copy is not + * enough: the session store cannot tell one sampled-out session from the next, so a page can keep + * the copy of a session another tab has already replaced. */ const NOT_TRACKED_DRAW_ID = 'not-tracked' @@ -201,8 +200,8 @@ export function startRumSessionManager( // synchronous stack: a tab whose storage poll fell exactly between the two would find no record // and keep its own settings for that session. Writing it earlier is not possible from here ā€” the // id it belongs to is generated inside the store, as that session is persisted. The record is - // read only here, when a session is adopted, so such a tab keeps its own settings for the whole - // remaining life of that session rather than until its next poll. + // read into the history only here, when a session is adopted, so such a tab keeps its own + // settings for the whole remaining life of that session rather than until its next poll. // // Storage is also per origin while the session need not be: with `trackSessionAcrossSubdomains` // a session arrives on the next subdomain with no record waiting, and is reported and traced @@ -311,17 +310,22 @@ export function startRumSessionManager( } const remote = readRemoteConfig(configuration.remoteConfig) - // What this session was created under, which is not the previously stored settings: settings - // are stored while a session runs, and the session was drawn under whatever was stored before - // that. No record means the draw used the init values ā€” `reportDraw` records every draw that - // did not, so a draw with nothing recorded is a draw that used them. - const drawn = drawnHistory.find() if (!isTypeTracked(session.trackingType)) { + // Read off storage rather than off `drawnHistory`, because the two can disagree here and only + // storage is right. Two sampled-out sessions look alike to the session store ā€” no id, the + // same tracking type ā€” so a page whose storage poll misses the expired state between them + // never learns that another tab ended the first and drew the second: nothing expires and + // nothing renews on this page, and the history keeps the draw of a session that is gone. The + // page that drew the replacement wrote its rate to storage in the same stack, so that is the + // one place this session's own rate can be found. A collected session cannot be confused this + // way, since its id changes with it. + // // Nothing forced can reach this comparison as a zero: a forced draw is recorded at 100 and is // collected besides, so the record already answers the question the tracked branch has to ask - // `forcedSession` about below. - const drawnSampleRate = drawn?.sessionSampleRate ?? configuration.sessionSampleRate + // `forcedSession` about below. No record means the draw used the init values, see `trackDraw`. + const drawnSampleRate = + readDrawRecord(configuration, NOT_TRACKED_DRAW_ID)?.sessionSampleRate ?? configuration.sessionSampleRate if (drawnSampleRate !== 0) { return } @@ -334,9 +338,11 @@ export function startRumSessionManager( return } - // What this session is masking pages with right now ā€” the recorder falls back to the init value - // the same way when there is no record, see `startRecording`. - const drawnPrivacyLevel = drawn?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + // What this session is masking pages with right now, which is not the previously stored + // settings: settings are stored while a session runs, and the session was drawn under whatever + // was stored before that. No record means the draw used the init value, and so does the + // recorder ā€” see `startRecording`, which falls back the same way. + const drawnPrivacyLevel = drawnHistory.find()?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel const nextPrivacyLevel = remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel if (PRIVACY_LEVEL_STRICTNESS[nextPrivacyLevel] > PRIVACY_LEVEL_STRICTNESS[drawnPrivacyLevel]) { sessionManager.expire()