fix(rum): read the previous release's remote settings until a release stores its own

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

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<RumInitConfiguration>) =>
+      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<string, unknown>) {
+      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<RumInitConfiguration>) =>

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 ?? ''))

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