From cae21374595506df6ff66d9beb0b671e8bc2da4a Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:00:23 -0700 Subject: [PATCH 1/3] 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 f1bcb1a2e6..c87ebc28e9 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -351,34 +351,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) @@ -396,6 +369,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 bae525f1fa45a94014044b2e5a7a5f436a83fde2 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:00:41 -0700 Subject: [PATCH 2/3] 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 | 21 +- .../configuration/remoteConfiguration.spec.ts | 61 ++++ .../configuration/remoteConfiguration.ts | 35 ++- packages/rum-core/src/domain/lifeCycle.ts | 8 + .../src/domain/rumSessionManager.spec.ts | 297 ++++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 96 +++++- 6 files changed, 504 insertions(+), 14 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index ac458dcb83..7c1563f54f 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -56,6 +56,13 @@ export interface RumInitConfiguration extends InitConfiguration { * collects, 0 never does — or nothing to leave the incoming rates alone. Runs inside session * creation, so it must be fast and synchronous; a thrown error or an out-of-range value is * ignored. A session already under way is never re-decided. + * + * It must be free of side effects, and must answer the same way for the same input. The SDK + * calls it outside a draw as well — 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 anything the + * callback does besides returning a rate (a metric, a log, a counter) happens more often than + * there are sessions, and a callback that answers differently each time can keep ending the + * session it was just asked about. */ beforeSampling?: BeforeSamplingCallback | undefined /** @@ -77,9 +84,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. * * @default false */ diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index 0365eb7b70..8b48b5bc10 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 9c4f01e0b8..fe4f1661de 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -19,11 +19,17 @@ 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; the server's `ttl` field is accepted and ignored, reserved for a future - * polling mode. + * 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 happens 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 server's `ttl` field is + * accepted and ignored, reserved for a future polling mode. 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. * * 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. @@ -100,6 +106,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 @@ -248,7 +257,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) { @@ -323,6 +337,11 @@ function fetchRemoteConfiguration( xhr.send() } +/** + * 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 can @@ -335,7 +354,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 } @@ -369,8 +388,10 @@ 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: the values simply do not survive this page load. + 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 92042a51e4..6f14fc8ffb 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -718,6 +718,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 }: { configuration?: Partial } = {}) { const sessionManager = startRumSessionManager( mockRumConfiguration({ diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index c87ebc28e9..739a41e698 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, @@ -173,6 +174,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() @@ -203,7 +276,10 @@ export function startRumSessionManager( }, expire: sessionManager.expire, expireObservable: sessionManager.expireObservable, - stop: drawnHistory.stop, + stop: () => { + remoteConfigSubscription.unsubscribe() + drawnHistory.stop() + }, setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), // FLASHCAT FORK - the escape hatch for "collect this visitor NOW": the host application knows // who needs debugging (its own allow-list, a support flow), the SDK only provides the switch. @@ -377,8 +453,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 @@ -407,6 +484,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 bb01eb9e844e61073358f6311772463e2c0f8fdd Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 30 Aug 2026 20:04:01 -0700 Subject: [PATCH 3/3] 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 6f14fc8ffb..88908991fa 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -869,10 +869,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 739a41e698..7604c22a9c 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -201,9 +201,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