feat(rum): end the session when new settings decide its fate

CHANGELOG.md

@@ -18,6 +18,46 @@
 
 ---
 
+## Unreleased
+
+- ✨ Three changes published from the console now end the running session, so they reach the
+  visitor at their next interaction instead of waiting for that session to end on its own: a
+  session sample rate of 0 while the visitor is being collected — the emergency stop — a rate of
+  100 while they are not, and a stricter Session Replay privacy level while they are being
+  collected. The session that ends is collected to its end as it began, so no recording is left
+  masked in one half and plain in the other. Every other change — any rate between 0 and 100, a
+  loosening privacy level, the replay and trace rates — still waits for the next session. Custom
+  values wait on their own too, but not once `beforeSampling` turns them into one of the three: a
+  callback answering 0 for the values just published ends the session exactly as a published 0
+  would. Nothing here happens without `remoteConfigurationEnabled: true`.
+- 📝 What you will see on the day you publish one of those three: session counts rise and average
+  session length drops, because each affected visitor's running session is split at that moment; a
+  replay in progress ends at the split, and the session that follows draws again, so it carries a
+  new recording only if that draw keeps one; a rate of 100 makes previously invisible visitors
+  appear within hours rather than the next day, so collected volume climbs the same day. That is
+  the change taking effect, not a defect.
+- 📝 "At once" means "as soon as this client hears of the change". Settings are fetched at page load
+  and at each new session, never on a timer, so a page nobody reloads hears of a publish at its next
+  session boundary — at most four hours away, the cap on a session's life. Opening a tab or
+  reloading any page fetches immediately and ends the session every tab shares, which is why a
+  visitor who touches the site converges in seconds. A change that is not one of the three still
+  takes effect one session after that.
+- 📝 The three act on what actually changed, not on the activation mode recorded with the publish:
+  a change the console files as "next session" still ends the running session if it is one of them.
+- 📝 `beforeSampling` is now also consulted when settings arrive, away from any draw, to work out
+  which rate would apply. It must stay free of side effects and answer the same way for the same
+  input: a callback that draws its own lottery — answering 0 or 100 at random — can end a session
+  that a steady answer would have left running.
+- 📝 A session forced with `setForcedSession()` is not ended by a rate while it is being collected:
+  forcing decides whether this visitor is collected, and every draw the page makes is collected
+  whatever the console says, so ending it would only produce the same session again. A
+  stricter privacy level still ends it, because forcing says nothing about how much of the page may
+  be uploaded in the clear. The page forces the next session on its own, so the visit continues as
+  two sessions.
+- 📝 Turning remote configuration off is itself a change: the rates go back to the ones passed to
+  `init`. On a site whose init rate is 0, switching it off stops collection at once rather than at
+  the next session.
+
 ## v0.2.0
 
 - 💥 **Breaking**: `remoteConfigurationId` is gone from `RumInitConfiguration`. It fetched a

packages/rum-core/src/domain/configuration/configuration.ts

@@ -62,6 +62,12 @@ export interface RumInitConfiguration extends InitConfiguration {
    * a single session. Keep it a pure decision: side effects will be repeated, and only the last
    * call's return value is used.
    *
+   * The SDK also calls it away from a draw: 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 it
+   * must answer the same way for the same input — one that answers differently each time can end a
+   * session that a steady one would have left running — and anything it does besides returning a
+   * rate (a metric, a log, a counter) happens more often than there are sessions.
+   *
    * Its failure modes never reach session creation: a thrown error or an out-of-range value leaves
    * the incoming rate in place, and a value that is not a function at all is reported once and
    * then ignored rather than refusing `init`.
@@ -86,9 +92,19 @@ 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` while they are being collected — a visitor who is not being collected
+   * records nothing, so a stricter level has no plaintext to catch there. 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.
    *
    * Requires `localStorage`. Sessions themselves are kept in a cookie unless `sessionPersistence`
    * says otherwise, but this SDK already reads one `localStorage` entry on every site — the record

packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts

@@ -196,6 +196,83 @@ 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 about an answer that repeats the settings it already holds', (done) => {
+      localStorage.setItem(setup!.storeKey, JSON.stringify({ sessionSampleRate: 42, version: 7 }))
+      const notified = watchStoredNotifications()
+
+      interceptor.withMockXhr((xhr) => {
+        xhr.complete(200, body({ version: 7, rum: { sessionSampleRate: 42 } }))
+
+        // The ordinary answer: every new session asks again and most find nothing changed. A
+        // subscriber woken by those would act on no news, once per session, for as long as the
+        // visitor stays.
+        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 }
 

packages/rum-core/src/domain/configuration/remoteConfiguration.ts

@@ -20,17 +20,23 @@ 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.
+ * 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 follows the session's rhythm: 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 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.
  *
  * Three fields the server sends are accepted and ignored, deliberately: `ttl` and
  * `refresh_on_foreground`, which describe when to ask again and are moot without a timer, and
- * `activation`, which offers to end a running session so a change applies at once. Everything here
- * is next-session, so a console that ever offers "apply immediately" would not be obeyed by this
- * build — named here so the mismatch is found by reading rather than by an operator wondering why
- * nothing happened.
+ * `activation`, which offers to end a running session so a change applies at once. This build ends
+ * a running session on its own reading of what changed rather than on the server's say-so, so a
+ * console that offers "apply immediately" as a switch would not be obeyed — named here so the
+ * mismatch is found by reading rather than by an operator wondering why nothing happened.
  *
  * 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.
@@ -112,6 +118,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
@@ -277,7 +286,13 @@ function keepConfigFresh(configuration: RumConfiguration, setup: RemoteConfigSet
       }
       if (response) {
         failedAttempts = 0
-        store(setup, response)
+        if (store(setup, response)) {
+          // Announced only once new 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, and must not
+          // be woken by an answer that changed nothing.
+          lifeCycle.notify(LifeCycleEventType.REMOTE_CONFIGURATION_STORED)
+        }
         return
       }
       if (failedAttempts < RETRY_DELAYS.length) {
@@ -387,6 +402,12 @@ function fetchRemoteConfiguration(
   }
 }
 
+/**
+ * Writes the response to storage, and answers whether it brought settings this client did not
+ * already hold. A refused or unwritable response answers `false`, and so does one that repeats the
+ * version already stored: settings only ever change under a higher number, so by that contract a
+ * repeat leaves the next draw reading what it would have read anyway.
+ */
 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 is
@@ -403,9 +424,17 @@ 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
   }
 
+  // Settings only ever change under a higher number, so a response repeating the number already
+  // stored carries nothing new — and that is the ordinary answer, since every new session refetches
+  // and most of them find the settings unchanged. It is written anyway, which costs one small
+  // `setItem` and keeps the entry in the shape this build writes, but it is not announced: a
+  // subscriber that ends the running session must hear about changes only, or an unchanged answer
+  // arriving at every renewal would end a session per renewal, forever.
+  const isNew = storedVersion === undefined || response.version > storedVersion
+
   const values: RemoteConfigValues = { version: response.version }
   if (response.enabled && response.rum) {
     // Each value is copied only when the server actually sent it. A knob nobody configured must
@@ -437,10 +466,13 @@ 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 isNew
   } catch {
     // Storage unavailable, or the origin is out of room. The previous entry stays as it is, which
     // is the same "keep what is already working" answer a failed request gets — the client goes on
-    // applying the settings it last stored, and goes on reporting their version.
+    // applying the settings it last stored, and goes on reporting their version. Reported as a
+    // failure all the same: nothing downstream may act on settings the next draw will not find.
+    return false
   }
 }
 

packages/rum-core/src/domain/lifeCycle.ts

@@ -37,6 +37,18 @@ export const enum LifeCycleEventType {
   RAW_RUM_EVENT_COLLECTED,
   RUM_EVENT_COLLECTED,
   RAW_ERROR_COLLECTED,
+
+  // FLASHCAT FORK - a remote configuration response has just changed what is in storage. Emitted
+  // only when the write actually happened and actually changed something, so a response refused as
+  // stale, one that merely repeats the settings already held, and a storage failure all stay
+  // silent: a subscriber acting on settings the next draw would have read anyway would be acting
+  // on no news at all.
+  //
+  // 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,
 }
 
 // This is a workaround for an issue occurring when the Browser SDK is included in a TypeScript
@@ -69,6 +81,7 @@ declare const LifeCycleEventTypeAsConst: {
   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
@@ -93,6 +106,7 @@ export interface LifeCycleEventMap {
     error: RawError
     customerContext?: Context
   }
+  [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void
 }
 
 export interface RawRumEventCollectedData<E extends RawRumEvent = RawRumEvent> {