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 })