From f9e7bc7fbf09c55873d73a67a900348399a3da55 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 15 Oct 2026 05:06:59 -0700 Subject: [PATCH] fix(rum): read the previous release's remote settings until a release stores its own Stored remote settings are keyed by application version, so that two releases live at the same time can be answered differently. The cost was that every deploy started with no entry of its own: each visitor's first session on the new release was drawn on the init settings and reported no remote configuration version, until the fetch it triggered was stored for the next session. When this release has no entry yet, readRemoteConfig now reads the entry the same site, application and environment most recently stored under another release. Those are the settings this release gets too unless the console targets releases apart, and even then they are the latest decision on record, which the init settings are not. The release's own answer replaces them as soon as it is stored. The version floor in store() still only looks at this release's own entry, so a borrowed version neither refuses this release's answer nor makes it look like a repeat, and REMOTE_CONFIGURATION_STORED still fires on the first own write. --- CHANGELOG.md | 8 ++ .../configuration/remoteConfiguration.spec.ts | 79 +++++++++++++++++++ .../configuration/remoteConfiguration.ts | 77 +++++++++++++++--- .../src/domain/rumSessionManager.spec.ts | 20 +++++ 4 files changed, 173 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f3e75030..6b8dc7c17a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,14 @@ --- +## Unreleased + +- 🐛 With `remoteConfigurationEnabled`, the first session a visitor starts on a newly deployed + release of the site no longer falls back to the init settings. Stored settings are kept per + application version, so every deploy used to put that session on the init rates and report no + remote configuration version for it. That session now uses the settings the same application and environment last + stored under another release, until this release's own answer arrives. + ## v0.3.0 - 💥 **Breaking**: `startSessionReplayRecordingManually` no longer defaults to `true` on an init diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index df490e069a..85e5fd2a5f 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -873,6 +873,85 @@ describe('remoteConfiguration', () => { }) }) + describe('a release that has not stored its own entry yet', () => { + // The key carries the application version, so the first session after every deploy finds no + // entry of its own. Reading the local settings there would put that session on the init values + // and report no version at all, on every deploy. + const keyOf = (partial: Partial) => + buildRemoteConfigSetup({ ...INIT_CONFIGURATION, ...partial })!.storeKey + const previousReleaseKey = keyOf({ version: '1.2.2' }) + const olderReleaseKey = keyOf({ version: '1.2.1' }) + const otherApplicationKey = keyOf({ applicationId: 'other-app', version: '1.2.2' }) + const otherEnvironmentKey = keyOf({ env: 'production', version: '1.2.2' }) + + beforeEach(() => { + registerCleanupTask(() => { + ;[previousReleaseKey, olderReleaseKey, otherApplicationKey, otherEnvironmentKey].forEach((key) => + localStorage.removeItem(key) + ) + }) + }) + + function writeEntryAt(key: string, age: number, values: Record) { + localStorage.setItem(key, JSON.stringify({ ...values, t: dateNow() - age })) + } + + it('reads the entry another release of the same application and environment stored last', () => { + writeEntryAt(olderReleaseKey, 2 * ONE_SECOND, { sessionSampleRate: 5, version: 2 }) + writeEntryAt(previousReleaseKey, ONE_SECOND, { sessionSampleRate: 42, version: 3 }) + + expect(readRemoteConfig(setup)).toEqual({ sessionSampleRate: 42, version: 3 }) + }) + + it('never reads the entry of another application or environment', () => { + writeEntryAt(otherApplicationKey, ONE_SECOND, { sessionSampleRate: 42, version: 3 }) + writeEntryAt(otherEnvironmentKey, ONE_SECOND, { sessionSampleRate: 42, version: 3 }) + + expect(readRemoteConfig(setup)).toEqual({}) + }) + + it('passes over an entry that does not say when it was written', () => { + localStorage.setItem(previousReleaseKey, JSON.stringify({ sessionSampleRate: 42, version: 3 })) + + expect(readRemoteConfig(setup)).toEqual({}) + }) + + it('prefers its own entry, however recently another release wrote one', () => { + writeEntryAt(setup!.storeKey, ONE_DAY, { sessionSampleRate: 7, version: 3 }) + writeEntryAt(previousReleaseKey, 0, { sessionSampleRate: 42, version: 4 }) + + expect(readRemoteConfig(setup)).toEqual({ sessionSampleRate: 7, version: 3 }) + }) + + it('tells the server the version it is running on meanwhile', (done) => { + writeEntryAt(previousReleaseKey, ONE_SECOND, { sessionSampleRate: 42, version: 3 }) + + interceptor.withMockXhr((xhr) => { + expect(xhr.url).toContain('applied_version=3') + done() + }) + start(configurationWith()) + }) + + it("stores and announces its own answer, even one numbered below the other release's", (done) => { + // The other release's number is not this release's floor: under targeting the two can be + // answered differently, and refusing this release's own answer would leave it on borrowed + // settings for good. + writeEntryAt(previousReleaseKey, ONE_SECOND, { sessionSampleRate: 42, version: 9 }) + const notified = jasmine.createSpy('remoteConfigurationStored') + lifeCycle.subscribe(LifeCycleEventType.REMOTE_CONFIGURATION_STORED, notified) + + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 1 }, version: 3 })) + + expect(readRemoteConfig(setup)).toEqual({ sessionSampleRate: 1, version: 3 }) + expect(notified).toHaveBeenCalledTimes(1) + done() + }) + start(configurationWith()) + }) + }) + describe('the storage key', () => { it('separates applications, environments and versions', () => { const keyOf = (partial: Partial) => diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 84b3cc34a1..84e582504c 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -229,19 +229,71 @@ function isSupportedResponse(body: unknown): body is RemoteConfigurationResponse * Read the settings that apply right now. Reading straight from storage rather than from a value * held in memory is what lets a value fetched by one page load apply to the very first session of * the next one, instead of every visit starting on the local settings until a request comes back. + * + * A release that has not stored its own entry yet reads the entry this application and environment + * last stored under another release. The key carries the application version, so without this every + * deploy would put each visitor's first session on it back on the local settings, reporting no + * version. The settings another release was given are what this one gets too unless the console targets + * releases apart, and even then they are the latest decision on record, which the local settings are + * not. This release's own answer replaces them as soon as it is stored. */ export function readRemoteConfig(setup: RemoteConfigSetup | undefined): RemoteConfigValues { if (!setup) { return {} } + return readEntry(setup.storeKey) ?? readEntry(findLatestOtherReleaseKey(setup.storeKey)) ?? {} +} +/** + * The values stored under `key`, or undefined when there is no entry to read. Storage unavailable or + * holding something we did not write reads as no entry: the caller falls back to the local settings. + */ +function readEntry(key: string | undefined): RemoteConfigValues | undefined { + if (key === undefined) { + return undefined + } try { - const stored = localStorage.getItem(setup.storeKey) - return stored ? readStoredValues(JSON.parse(stored)) : {} + const stored = localStorage.getItem(key) + return stored === null ? undefined : readStoredValues(JSON.parse(stored)) } catch { - // Storage unavailable or holding something we did not write: fall back to the local settings. - return {} + return undefined + } +} + +/** + * The key of the most recently written entry of the same application and environment under another + * release, if any. The version is the last part of the key and the separator cannot occur inside a + * part (see `buildKey`), so everything up to the last separator identifies the application and + * environment exactly. + * + * An entry without a write time cannot be ranked against the others and is passed over. + */ +function findLatestOtherReleaseKey(ownKey: string) { + const separatorIndex = ownKey.lastIndexOf('|') + if (separatorIndex === -1) { + // Not a key `buildStoreKey` built: there is no identity to match, and an empty prefix would + // match every key on the origin. + return undefined + } + const identityPrefix = ownKey.slice(0, separatorIndex + 1) + let latestKey: string | undefined + let latestWriteTime = -Infinity + try { + for (let i = 0; i < localStorage.length; i += 1) { + const key = localStorage.key(i) + if (key === null || key === ownKey || key.indexOf(identityPrefix) !== 0) { + continue + } + const writeTime = readWriteTime(key) + if (writeTime !== undefined && writeTime > latestWriteTime) { + latestKey = key + latestWriteTime = writeTime + } + } + } catch { + return undefined } + return latestKey } /** @@ -460,8 +512,11 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) // would strand every client that had seen the higher number until the entry is replaced. // // What it is compared against is storage, not a version held in memory here, because the two - // requests that can cross are two pages, and storage is the only thing they share. - const stored = readRemoteConfig(setup) + // requests that can cross are two pages, and storage is the only thing they share. Only this + // release's own entry counts: the one another release left is read in its place until this one + // stores its own, but it is not this release's answer, so it neither refuses one nor makes one + // look like a repeat. + const stored = readEntry(setup.storeKey) ?? {} const storedVersion = stored.version if (storedVersion !== undefined && response.version < storedVersion) { // Refused, but the entry is plainly still in use — a request was just made for it and answered. @@ -673,11 +728,11 @@ function validFetchTimeout(timeout: number | undefined) { * as long as both are being served — so the version has to stay, and cannot be dropped to make the * limitation below go away. * - * That costs an entry per release — the first session after one reads the local settings, and the - * entry the release before it used is never read again — so the entries are swept by age rather - * than left to accumulate in a quota the host application shares. See `sweepAbandonedEntries` for - * why age is the only thing that can tell an abandoned entry from the entry of a tab still open on - * yesterday's release. + * That costs an entry per release. The first session after a deploy reads the entry the release + * before it stored, until its own arrives (see `readRemoteConfig`), and after that the older entry + * is never read again — so the entries are swept by age rather than left to accumulate in a quota + * the host application shares. See `sweepAbandonedEntries` for why age is the only thing that can + * tell an abandoned entry from the entry of a tab still open on yesterday's release. */ function buildStoreKey(initConfiguration: RumInitConfiguration) { return buildKey(STORE_KEY_PREFIX, identityParts(initConfiguration).concat(initConfiguration.version ?? '')) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index bdcebf5e91..faaeb94445 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -247,6 +247,26 @@ describe('rum session manager', () => { expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) }) + it('draws the first session of a new release on the rate the previous release stored', () => { + // Store keys carry the application version, so a deploy starts with no entry of its own. + const previousReleaseKey = '_fc_rc_1_site|app|env|1.0.0' + localStorage.setItem( + previousReleaseKey, + JSON.stringify({ sessionSampleRate: 100, sessionReplaySampleRate: 100, version: 5, t: Date.now() }) + ) + registerCleanupTask(() => localStorage.removeItem(previousReleaseKey)) + + startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 0, + remoteConfig: { ...REMOTE_SAMPLING_SETUP, storeKey: '_fc_rc_1_site|app|env|2.0.0' }, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + }) + it('draws replay on the remote replay rate', () => { storeRemoteConfigValues({ sessionReplaySampleRate: 100 })