fix: resetting sampling to null should clear sampling for users (#1859)

* fix: resetting sampling to null should clear sampling for users

* simplify test

* another test

* and so that it doesn't clash with over-riding

* unregister instead of setting to null

Author: pauldambra

src/__tests__/extensions/replay/sessionrecording.test.ts

@@ -854,6 +854,47 @@ describe('SessionRecording', () => {
                 expect(emitValues.filter((v) => v === 'sampled').length).toBeGreaterThan(30)
                 expect(emitValues.filter((v) => v === 'disabled').length).toBeGreaterThan(30)
             })
+
+            it('turning sample rate to null, means sessions are no longer sampled out', () => {
+                sessionRecording.startIfEnabledOrStop()
+                // set sample rate to 0, i.e. no sessions will run
+                sessionRecording.onRemoteConfig(
+                    makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.00' } })
+                )
+                // then check that a session is sampled (i.e. storage is false not true or null)
+                expect(posthog.get_property(SESSION_RECORDING_IS_SAMPLED)).toBe(false)
+                expect(sessionRecording['status']).toBe('disabled')
+
+                // then turn sample rate to null
+                sessionRecording.onRemoteConfig(
+                    makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: null } })
+                )
+
+                // then check that a session is no longer sampled out (i.e. storage is cleared not false)
+                expect(posthog.get_property(SESSION_RECORDING_IS_SAMPLED)).toBe(undefined)
+                expect(sessionRecording['status']).toBe('active')
+            })
+
+            it('turning sample rate from null to 0, resets values as expected', () => {
+                sessionRecording.startIfEnabledOrStop()
+
+                // first turn sample rate to null
+                sessionRecording.onRemoteConfig(
+                    makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: null } })
+                )
+
+                // then check that a session is no longer sampled out (i.e. storage is cleared not false)
+                expect(posthog.get_property(SESSION_RECORDING_IS_SAMPLED)).toBe(undefined)
+                expect(sessionRecording['status']).toBe('active')
+
+                // set sample rate to 0, i.e. no sessions will run
+                sessionRecording.onRemoteConfig(
+                    makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.00' } })
+                )
+                // then check that a session is sampled (i.e. storage is false not true or null)
+                expect(posthog.get_property(SESSION_RECORDING_IS_SAMPLED)).toBe(false)
+                expect(sessionRecording['status']).toBe('disabled')
+            })
         })
 
         describe('canvas', () => {

src/extensions/replay/sessionrecording.ts

@@ -610,6 +610,10 @@ export class SessionRecording {
         }
     }
 
+    private _resetSampling() {
+        this.instance.persistence?.unregister(SESSION_RECORDING_IS_SAMPLED)
+    }
+
     private makeSamplingDecision(sessionId: string): void {
         const sessionIdChanged = this.sessionId !== sessionId
 
@@ -619,9 +623,7 @@ export class SessionRecording {
         const currentSampleRate = this.sampleRate
 
         if (!isNumber(currentSampleRate)) {
-            this.instance.persistence?.register({
-                [SESSION_RECORDING_IS_SAMPLED]: null,
-            })
+            this._resetSampling()
             return
         }
 
@@ -720,6 +722,10 @@ export class SessionRecording {
                 const receivedSampleRate = response.sessionRecording?.sampleRate
 
                 const parsedSampleRate = isNullish(receivedSampleRate) ? null : parseFloat(receivedSampleRate)
+                if (isNullish(parsedSampleRate)) {
+                    this._resetSampling()
+                }
+
                 const receivedMinimumDuration = response.sessionRecording?.minimumDurationMilliseconds
 
                 persistence.register({