From d80109a2af3e23564f8ed72caad90d06b1d9fab6 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 15 Oct 2026 06:14:43 -0700 Subject: [PATCH] fix(rum): keep remote settings across releases of the application The stored remote settings were keyed by application version as well as site, application and environment. Every deploy therefore started with no entry: each visitor's first session on the new release was drawn on the init settings and reported no remote configuration version, and the fetch it triggered only applied to the session after it. The key no longer includes the application version, so the settings the previous release stored are read by the next one. Releases of one site live at the same time share the entry, which is right as long as they are given the same settings, and is still the latest decision on record if they ever are not: the fetch every page makes stores its own release's answer for its next draw. The SDK keeps sending app_version on the request. Entries written by earlier builds carry the version in their key and are no longer read; the existing age-based sweep removes them after two days. The first session after upgrading the SDK uses the init settings once. --- CHANGELOG.md | 9 +++ .../configuration/remoteConfiguration.spec.ts | 38 ++++++++++--- .../configuration/remoteConfiguration.ts | 56 ++++++++----------- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f3e75030..779b13f49b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,15 @@ --- +## Unreleased + +- 🐛 With `remoteConfigurationEnabled`, settings stored by one release of a site are now read by + the next. The stored settings used to be keyed by application version as well, so the first + session a visitor started on a newly deployed release found nothing, was drawn on the init + settings, and reported no remote configuration version. Entries left by earlier builds are no + longer read and are swept after two days; the first session after upgrading the SDK uses the + init settings once. + ## 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..4b79081e71 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -777,10 +777,10 @@ describe('remoteConfiguration', () => { }) }) - describe('sweeping the entries of releases nobody runs', () => { - // The key carries the application version, so every release leaves one behind. Without a sweep - // they accumulate for good in a quota the host application shares. - const otherReleaseKey = buildRemoteConfigSetup({ ...INIT_CONFIGURATION, version: '0.9.0' })!.storeKey + describe('sweeping the entries nothing reads', () => { + // Builds that keyed the entry by application version left one behind per release. Without a + // sweep they accumulate for good in a quota the host application shares. + const otherReleaseKey = `${buildRemoteConfigSetup(INIT_CONFIGURATION)!.storeKey}|0.9.0` const drawKey = buildDrawStoreKey(INIT_CONFIGURATION) const foreignKey = 'a-key-the-host-application-owns' @@ -874,13 +874,33 @@ describe('remoteConfiguration', () => { }) describe('the storage key', () => { - it('separates applications, environments and versions', () => { + it('separates applications and environments', () => { const keyOf = (partial: Partial) => buildRemoteConfigSetup({ ...INIT_CONFIGURATION, ...partial })!.storeKey expect(keyOf({})).not.toEqual(keyOf({ applicationId: 'other' })) expect(keyOf({})).not.toEqual(keyOf({ env: 'production' })) - expect(keyOf({})).not.toEqual(keyOf({ version: '1.2.4' })) + }) + + it('is shared by every release of the application', () => { + // Keyed by version, the first session after every deploy found no entry and was drawn on the + // init values. The settings the previous release stored are the better answer. + const keyOf = (partial: Partial) => + buildRemoteConfigSetup({ ...INIT_CONFIGURATION, ...partial })!.storeKey + + expect(keyOf({ version: '1.2.4' })).toEqual(keyOf({})) + expect(keyOf({ version: undefined })).toEqual(keyOf({})) + }) + + it('lets a new release read the settings the previous one stored', (done) => { + const previousRelease = buildRemoteConfigSetup({ ...INIT_CONFIGURATION, version: '1.2.2' })! + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 42 }, version: 3 })) + + expect(readRemoteConfig(setup)).toEqual({ sessionSampleRate: 42, version: 3 }) + done() + }) + start(configurationWith({ remoteConfig: previousRelease })) }) it('cannot be spelled the same way by two different identities', () => { @@ -889,7 +909,7 @@ describe('remoteConfiguration', () => { const keyOf = (partial: Partial) => buildRemoteConfigSetup({ ...INIT_CONFIGURATION, ...partial })!.storeKey - expect(keyOf({ env: 'prod', version: '1_0' })).not.toEqual(keyOf({ env: 'prod_1', version: '0' })) + expect(keyOf({ applicationId: 'app', env: '1_0' })).not.toEqual(keyOf({ applicationId: 'app_1', env: '0' })) }) it('separates whatever the server is able to tell apart', () => { @@ -906,7 +926,8 @@ describe('remoteConfiguration', () => { // part. `service` is on it while it is still inert: nothing reports it today, so the check // skips it, and the day it starts being reported this turns into a real assertion. Fields // that identify the application rather than target it are left off — `clientToken` travels - // on the request but the key separates the same clients through `applicationId`. + // on the request but the key separates the same clients through `applicationId`. So is the + // application version, deliberately: see `buildStoreKey`, and the test below that pins it. const urlOf = (partial: Partial) => buildRemoteConfigSetup({ ...INIT_CONFIGURATION, ...partial })!.buildUrl(undefined) const keyOf = (partial: Partial) => @@ -914,7 +935,6 @@ describe('remoteConfiguration', () => { const candidates: Array> = [ { env: 'production' }, - { version: '9.9.9' }, { service: 'checkout' }, { applicationId: 'other-app' }, ] diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 84b3cc34a1..36bd4821fe 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -60,15 +60,15 @@ const DRAW_STORE_KEY_PREFIX = '_fc_draw_1_' const DEFAULT_FETCH_TIMEOUT = 3 * ONE_SECOND /** - * How long an entry may go unrefreshed before `sweepAbandonedEntries` treats it as belonging to a - * release nobody is running any more. + * How long an entry may go unrefreshed before `sweepAbandonedEntries` treats it as one nothing reads + * any more. * * An entry written by this SDK that is still being read is also being rewritten: the page reading * it refetches at every session renewal and stores the answer. So the threshold only has to clear * the longest a live entry can legitimately stay silent, which is the longest session (four hours, * after which a renewal refetches) plus the longest endpoint outage we are willing to survive * without dropping anyone — a failed fetch stores nothing. Two days leaves better than a day and a - * half of outage, and still bounds the leak at the entries of two days of releases. + * half of outage. * * Erring long is deliberate. Deleting an entry too early costs the page reading it one session on * its init values; keeping a dead one costs a few hundred bytes. @@ -346,7 +346,7 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet const renewSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, onTrigger) - // Before the first request, so the room the entries of dead releases are holding is free by the + // Before the first request, so the room abandoned entries are holding is free by the // time there is an answer to store. See `sweepAbandonedEntries`. sweepAbandonedEntries(setup.storeKey) @@ -553,13 +553,12 @@ function writeEntry(setup: RemoteConfigSetup, values: RemoteConfigValues) { } /** - * Delete the entries of releases nobody is running any more. + * Delete the entries nothing reads any more. * - * The store key carries the application version, because two releases live at the same time are - * entitled to different rates and one entry between them would have each overwrite the other's at - * every fetch. The cost of that is an entry per release, and nothing ever read or removed them - * again — on a site that deploys daily they accumulate for good, in a quota the host application - * shares. + * Builds before this one keyed the entry by application version as well, which left an entry per + * release that nothing ever read or removed again — on a site that deploys daily they accumulate for + * good, in a quota the host application shares. Those entries, and the entries of an application or + * environment a visitor no longer uses, are what this clears. * * Run once per initialisation rather than at every write. Sweeping on write was the shape tried * first and it is the wrong one: `localStorage` is synchronous, a session renewal is a hot path, @@ -660,35 +659,26 @@ function validFetchTimeout(timeout: number | undefined) { } /** - * The key covers everything that can change the answer — which application, on which host, in which - * environment, at which version — so a visitor moving between two of them does not read the other's - * rates. It deliberately leaves out the SDK version: including it would throw the stored rates away - * on every SDK upgrade and put the first session after an upgrade back on the local settings. The - * storage format version lives in `STORE_KEY_PREFIX` instead, so only a real format change orphans - * the cache. + * The key covers which application, on which host, in which environment, so a visitor moving + * between two of them does not read the other's rates. * - * The application version is in it for a reason the SDK version does not have: settings can be - * targeted at a release, so two releases of one site that are live at the same time are entitled to - * different rates. One entry between them would have each overwrite the other's on every fetch, for - * 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. + * It deliberately leaves out both versions. The SDK version, because including it would throw the + * stored rates away on every SDK upgrade; the storage format version lives in `STORE_KEY_PREFIX` + * instead, so only a real format change orphans the cache. The application version, because the + * settings stored by the release a visitor used last are the best answer the first session on a new + * release can get: keyed by version, that session found nothing and was drawn on the init values, + * after every deploy. Releases of one site live at the same time share the entry, which is right as + * long as they are given the same settings, and is still the latest decision on record if they ever + * are not — the fetch every page makes stores its own release's answer for its next draw. */ function buildStoreKey(initConfiguration: RumInitConfiguration) { - return buildKey(STORE_KEY_PREFIX, identityParts(initConfiguration).concat(initConfiguration.version ?? '')) + return buildKey(STORE_KEY_PREFIX, identityParts(initConfiguration)) } /** - * The key of the draw record the session manager writes. It shares the identity of the settings - * cache but deliberately not its application version: the record belongs to the session, and a - * session outlives a deploy. Keying it by version would lose the decision the moment a visitor with - * a live session navigates onto a newly deployed page, putting that session's events and its - * tracer back on the init values — the mid-session flip the record exists to prevent. + * The key of the draw record the session manager writes. It has the same identity as the settings + * cache — the record belongs to the session, and a session outlives a deploy, so it is not keyed by + * application version either — and its own prefix, so the two never collide. * * Built for every site, not only the ones that opted in: `beforeSampling` and `setForcedSession()` * move a draw off the init values with remote configuration switched off.