From f84fd008c3a32e56365e050bd296edba6de7434f Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:00:23 -0700 Subject: [PATCH 1/7] refactor(rum): resolve the rates a draw would use in one place The rate a session is drawn on is the console's value falling back to init, with the application's beforeSampling given the last word. That resolution was written inline in the only branch that draws, which is fine as long as a draw is the only thing that needs to know the answer. Move it into a function that resolves and never draws, so the same question can be asked without spending a lottery ticket to find out. No behaviour changes. --- .../rum-core/src/domain/rumSessionManager.ts | 67 +++++++++++-------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 423c3c9c27..e25bf8da37 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -387,34 +387,7 @@ function computeSessionState( // the decision it was created with: settings arriving mid-session never start or stop // collecting for a visitor already on the site. const remote = readRemoteConfig(configuration.remoteConfig) - - let sessionSampleRate = remote.sessionSampleRate ?? configuration.sessionSampleRate - let sessionReplaySampleRate = remote.sessionReplaySampleRate ?? configuration.sessionReplaySampleRate - - // FLASHCAT FORK - the application gets the last word, right at the draw. This is what turns the - // delivered custom values into sampling decisions without a wasted first draw or a session - // restart: the console ships the data (an allow-list, a cohort rule), the application's own - // code interprets it here. Its failure modes must never reach session creation, so a thrown - // error or a value outside 0..100 leaves the incoming rate in place. - if (configuration.beforeSampling) { - try { - const override = configuration.beforeSampling({ - sessionSampleRate, - sessionReplaySampleRate, - custom: remote.custom, - }) - if (override) { - if (isRate(override.sessionSampleRate)) { - sessionSampleRate = override.sessionSampleRate - } - if (isRate(override.sessionReplaySampleRate)) { - sessionReplaySampleRate = override.sessionReplaySampleRate - } - } - } catch (e) { - display.error('beforeSampling threw an error:', e) - } - } + const { sessionSampleRate, sessionReplaySampleRate } = resolveSampleRates(configuration, remote) reportDraw(configuration, remote, sessionSampleRate, sessionReplaySampleRate, onDraw) @@ -432,6 +405,44 @@ function computeSessionState( } } +/** + * FLASHCAT FORK - the rates a draw would use right now: what the console delivered, falling back to + * what the site passed to init, with the application's `beforeSampling` given the last word. This + * is what turns the delivered custom values into sampling decisions without a wasted first draw or + * a session restart: the console ships the data (an allow-list, a cohort rule), the application's + * own code interprets it here. Its failure modes must never reach session creation, so a thrown + * error or a value outside 0..100 leaves the incoming rate in place. + * + * Resolving is all it does — it never draws on the rates it returns — so the same question can be + * asked away from a draw. + */ +function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfigValues) { + let sessionSampleRate = remote.sessionSampleRate ?? configuration.sessionSampleRate + let sessionReplaySampleRate = remote.sessionReplaySampleRate ?? configuration.sessionReplaySampleRate + + if (configuration.beforeSampling) { + try { + const override = configuration.beforeSampling({ + sessionSampleRate, + sessionReplaySampleRate, + custom: remote.custom, + }) + if (override) { + if (isRate(override.sessionSampleRate)) { + sessionSampleRate = override.sessionSampleRate + } + if (isRate(override.sessionReplaySampleRate)) { + sessionReplaySampleRate = override.sessionReplaySampleRate + } + } + } catch (e) { + display.error('beforeSampling threw an error:', e) + } + } + + return { sessionSampleRate, sessionReplaySampleRate } +} + /** * FLASHCAT FORK - hands the draw that just happened to whoever records it. Both draw branches * report the same shape and differ only in the rates: forcing pins them, an ordinary draw uses From 2acc5d1f859741bba5b3a887cc7525fea7b7bf65 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:00:41 -0700 Subject: [PATCH 2/7] feat(rum): end the session when new settings decide its fate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings published from the console applied to sessions created after they arrived, and to nothing else. For a visitor who never goes idle that is hours: a session ends after fifteen minutes without activity or four hours outright, so the change everyone is waiting on reaches the people generating the most data last. Three changes cannot wait, and they are exactly the three whose effect on the running session can be told without drawing again: - a session sample rate of 0 while the visitor is being collected; - a rate of 100 while they are not; - a stricter defaultPrivacyLevel, where every further second recorded is a second of plaintext uploaded that masking cannot reach back for. Each of them ends the current session; the visitor's next action starts a new one under the new settings. Ending rather than flipping is the point: the old session is collected to its end as it was begun, so no replay is masked in one half and plain in the other, and no session is invented that starts in the middle of a visit. No other rate says anything about whether THIS session should have been kept. Only a second draw could, and drawing twice quietly turns a rate p into p², so every other change waits for the next session — a loosening privacy level included, where being slow is what leaves room to undo a mistake. It needs no bookkeeping to stay idempotent: what it compares is what the session was drawn under against what a draw would use now, and ending the session is exactly what makes that difference disappear. The same response arriving again, in another tab or after a reload, finds nothing left to act on. beforeSampling is now called outside a draw as well, to resolve the rate that would actually apply, so the documentation asks for a callback free of side effects and stable for the same input. --- .../src/domain/configuration/configuration.ts | 20 +- .../configuration/remoteConfiguration.spec.ts | 61 ++++ .../configuration/remoteConfiguration.ts | 44 ++- packages/rum-core/src/domain/lifeCycle.ts | 8 + .../src/domain/rumSessionManager.spec.ts | 297 ++++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 92 +++++- 6 files changed, 505 insertions(+), 17 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index ee15cedb80..5d5a4c1025 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -62,6 +62,12 @@ export interface RumInitConfiguration extends InitConfiguration { * a single session. Keep it a pure decision: side effects will be repeated, and only the last * call's return value is used. * + * The SDK also calls it away from a draw: when new settings arrive it asks which rate would + * apply now, to decide whether the running session has to end for them to take effect. So it + * must answer the same way for the same input — one that answers differently each time can keep + * ending the session it was just asked about — and anything it does besides returning a rate (a + * metric, a log, a counter) happens more often than there are sessions. + * * Its failure modes never reach session creation: a thrown error or an out-of-range value leaves * the incoming rate in place, and a value that is not a function at all is reported once and * then ignored rather than refusing `init`. @@ -86,9 +92,17 @@ export interface RumInitConfiguration extends InitConfiguration { * Take the sampling rates from the application's settings in the console instead of only from the * 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; a session already under way keeps the - * decision it was created with. The values below stay in use until the first settings arrive, and - * whenever the settings cannot be reached. + * A change applies to sessions started after it arrives, and a session already under way is never + * 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 session sample rate of 0 while the + * visitor is being collected, a rate of 100 while they are not, and a stricter + * `defaultPrivacyLevel`. Each of those 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, + * a loosening privacy level included, waits for the next session. + * + * The values below stay in use until the first settings arrive, and whenever the settings cannot + * be reached. * * Requires `localStorage`. Sessions themselves are kept in a cookie unless `sessionPersistence` * says otherwise, but this SDK already reads one `localStorage` entry on every site — the record diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index 214a097342..e81b8c4c2a 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -196,6 +196,67 @@ describe('remoteConfiguration', () => { }) }) + describe('announcing that new settings are in storage', () => { + function watchStoredNotifications() { + const notified = jasmine.createSpy('remoteConfigurationStored') + lifeCycle.subscribe(LifeCycleEventType.REMOTE_CONFIGURATION_STORED, notified) + return notified + } + + it('announces settings that reached storage, so a subscriber can act on them', (done) => { + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 0 } })) + + expect(notified).toHaveBeenCalledTimes(1) + done() + }) + start(configurationWith()) + }) + + it('stays silent about settings it refused as older than the ones it holds', (done) => { + localStorage.setItem(setup!.storeKey, JSON.stringify({ sessionSampleRate: 42, version: 8 })) + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ version: 7, rum: { sessionSampleRate: 0 } })) + + // Nothing changed in storage, so nothing downstream may behave as though it had. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + + it('stays silent when the answer never reached storage', (done) => { + const notified = watchStoredNotifications() + spyOn(Storage.prototype, 'setItem').and.throwError('storage is full') + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 0 } })) + + // The next draw will not find these settings, so ending a session for their sake would end + // it for nothing. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + + it('stays silent about an answer that never made it', (done) => { + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(500) + + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + }) + describe('refusing a payload it cannot read', () => { const STORED = { sessionSampleRate: 42, version: 2 } diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 2180dfbc8f..e883e86486 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -20,17 +20,23 @@ declare const __BUILD_ENV__SDK_VERSION__: string * masks a page by default. * * A change only affects sessions created after it arrives, so a visitor is never dropped halfway - * through and never starts being recorded halfway through. Fetching follows the same rhythm: once - * at start-up and once whenever a new session begins — a change can only matter at the next draw, - * so asking more often than sessions are drawn would be requests for nothing. There is no timer - * between sessions. + * through and never starts being recorded halfway through. What "immediately" means for the + * handful of changes that cannot wait is therefore not a flip of the running session but its end: + * see `endSessionIfSettingsAreDecisive` in the session manager, which subscribes to the event this + * module emits once new settings are in storage. + * + * Fetching follows the session's rhythm: once at start-up and once whenever a new session begins — + * a change can only matter at a draw, and every draw is a new session — so asking more often than + * sessions are drawn would be requests for nothing. There is no timer between sessions. The cost + * of that rhythm is that a visitor who never goes idle stays on one session, and so on one set of + * settings, for as long as they keep using the site. * * Three fields the server sends are accepted and ignored, deliberately: `ttl` and * `refresh_on_foreground`, which describe when to ask again and are moot without a timer, and - * `activation`, which offers to end a running session so a change applies at once. Everything here - * is next-session, so a console that ever offers "apply immediately" would not be obeyed by this - * build — named here so the mismatch is found by reading rather than by an operator wondering why - * nothing happened. + * `activation`, which offers to end a running session so a change applies at once. This build ends + * a running session on its own reading of what changed rather than on the server's say-so, so a + * console that offers "apply immediately" as a switch would not be obeyed — named here so the + * mismatch is found by reading rather than by an operator wondering why nothing happened. * * Nothing here runs unless `remoteConfigurationEnabled: true`. Left off — the default — the SDK makes no * extra request and behaves exactly as it did before this existed. @@ -112,6 +118,9 @@ export interface BeforeSamplingContext { * The application's last word on the sampling of the session about to be drawn — see the * `beforeSampling` init option. Returning nothing, or an out-of-range rate, leaves the incoming * value in place. + * + * Must be free of side effects and answer the same way for the same input: it is also called away + * from a draw, to work out which rate newly delivered settings would actually apply. */ export type BeforeSamplingCallback = ( context: BeforeSamplingContext @@ -277,7 +286,12 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet } if (response) { failedAttempts = 0 - store(setup, response) + if (store(setup, response)) { + // Announced only once the settings are in storage, because that is where the next draw + // reads them: a subscriber that ends the running session so the new values can take + // effect immediately has to be sure the draw that follows will find them. + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + } return } if (failedAttempts < RETRY_DELAYS.length) { @@ -387,6 +401,11 @@ function fetchRemoteConfiguration( } } +/** + * Writes the response to storage, and answers whether it actually landed there. A refused or + * unwritable response answers `false`: nothing changed for the next draw, so nothing downstream + * should act as if it had. + */ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) { // Settings are published under a number that only ever goes up — rolling back republishes the // old settings under a new, higher one — so a response numbered below what is already stored is @@ -403,7 +422,7 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) // requests that can cross are two pages, and storage is the only thing they share. const storedVersion = readRemoteConfig(setup).version if (storedVersion !== undefined && response.version < storedVersion) { - return + return false } const values: RemoteConfigValues = { version: response.version } @@ -437,10 +456,13 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) // settings" looks like — so that the version is kept either way and the console can still see // that this client is up to date with the change that turned it off. localStorage.setItem(setup.storeKey, JSON.stringify(values)) + return true } catch { // Storage unavailable, or the origin is out of room. The previous entry stays as it is, which // is the same "keep what is already working" answer a failed request gets — the client goes on - // applying the settings it last stored, and goes on reporting their version. + // applying the settings it last stored, and goes on reporting their version. Reported as a + // failure all the same: nothing downstream may act on settings the next draw will not find. + return false } } diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index b1abd3fb46..c7453faadc 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -32,6 +32,12 @@ export const enum LifeCycleEventType { // on the same domain. SESSION_EXPIRED, SESSION_RENEWED, + + // FLASHCAT FORK - a remote configuration response has just been written to storage. Emitted only + // when the write actually happened, so a response refused as stale and a storage failure both + // stay silent: a subscriber acting on settings that are not in storage would act on values the + // next draw is not going to read. + REMOTE_CONFIGURATION_STORED, PAGE_MAY_EXIT, PAGE_REACTIVATED, RAW_RUM_EVENT_COLLECTED, @@ -64,6 +70,7 @@ declare const LifeCycleEventTypeAsConst: { REQUEST_COMPLETED: LifeCycleEventType.REQUEST_COMPLETED SESSION_EXPIRED: LifeCycleEventType.SESSION_EXPIRED SESSION_RENEWED: LifeCycleEventType.SESSION_RENEWED + REMOTE_CONFIGURATION_STORED: LifeCycleEventType.REMOTE_CONFIGURATION_STORED PAGE_MAY_EXIT: LifeCycleEventType.PAGE_MAY_EXIT PAGE_REACTIVATED: LifeCycleEventType.PAGE_REACTIVATED RAW_RUM_EVENT_COLLECTED: LifeCycleEventType.RAW_RUM_EVENT_COLLECTED @@ -85,6 +92,7 @@ export interface LifeCycleEventMap { [LifeCycleEventTypeAsConst.REQUEST_COMPLETED]: RequestCompleteEvent [LifeCycleEventTypeAsConst.SESSION_EXPIRED]: void [LifeCycleEventTypeAsConst.SESSION_RENEWED]: void + [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void [LifeCycleEventTypeAsConst.PAGE_MAY_EXIT]: PageMayExitEvent [LifeCycleEventTypeAsConst.PAGE_REACTIVATED]: void [LifeCycleEventTypeAsConst.RAW_RUM_EVENT_COLLECTED]: RawRumEventCollectedData diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index f122708907..87bbf29d60 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -820,6 +820,303 @@ describe('rum session manager', () => { }) }) + describe('restarting the session when the settings are decisive', () => { + const STORE_KEY = 'test-decisive-settings' + const DRAW_KEY = 'test-decisive-settings-draw' + const REMOTE_SETUP = { + buildUrl: () => 'https://example.com/config', + storeKey: STORE_KEY, + fetchTimeout: 3000, + } + + afterEach(() => localStorage.removeItem(DRAW_KEY)) + + function storeRemote(stored: object) { + localStorage.setItem(STORE_KEY, JSON.stringify(stored)) + registerCleanupTask(() => localStorage.removeItem(STORE_KEY)) + } + + function startWith(configuration: Partial = {}) { + return startRumSessionManagerWithDefaults({ + configuration: { remoteConfig: REMOTE_SETUP, drawStoreKey: DRAW_KEY, ...configuration }, + }) + } + + // Settings reach storage first and are announced afterwards, the order the fetcher uses: the + // draw that may follow reads storage, so it has to find them already there. + function deliver(stored: object) { + storeRemote(stored) + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + } + + function isSessionEnded() { + return getSessionState(SESSION_STORE_KEY).isExpired === '1' + } + + describe('the three changes it can decide on its own', () => { + it('ends a session being collected when the rate goes to zero', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + + deliver({ version: 2, sessionSampleRate: 0, sessionReplaySampleRate: 0 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a session that is not being collected when the rate goes to a hundred', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends the session when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends the session on the tightening step that masks everything', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('draws the session that follows on the settings that have just landed', () => { + 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) + }) + }) + + describe('everything else waits for the next session', () => { + it('leaves the session alone when the rate moves to a value it cannot decide on', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 30 }) + + 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 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when the privacy level loosens', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + startWith({ sessionSampleRate: 100 }) + + // Being slow here is the point: it leaves an operator time to undo a mistake, and what it + // costs meanwhile is more of the data already being collected. + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the custom bag changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, custom: { cohort: 'a' } }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 100, custom: { cohort: 'b' } }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the trace rate changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, traceSampleRate: 10 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 100, traceSampleRate: 90 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('leaves the session alone when only the replay rate changed', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + // The replay rate is deliberately not one of the three: it decides a draw nested inside the + // session draw, and a rule for it would have to say what happens to a replay the host + // application forced on. Until that is settled, a replay rate change waits for the next + // session like every other change. + deliver({ version: 2, sessionSampleRate: 100, sessionReplaySampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('ignores what is in storage when the site did not opt in', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 0, drawStoreKey: DRAW_KEY } }) + + deliver({ version: 2, sessionSampleRate: 100 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('has nothing to end when the session is already over', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startWith({ sessionSampleRate: 0 }) + expireCookie() + clock.tick(STORAGE_POLL_DELAY) + expireSessionSpy.calls.reset() + + // There is no session to read a decision off, and nothing to end: the next activity draws + // on what has just been stored, which is all this change needs. + expect(() => deliver({ version: 2, sessionSampleRate: 100 })).not.toThrow() + expect(expireSessionSpy).not.toHaveBeenCalled() + }) + }) + + describe('what it compares', () => { + it('never draws again to reach its decision', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + const draw = spyOn(Math, 'random').and.callThrough() + + deliver({ version: 2, sessionSampleRate: 30 }) + + // Drawing here would be a second lottery on top of the one the next session runs, quietly + // turning a rate p into p². + expect(draw).not.toHaveBeenCalled() + }) + + it('compares against the level the session was drawn under, not the settings stored since', () => { + storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + // A loosening leaves the running session masking everything, as it was drawn to. + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + expect(expireSessionSpy).not.toHaveBeenCalled() + + // Stricter than what was stored a moment ago, still looser than what this session actually + // masks with. Judged against the stored settings it would end a session with nothing to + // gain from restarting. + deliver({ version: 3, sessionSampleRate: 100, defaultPrivacyLevel: 'mask-user-input' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('lets beforeSampling have the last word on the rate it judges', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ + sessionSampleRate: 100, + beforeSampling: ({ sessionSampleRate }) => ({ sessionSampleRate: sessionSampleRate === 0 ? 50 : 100 }), + }) + + // The console says zero, the application puts it back in the middle: the rate that would + // actually apply is fifty, which decides nothing. + deliver({ version: 2, sessionSampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + }) + + describe('arriving more than once', () => { + it('does not end the session a second time when the same settings arrive again', () => { + storeRemote({ version: 1, sessionSampleRate: 100, sessionReplaySampleRate: 100 }) + startWith({ sessionSampleRate: 100 }) + + deliver({ version: 2, sessionSampleRate: 0 }) + 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() + + // Another tab, a retry, a reload: the same answer arrives again and finds the difference + // that justified ending a session already gone. + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + 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' }) + + deliver({ version: 2, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + expect(isSessionEnded()).toBeTrue() + + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + expireSessionSpy.calls.reset() + + lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + }) + + describe('a session the host application forced', () => { + function startForced(configuration: Partial = {}) { + const rumSessionManager = startWith({ sessionSampleRate: 0, ...configuration }) + rumSessionManager.setForcedSession() + 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) + expireSessionSpy.calls.reset() + return rumSessionManager + } + + it('is not ended by a rate, since every draw it makes is collected anyway', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + startForced() + + // Ending it would only replace it with another forced session — the same difference, for + // as long as the page lives. + deliver({ version: 2, sessionSampleRate: 0 }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + it('is still ended when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startForced({ defaultPrivacyLevel: 'allow' }) + + // Forcing decides whether this visitor is collected. It says nothing about how much of + // their page may be uploaded in the clear. + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + }) + }) + function startRumSessionManagerWithDefaults({ configuration, trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED), diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index e25bf8da37..ff71bdc22c 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -1,6 +1,7 @@ -import type { DefaultPrivacyLevel, RelativeTime, TrackingConsentState } from '@flashcatcloud/browser-core' +import type { RelativeTime, TrackingConsentState } from '@flashcatcloud/browser-core' import { BridgeCapability, + DefaultPrivacyLevel, Observable, SESSION_TIME_OUT_DELAY, STORAGE_POLL_DELAY, @@ -206,6 +207,78 @@ export function startRumSessionManager( lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) }) + // FLASHCAT FORK - a change published mid-session normally waits for that session to end on its + // own, which for a visitor who never goes idle is hours away. Three changes cannot afford the + // wait, and what makes exactly those three special is that their outcome for the running session + // can be asserted without drawing again: + // + // - a session sample rate of 0 while this session is being collected: nothing is meant to be + // collected any more, and this is the emergency stop the console offers; + // - a session sample rate of 100 while this session is not: everything is meant to be + // collected, and this visitor is the exception; + // - a stricter default privacy level: every further second recorded is a second of plaintext + // uploaded, and masking cannot reach back for it. + // + // No other rate 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². 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. + // + // The action is always 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 invent a session that begins in the middle of a visit. + // + // It stays idempotent with no bookkeeping at all: it compares what this session was drawn under + // against what a draw would use now, and ending the session is exactly what makes that + // difference disappear. The same response arriving again — another tab, a retry, a reload — + // finds nothing left to act on. + function endSessionIfSettingsAreDecisive() { + if (!configuration.remoteConfig) { + return + } + const session = sessionManager.findSession() + if (!session) { + // Nothing to end. Whatever starts the next session draws on the settings just stored, which + // is the ordinary path and already gives them their effect. + return + } + + const remote = readRemoteConfig(configuration.remoteConfig) + + // 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 + // session. + const drawnPrivacyLevel = drawnHistory.find()?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + const nextPrivacyLevel = remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel + if (PRIVACY_LEVEL_STRICTNESS[nextPrivacyLevel] > PRIVACY_LEVEL_STRICTNESS[drawnPrivacyLevel]) { + sessionManager.expire() + return + } + + if (forcedSession) { + // The host application has taken this page off the rates deliberately, and every draw it + // makes from now on is collected whatever the console says. Ending the session on a rate + // would only replace it with another forced one — the same difference, forever. + return + } + + // Whether this session is collected is read off the session itself rather than reconstructed + // by comparing rates: the session IS the outcome its draw produced, and an outcome is the only + // thing 0 and 100 let us assert anything about. + const isCollected = isTypeTracked(session.trackingType) + const { sessionSampleRate } = resolveSampleRates(configuration, remote) + if ((sessionSampleRate === 0 && isCollected) || (sessionSampleRate === 100 && !isCollected)) { + sessionManager.expire() + } + } + + const remoteConfigSubscription = lifeCycle.subscribe( + LifeCycleEventType.REMOTE_CONFIGURATION_STORED, + endSessionIfSettingsAreDecisive + ) + sessionManager.sessionStateUpdateObservable.subscribe(({ previousState, newState }) => { if (!previousState.forcedReplay && newState.forcedReplay) { const sessionEntity = sessionManager.findSession() @@ -238,6 +311,7 @@ export function startRumSessionManager( expireObservable: sessionManager.expireObservable, stop: () => { consentSubscription.unsubscribe() + remoteConfigSubscription.unsubscribe() drawnHistory.stop() }, setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), @@ -413,8 +487,9 @@ function computeSessionState( * own code interprets it here. Its failure modes must never reach session creation, so a thrown * error or a value outside 0..100 leaves the incoming rate in place. * - * Resolving is all it does — it never draws on the rates it returns — so the same question can be - * asked away from a draw. + * It resolves rates and never draws on them, which is what lets the same question be asked away + * from a draw — see `endSessionIfSettingsAreDecisive`, which needs to know which rate would apply + * without spending a lottery ticket to find out. */ function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfigValues) { let sessionSampleRate = remote.sessionSampleRate ?? configuration.sessionSampleRate @@ -443,6 +518,17 @@ function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfi return { sessionSampleRate, sessionReplaySampleRate } } +/** + * FLASHCAT FORK - how much of a page each level keeps out of a recording, ordered so two levels can + * be compared. Only the direction matters: tightening is the change that cannot be undone after the + * fact, because a second already recorded in the clear has already been uploaded in the clear. + */ +const PRIVACY_LEVEL_STRICTNESS: { [level in DefaultPrivacyLevel]: number } = { + [DefaultPrivacyLevel.ALLOW]: 0, + [DefaultPrivacyLevel.MASK_USER_INPUT]: 1, + [DefaultPrivacyLevel.MASK]: 2, +} + /** * FLASHCAT FORK - hands the draw that just happened to whoever records it. Both draw branches * report the same shape and differ only in the rates: forcing pins them, an ordinary draw uses From f6d9b5c893fa5575e9b1bdd8bbcf5fce7f3b3090 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:04:01 -0700 Subject: [PATCH 3/7] refactor(rum): keep the fork's lifecycle event out of upstream's numbering A const enum's values are inlined at build time and every entry after an insertion shifts, so an entry wedged into the middle of a list that is otherwise upstream's is both a renumbering and a conflict on the next upstream merge. Move it to the end. Also drop a guard that restated its caller's precondition: the event is only ever emitted by the fetcher, which does not exist unless the site opted in, and reading the settings already answers with nothing when it did not. --- packages/rum-core/src/domain/lifeCycle.ts | 19 ++++++++++++------- .../src/domain/rumSessionManager.spec.ts | 5 ++++- .../rum-core/src/domain/rumSessionManager.ts | 3 --- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index c7453faadc..b185daa394 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -32,17 +32,22 @@ export const enum LifeCycleEventType { // on the same domain. SESSION_EXPIRED, SESSION_RENEWED, + PAGE_MAY_EXIT, + PAGE_REACTIVATED, + RAW_RUM_EVENT_COLLECTED, + RUM_EVENT_COLLECTED, + RAW_ERROR_COLLECTED, // FLASHCAT FORK - a remote configuration response has just been written to storage. Emitted only // when the write actually happened, so a response refused as stale and a storage failure both // stay silent: a subscriber acting on settings that are not in storage would act on values the // next draw is not going to read. + // + // Added last on purpose. The values of a const enum are inlined at build time and shift when an + // entry is inserted, and everything above this line is upstream's — keeping the fork's own entry + // at the end leaves upstream's numbering alone and keeps this file out of the way of the next + // upstream merge. REMOTE_CONFIGURATION_STORED, - PAGE_MAY_EXIT, - PAGE_REACTIVATED, - RAW_RUM_EVENT_COLLECTED, - RUM_EVENT_COLLECTED, - RAW_ERROR_COLLECTED, } // This is a workaround for an issue occurring when the Browser SDK is included in a TypeScript @@ -70,12 +75,12 @@ declare const LifeCycleEventTypeAsConst: { REQUEST_COMPLETED: LifeCycleEventType.REQUEST_COMPLETED SESSION_EXPIRED: LifeCycleEventType.SESSION_EXPIRED SESSION_RENEWED: LifeCycleEventType.SESSION_RENEWED - REMOTE_CONFIGURATION_STORED: LifeCycleEventType.REMOTE_CONFIGURATION_STORED PAGE_MAY_EXIT: LifeCycleEventType.PAGE_MAY_EXIT PAGE_REACTIVATED: LifeCycleEventType.PAGE_REACTIVATED RAW_RUM_EVENT_COLLECTED: LifeCycleEventType.RAW_RUM_EVENT_COLLECTED RUM_EVENT_COLLECTED: LifeCycleEventType.RUM_EVENT_COLLECTED RAW_ERROR_COLLECTED: LifeCycleEventType.RAW_ERROR_COLLECTED + REMOTE_CONFIGURATION_STORED: LifeCycleEventType.REMOTE_CONFIGURATION_STORED } // Note: this interface needs to be exported even if it is not used outside of this module, else TS @@ -92,7 +97,6 @@ export interface LifeCycleEventMap { [LifeCycleEventTypeAsConst.REQUEST_COMPLETED]: RequestCompleteEvent [LifeCycleEventTypeAsConst.SESSION_EXPIRED]: void [LifeCycleEventTypeAsConst.SESSION_RENEWED]: void - [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void [LifeCycleEventTypeAsConst.PAGE_MAY_EXIT]: PageMayExitEvent [LifeCycleEventTypeAsConst.PAGE_REACTIVATED]: void [LifeCycleEventTypeAsConst.RAW_RUM_EVENT_COLLECTED]: RawRumEventCollectedData @@ -101,6 +105,7 @@ export interface LifeCycleEventMap { error: RawError customerContext?: Context } + [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void } export interface RawRumEventCollectedData { diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 87bbf29d60..1417bfc34c 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -971,10 +971,13 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeFalse() }) - it('ignores what is in storage when the site did not opt in', () => { + it('reads nothing out of the settings store when the site did not opt in', () => { storeRemote({ version: 1, sessionSampleRate: 100 }) startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 0, drawStoreKey: DRAW_KEY } }) + // Such a site never fetches, so this can only ever be reached by hand. What matters is that + // the settings store is out of reach without the opt-in: the rate that would apply is the + // one init passed, which is the one this session was already drawn on. deliver({ version: 2, sessionSampleRate: 100 }) expect(expireSessionSpy).not.toHaveBeenCalled() diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index ff71bdc22c..a25813f526 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -234,9 +234,6 @@ export function startRumSessionManager( // difference disappear. The same response arriving again — another tab, a retry, a reload — // finds nothing left to act on. function endSessionIfSettingsAreDecisive() { - if (!configuration.remoteConfig) { - return - } const session = sessionManager.findSession() if (!session) { // Nothing to end. Whatever starts the next session draws on the settings just stored, which From c370e928a61694599a25fbab48850cc914dacff9 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:03:03 -0700 Subject: [PATCH 4/7] fix(rum): stop ending the sessions of visitors who are not collected A session that is not being collected is given no id, so no record of its draw is kept and the privacy level it was drawn under cannot be read back. The comparison fell through to the init value on every announcement and kept answering "tighter", so once an operator tightened `defaultPrivacyLevel` from the console, every sampled-out visitor was put on a loop: end the session, renew on the next click, refetch, end it again. It bought no privacy either -- a visitor who is not collected records nothing, so a stricter level has no plaintext to catch there. The rule now carries its own precondition and applies only while the session is being collected, which is also the only state in which a recording exists. Its fuel was the announcement firing on settings that had not changed: `store()` answered "stored" for a response repeating the version already held, which is the ordinary answer, since every new session refetches and most find nothing new. It now answers whether the stored version actually advanced. Three tests, each checked against the unfixed source first: a sampled-out session is left alone when the level tightens, it is still left alone as further settings arrive, and a response repeating the stored version is not announced. --- .../src/domain/configuration/configuration.ts | 16 ++++--- .../configuration/remoteConfiguration.spec.ts | 16 +++++++ .../configuration/remoteConfiguration.ts | 22 +++++++--- packages/rum-core/src/domain/lifeCycle.ts | 9 ++-- .../src/domain/rumSessionManager.spec.ts | 29 +++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 43 ++++++++++++------- 6 files changed, 102 insertions(+), 33 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 5d5a4c1025..62cc56db14 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -64,9 +64,9 @@ export interface RumInitConfiguration extends InitConfiguration { * * The SDK also calls it away from a draw: when new settings arrive it asks which rate would * apply now, to decide whether the running session has to end for them to take effect. So it - * must answer the same way for the same input — one that answers differently each time can keep - * ending the session it was just asked about — and anything it does besides returning a rate (a - * metric, a log, a counter) happens more often than there are sessions. + * must answer the same way for the same input — one that answers differently each time can end a + * session that a steady one would have left running — and anything it does besides returning a + * rate (a metric, a log, a counter) happens more often than there are sessions. * * Its failure modes never reach session creation: a thrown error or an out-of-range value leaves * the incoming rate in place, and a value that is not a function at all is reported once and @@ -96,10 +96,12 @@ export interface RumInitConfiguration extends InitConfiguration { * 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 session sample rate of 0 while the * visitor is being collected, a rate of 100 while they are not, and a stricter - * `defaultPrivacyLevel`. Each of those 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, - * a loosening privacy level included, waits for the next session. + * `defaultPrivacyLevel` while they are being collected — a visitor who is not being collected + * records nothing, so a stricter level has no plaintext to catch there. Each of those 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, a loosening privacy level included, waits for + * the next session. * * The values below stay in use until the first settings arrive, and whenever the settings cannot * be reached. diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index e81b8c4c2a..7ba6456c20 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -229,6 +229,22 @@ describe('remoteConfiguration', () => { start(configurationWith()) }) + it('stays silent about an answer that repeats the settings it already holds', (done) => { + localStorage.setItem(setup!.storeKey, JSON.stringify({ sessionSampleRate: 42, version: 7 })) + const notified = watchStoredNotifications() + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ version: 7, rum: { sessionSampleRate: 42 } })) + + // The ordinary answer: every new session asks again and most find nothing changed. A + // subscriber woken by those would act on no news, once per session, for as long as the + // visitor stays. + expect(notified).not.toHaveBeenCalled() + done() + }) + start(configurationWith()) + }) + it('stays silent when the answer never reached storage', (done) => { const notified = watchStoredNotifications() spyOn(Storage.prototype, 'setItem').and.throwError('storage is full') diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index e883e86486..190e6df96f 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -287,9 +287,10 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet if (response) { failedAttempts = 0 if (store(setup, response)) { - // Announced only once the settings are in storage, because that is where the next draw + // Announced only once new settings are in storage, because that is where the next draw // reads them: a subscriber that ends the running session so the new values can take - // effect immediately has to be sure the draw that follows will find them. + // effect immediately has to be sure the draw that follows will find them, and must not + // be woken by an answer that changed nothing. lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED) } return @@ -402,9 +403,10 @@ function fetchRemoteConfiguration( } /** - * Writes the response to storage, and answers whether it actually landed there. A refused or - * unwritable response answers `false`: nothing changed for the next draw, so nothing downstream - * should act as if it had. + * Writes the response to storage, and answers whether it brought settings this client did not + * already hold. A refused or unwritable response answers `false`, and so does one that repeats the + * version already stored: settings only ever change under a higher number, so by that contract a + * repeat leaves the next draw reading what it would have read anyway. */ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) { // Settings are published under a number that only ever goes up — rolling back republishes the @@ -425,6 +427,14 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) return false } + // Settings only ever change under a higher number, so a response repeating the number already + // stored carries nothing new — and that is the ordinary answer, since every new session refetches + // and most of them find the settings unchanged. It is written anyway, which costs one small + // `setItem` and keeps the entry in the shape this build writes, but it is not announced: a + // subscriber that ends the running session must hear about changes only, or an unchanged answer + // arriving at every renewal would end a session per renewal, forever. + const isNew = storedVersion === undefined || response.version > storedVersion + const values: RemoteConfigValues = { version: response.version } if (response.enabled && response.rum) { // Each value is copied only when the server actually sent it. A knob nobody configured must @@ -456,7 +466,7 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) // settings" looks like — so that the version is kept either way and the console can still see // that this client is up to date with the change that turned it off. localStorage.setItem(setup.storeKey, JSON.stringify(values)) - return true + return isNew } catch { // Storage unavailable, or the origin is out of room. The previous entry stays as it is, which // is the same "keep what is already working" answer a failed request gets — the client goes on diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index b185daa394..78b6d9fccb 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -38,10 +38,11 @@ export const enum LifeCycleEventType { RUM_EVENT_COLLECTED, RAW_ERROR_COLLECTED, - // FLASHCAT FORK - a remote configuration response has just been written to storage. Emitted only - // when the write actually happened, so a response refused as stale and a storage failure both - // stay silent: a subscriber acting on settings that are not in storage would act on values the - // next draw is not going to read. + // FLASHCAT FORK - a remote configuration response has just changed what is in storage. Emitted + // only when the write actually happened and actually changed something, so a response refused as + // stale, one that merely repeats the settings already held, and a storage failure all stay + // silent: a subscriber acting on settings the next draw would have read anyway would be acting + // on no news at all. // // Added last on purpose. The values of a const enum are inlined at build time and shift when an // entry is inserted, and everything above this line is upstream's — keeping the fork's own entry diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 1417bfc34c..261906ed64 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -925,6 +925,35 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeFalse() }) + it('leaves a session that is not being collected alone when the privacy level tightens', () => { + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + + // Nothing is being recorded for this visitor, so there is no plaintext for the stricter + // level to catch and nothing to gain by ending their session. + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + + 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. + storeRemote({ version: 1, sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + startWith({ sessionSampleRate: 0, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 2, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + clock.tick(STORAGE_POLL_DELAY) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + deliver({ version: 3, sessionSampleRate: 0, defaultPrivacyLevel: 'mask' }) + + expect(expireSessionSpy).not.toHaveBeenCalled() + expect(isSessionEnded()).toBeFalse() + }) + it('leaves the session alone when the privacy level loosens', () => { storeRemote({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) startWith({ sessionSampleRate: 100 }) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index a25813f526..af55201dbe 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -216,8 +216,8 @@ export function startRumSessionManager( // collected any more, and this is the emergency stop the console offers; // - a session sample rate of 100 while this session is not: everything is meant to be // collected, and this visitor is the exception; - // - a stricter default privacy level: every further second recorded is a second of plaintext - // uploaded, and masking cannot reach back for it. + // - a stricter default privacy level while this session is being collected: every further + // second recorded is a second of plaintext uploaded, and masking cannot reach back for it. // // No other rate 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². So everything else waits for @@ -243,28 +243,39 @@ export function startRumSessionManager( const remote = readRemoteConfig(configuration.remoteConfig) - // 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 - // session. - const drawnPrivacyLevel = drawnHistory.find()?.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel - const nextPrivacyLevel = remote.defaultPrivacyLevel ?? configuration.defaultPrivacyLevel - if (PRIVACY_LEVEL_STRICTNESS[nextPrivacyLevel] > PRIVACY_LEVEL_STRICTNESS[drawnPrivacyLevel]) { - sessionManager.expire() - return + // Whether this session is collected is read off the session itself rather than reconstructed + // by comparing rates: the session IS the outcome its draw produced, and an outcome is the only + // thing 0 and 100 let us assert anything about. + const isCollected = isTypeTracked(session.trackingType) + + // Only a session that is being collected can be recording, and only a recording can be too + // plain. A sampled-out visitor uploads nothing, so a stricter level has nothing to protect + // there — and nothing to compare against either: a session that is not collected is given no + // id, so no draw is recorded for it and what it was drawn under cannot be read back here. The + // comparison 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 stays. + if (isCollected) { + // 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() + return + } } if (forcedSession) { // The host application has taken this page off the rates deliberately, and every draw it // makes from now on is collected whatever the console says. Ending the session on a rate - // would only replace it with another forced one — the same difference, forever. + // would only replace it with another forced one — the same difference, forever. The flag is + // this page's: another tab of the same visitor that never called `setForcedSession` reads + // the shared session as an ordinary one and may end it on a rate. return } - // Whether this session is collected is read off the session itself rather than reconstructed - // by comparing rates: the session IS the outcome its draw produced, and an outcome is the only - // thing 0 and 100 let us assert anything about. - const isCollected = isTypeTracked(session.trackingType) const { sessionSampleRate } = resolveSampleRates(configuration, remote) if ((sessionSampleRate === 0 && isCollected) || (sessionSampleRate === 100 && !isCollected)) { sessionManager.expire() From 738ef961c7ac5956020bd168ddcb93e28fd49c12 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:13:31 -0700 Subject: [PATCH 5/7] test(rum): cover the decision paths nothing was holding Three paths the implementation documents had no test standing on them, each found by mutating the source and watching the suite stay green: - A session drawn before any settings arrived. A draw that lands exactly on the init values records nothing, so the level such a session runs under can only be read back off init -- the fallback every existing privacy test stepped around by storing settings before starting. Deleting that fallback passed the whole suite. - A response that carries no rate at all, with `beforeSampling` turning the delivered custom values into the decision. This is the "called away from a draw" contract, and both resolving the rate without the callback and bailing out when the console sends no rate passed the whole suite. - The console's kill switch, which stores a version and nothing else and so puts the rates back to the ones init passed. That is a change like any other, and where init never collected it is the decisive one. Also renames the opt-out test to what it actually pins down. Its store key is one no implementation could derive, so it cannot witness the store being left alone; what it does witness is the decision surviving an undefined `remoteConfig`. --- .../src/domain/rumSessionManager.spec.ts | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 261906ed64..3ba8dfa368 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -892,6 +892,47 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeTrue() }) + it('ends a session drawn before any settings arrived when the first ones tighten the level', () => { + // Nothing in storage yet, so this session was drawn on the init values — and a draw that + // lands exactly on them records nothing, which is why the level it runs under can only be + // read back off init. The recorder falls back the same way, so this is the level the page + // is really being masked with. + startWith({ sessionSampleRate: 100, defaultPrivacyLevel: 'allow' }) + + deliver({ version: 1, sessionSampleRate: 100, defaultPrivacyLevel: 'mask' }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a collected session when the callback turns the delivered values into a zero', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ + sessionSampleRate: 100, + beforeSampling: ({ custom }) => (custom?.optOut === true ? { sessionSampleRate: 0 } : undefined), + }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + // The response carries no rate at all: the console ships the data and the application's own + // code turns it into the decision. Asking the callback away from a draw is the whole reason + // that decision can reach the session already running. + deliver({ version: 2, custom: { optOut: true } }) + + expect(isSessionEnded()).toBeTrue() + }) + + it('ends a collected session when the settings are switched off and init never collected', () => { + storeRemote({ version: 1, sessionSampleRate: 100 }) + startWith({ sessionSampleRate: 0 }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + // Turning remote configuration off in the console stores the version and nothing else, so + // the rates go back to the ones the site passed to init. That is a change like any other, + // and here it is the decisive one. + deliver({ version: 2 }) + + expect(isSessionEnded()).toBeTrue() + }) + it('draws the session that follows on the settings that have just landed', () => { storeRemote({ version: 1, sessionSampleRate: 0 }) startWith({ sessionSampleRate: 0 }) @@ -1000,13 +1041,14 @@ describe('rum session manager', () => { expect(isSessionEnded()).toBeFalse() }) - it('reads nothing out of the settings store when the site did not opt in', () => { + it('does not fall over when the site never opted in and has no settings store', () => { storeRemote({ version: 1, sessionSampleRate: 100 }) startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 0, drawStoreKey: DRAW_KEY } }) - // Such a site never fetches, so this can only ever be reached by hand. What matters is that - // the settings store is out of reach without the opt-in: the rate that would apply is the - // one init passed, which is the one this session was already drawn on. + // Such a site never fetches, so the announcement can only ever be reached by hand and the + // store key below is one nothing would look under. All this pins down is that the decision + // survives `remoteConfig` being undefined; that the opt-out is respected is settled where + // the fetcher is never started, not here. deliver({ version: 2, sessionSampleRate: 100 }) expect(expireSessionSpy).not.toHaveBeenCalled() From 20fbc0b47bcd0693e0f04cf6f4bffc95780f2079 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:21:30 -0700 Subject: [PATCH 6/7] docs(changelog): say what a decisive publish does to a running visit --- CHANGELOG.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4999e4a1d3..aded3bd2ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,42 @@ --- +## Unreleased + +- ✨ Three changes published from the console now end the running session, so they reach the + visitor at their next interaction instead of waiting for that session to end on its own: a + session sample rate of 0 while the visitor is being collected — the emergency stop — a rate of + 100 while they are not, and a stricter Session Replay privacy level while they are being + collected. The session that ends is collected to its end as it began, so no recording is left + masked in one half and plain in the other. Every other change — any rate between 0 and 100, a + loosening privacy level, the replay and trace rates, the custom values — still waits for the next + session. Nothing here happens without `remoteConfigurationEnabled: true`. +- 📝 What you will see on the day you publish one of those three: session counts rise and average + session length drops, because each affected visitor's running session is split at that moment; a + replay in progress ends at the split and a new one starts under the new settings; a rate of 100 + makes previously invisible visitors appear within hours rather than the next day, so collected + volume climbs the same day. That is the change taking effect, not a defect. +- 📝 "At once" means "as soon as this client hears of the change". Settings are fetched at page load + and at each new session, never on a timer, so a page nobody reloads hears of a publish at its next + session boundary — at most four hours away, the cap on a session's life. Opening a tab or + reloading any page fetches immediately and ends the session every tab shares, which is why a + visitor who touches the site converges in seconds. A change that is not one of the three still + takes effect one session after that. +- 📝 The three act on what actually changed, not on the activation mode recorded with the publish: + a change the console files as "next session" still ends the running session if it is one of them. +- 📝 `beforeSampling` is now also consulted when settings arrive, away from any draw, to work out + which rate would apply. It must stay free of side effects and answer the same way for the same + input: a callback that draws its own lottery — answering 0 or 100 at random — can end a session + that a steady answer would have left running. +- 📝 A session forced with `setForcedSession()` is never ended by a rate: forcing decides whether + this visitor is collected, and every draw it makes is collected whatever the console says. A + stricter privacy level still ends it, because forcing says nothing about how much of the page may + be uploaded in the clear. The page forces the next session on its own, so the visit continues as + two sessions. +- 📝 Turning remote configuration off is itself a change: the rates go back to the ones passed to + `init`. On a site whose init rate is 0, switching it off stops collection at once rather than at + the next session. + ## v0.2.0 - 💥 **Breaking**: `remoteConfigurationId` is gone from `RumInitConfiguration`. It fetched a From 9c76b2062744f5d4f54d4e03e43101ea388a0e89 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:34:06 -0700 Subject: [PATCH 7/7] fix(rum): let a rate of a hundred through to a forced page that collects nothing The exemption that keeps a rate from ending a forced session was written for the case where ending it changes nothing: the page collects this visitor whatever the console says, so the replacement session would be the same session again. That reasoning runs out when the session is not collected. A page can adopt one drawn by a tab that never forced anything, and there a rate of 100 has something to change -- it is exactly the draw the page asked for. The guard now carries the precondition its own reasoning rests on. Also corrects two claims in the changelog entry that the code does not make good on: custom values do not always wait for the next session, since `beforeSampling` can turn them into a decisive rate -- the flagship pattern for this feature, and something the suite already pins down -- and the session after a split carries a new recording only if its draw keeps one. --- CHANGELOG.md | 18 +++++++++------ .../src/domain/rumSessionManager.spec.ts | 22 +++++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 12 +++++----- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aded3bd2ec..b97349e43d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,13 +26,16 @@ 100 while they are not, and a stricter Session Replay privacy level while they are being collected. The session that ends is collected to its end as it began, so no recording is left masked in one half and plain in the other. Every other change — any rate between 0 and 100, a - loosening privacy level, the replay and trace rates, the custom values — still waits for the next - session. Nothing here happens without `remoteConfigurationEnabled: true`. + loosening privacy level, the replay and trace rates — still waits for the next session. Custom + values wait on their own too, but not once `beforeSampling` turns them into one of the three: a + callback answering 0 for the values just published ends the session exactly as a published 0 + would. Nothing here happens without `remoteConfigurationEnabled: true`. - 📝 What you will see on the day you publish one of those three: session counts rise and average session length drops, because each affected visitor's running session is split at that moment; a - replay in progress ends at the split and a new one starts under the new settings; a rate of 100 - makes previously invisible visitors appear within hours rather than the next day, so collected - volume climbs the same day. That is the change taking effect, not a defect. + replay in progress ends at the split, and the session that follows draws again, so it carries a + new recording only if that draw keeps one; a rate of 100 makes previously invisible visitors + appear within hours rather than the next day, so collected volume climbs the same day. That is + the change taking effect, not a defect. - 📝 "At once" means "as soon as this client hears of the change". Settings are fetched at page load and at each new session, never on a timer, so a page nobody reloads hears of a publish at its next session boundary — at most four hours away, the cap on a session's life. Opening a tab or @@ -45,8 +48,9 @@ which rate would apply. It must stay free of side effects and answer the same way for the same input: a callback that draws its own lottery — answering 0 or 100 at random — can end a session that a steady answer would have left running. -- 📝 A session forced with `setForcedSession()` is never ended by a rate: forcing decides whether - this visitor is collected, and every draw it makes is collected whatever the console says. A +- 📝 A session forced with `setForcedSession()` is not ended by a rate while it is being collected: + forcing decides whether this visitor is collected, and every draw the page makes is collected + whatever the console says, so ending it would only produce the same session again. A stricter privacy level still ends it, because forcing says nothing about how much of the page may be uploaded in the clear. The page forces the next session on its own, so the visit continues as two sessions. diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 3ba8dfa368..cfa6debf94 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -1166,6 +1166,28 @@ describe('rum session manager', () => { return rumSessionManager } + it('is still ended by a rate of a hundred when the session it adopted collects nothing', () => { + storeRemote({ version: 1, sessionSampleRate: 0 }) + setCookie(SESSION_STORE_KEY, `rum=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) + const rumSessionManager = startWith({ sessionSampleRate: 0 }) + + // Forcing ends a session that collects nothing, so that the next draw can be the forced + // one. Before that draw happens, a tab that never forced anything starts a session of its + // own, and this page adopts it: the page is forced while the session it holds is not. + rumSessionManager.setForcedSession() + setCookie(SESSION_STORE_KEY, `rum=0&created=${Date.now()}&expire=${Date.now() + DURATION}`, DURATION) + 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() + + // Here the rate has something to change, so the exemption does not apply: ending the + // session is what lets the next draw be the forced one this page asked for. + deliver({ version: 2, sessionSampleRate: 100 }) + + expect(isSessionEnded()).toBeTrue() + }) + it('is not ended by a rate, since every draw it makes is collected anyway', () => { storeRemote({ version: 1, sessionSampleRate: 0 }) startForced() diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index af55201dbe..dc82b415c9 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -267,12 +267,14 @@ export function startRumSessionManager( } } - if (forcedSession) { + if (forcedSession && isCollected) { // The host application has taken this page off the rates deliberately, and every draw it - // makes from now on is collected whatever the console says. Ending the session on a rate - // would only replace it with another forced one — the same difference, forever. The flag is - // this page's: another tab of the same visitor that never called `setForcedSession` reads - // the shared session as an ordinary one and may end it on a rate. + // makes from now on is collected whatever the console says. Ending a collected session on a + // rate would only replace it with another collected one — the same difference, forever. That + // reasoning runs out when the session is not collected: this page can adopt one an unforced + // tab drew, and there a rate of 100 has something to change, so it is left to the rule + // below. The flag is this page's either way — another tab that never called + // `setForcedSession` reads the shared session as an ordinary one. return }