From ecc0f54cbabfe86db0ba41cbbea55c4423a82ce4 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 18 Oct 2023 16:11:13 +0200 Subject: [PATCH] feat: allow sampling based on decide response --- src/__tests__/extensions/sessionrecording.ts | 158 +++++++++++++++++-- src/constants.ts | 2 + src/extensions/sessionrecording.ts | 83 ++++++++-- src/types.ts | 1 + 4 files changed, 219 insertions(+), 25 deletions(-) diff --git a/src/__tests__/extensions/sessionrecording.ts b/src/__tests__/extensions/sessionrecording.ts index f78b2d743..f566e5e3d 100644 --- a/src/__tests__/extensions/sessionrecording.ts +++ b/src/__tests__/extensions/sessionrecording.ts @@ -11,6 +11,8 @@ import { CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE, SESSION_RECORDING_ENABLED_SERVER_SIDE, SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE, + SESSION_RECORDING_SAMPLE_RATE, + SESSION_RECORDING_SAMPLING_EXCLUDED, } from '../../constants' import { SessionIdManager } from '../../sessionid' import { @@ -48,6 +50,8 @@ describe('SessionRecording', () => { let session_recording_recorder_version_server_side: 'v1' | 'v2' | undefined let session_recording_enabled_server_side: boolean let console_log_enabled_server_side: boolean + let session_recording_sample_rate: number | undefined + let session_recording_sampling_excluded: string | null | undefined let checkAndGetSessionAndWindowIdMock: Mock beforeEach(() => { @@ -59,6 +63,8 @@ describe('SessionRecording', () => { session_recording_enabled_server_side = true console_log_enabled_server_side = false session_recording_recorder_version_server_side = 'v2' + session_recording_sample_rate = undefined + session_recording_sampling_excluded = undefined config = { api_host: 'https://test.com', @@ -80,19 +86,49 @@ describe('SessionRecording', () => { posthog = { get_property: (property_key: string): Property | undefined => { - if (property_key === SESSION_RECORDING_ENABLED_SERVER_SIDE) { - return session_recording_enabled_server_side - } else if (property_key === SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE) { - return session_recording_recorder_version_server_side - } else if (property_key === CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE) { - return console_log_enabled_server_side - } else { - throw new Error('config has not been mocked for this property key: ' + property_key) + switch (property_key) { + case SESSION_RECORDING_ENABLED_SERVER_SIDE: + return session_recording_enabled_server_side + case SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE: + return session_recording_recorder_version_server_side + case CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE: + return console_log_enabled_server_side + case SESSION_RECORDING_SAMPLE_RATE: + return session_recording_sample_rate + case SESSION_RECORDING_SAMPLING_EXCLUDED: + return session_recording_sampling_excluded + default: + throw new Error('config has not been mocked for this property key: ' + property_key) } }, config: config, capture: jest.fn(), - persistence: { register: jest.fn() } as unknown as PostHogPersistence, + persistence: { + register: jest.fn().mockImplementation((props) => { + Object.entries(props).forEach(([property_key, value]) => { + switch (property_key) { + case SESSION_RECORDING_ENABLED_SERVER_SIDE: + session_recording_enabled_server_side = value + break + case SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE: + session_recording_recorder_version_server_side = <'v1' | 'v2' | undefined>value + break + case CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE: + console_log_enabled_server_side = value + break + case SESSION_RECORDING_SAMPLE_RATE: + session_recording_sample_rate = value + break + case SESSION_RECORDING_SAMPLING_EXCLUDED: + session_recording_sampling_excluded = value + break + default: + throw new Error('config has not been mocked for this property key: ' + property_key) + } + }) + }), + } as unknown as PostHogPersistence, + sessionManager: sessionManager, _addCaptureHook: jest.fn(), } as unknown as PostHog @@ -192,13 +228,13 @@ describe('SessionRecording', () => { ;(loadScript as any).mockImplementation((_path: any, callback: any) => callback()) }) - it('emit is not set to true until decide is called', () => { + it('emit is not active until decide is called', () => { sessionRecording.startRecordingIfEnabled() expect(loadScript).toHaveBeenCalled() - expect((sessionRecording as any).emit).toBe(false) + expect(sessionRecording.emit).toBe('buffering') sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/' } } as unknown as DecideResponse) - expect((sessionRecording as any).emit).toBe(true) + expect(sessionRecording.emit).toBe('active') }) it('stores true in persistence if recording is enabled from the server', () => { @@ -216,6 +252,19 @@ describe('SessionRecording', () => { }) }) + it('stores sample rate in persistence', () => { + sessionRecording.afterDecideResponse({ + sessionRecording: { endpoint: '/s/', sampleRate: 0.7 }, + } as unknown as DecideResponse) + + expect(posthog.persistence?.register).toHaveBeenCalledWith({ + [CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: undefined, + [SESSION_RECORDING_ENABLED_SERVER_SIDE]: true, + [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: undefined, + [SESSION_RECORDING_SAMPLE_RATE]: 0.7, + }) + }) + it('starts session recording, saves setting and endpoint when enabled', () => { sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/ses/' }, @@ -241,6 +290,77 @@ describe('SessionRecording', () => { ;(loadScript as any).mockImplementation((_path: any, callback: any) => callback()) }) + describe('sampling', () => { + it('does not emit to capture if the sample rate is 0', () => { + sessionRecording.startRecordingIfEnabled() + + sessionRecording.afterDecideResponse({ + sessionRecording: { endpoint: '/s/', sampleRate: 0 }, + } as unknown as DecideResponse) + + _emit(createIncrementalSnapshot({ data: { source: 1 } })) + expect(posthog.capture).not.toHaveBeenCalled() + expect(sessionRecording.emit).toBe(false) + }) + + it('stores excluded session when excluded', () => { + sessionRecording.startRecordingIfEnabled() + + sessionRecording.afterDecideResponse({ + sessionRecording: { endpoint: '/s/', sampleRate: 0 }, + } as unknown as DecideResponse) + + expect(posthog.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED)).toBe(sessionRecording['sessionId']) + }) + + it('does emit to capture if the sample rate is 1', () => { + sessionRecording.startRecordingIfEnabled() + + _emit(createIncrementalSnapshot({ data: { source: 1 } })) + expect(posthog.capture).not.toHaveBeenCalled() + + sessionRecording.afterDecideResponse({ + sessionRecording: { endpoint: '/s/', sampleRate: 1 }, + } as unknown as DecideResponse) + expect(sessionRecording.emit).toBe('sampled') + expect(posthog.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED)).toBe(null) + + // don't wait two seconds for the flush timer + sessionRecording['_flushBuffer']() + + _emit(createIncrementalSnapshot({ data: { source: 1 } })) + expect(posthog.capture).toHaveBeenCalled() + }) + + it('sets emit as expected when sample rate is 0.5', () => { + sessionRecording.startRecordingIfEnabled() + + sessionRecording.afterDecideResponse({ + sessionRecording: { endpoint: '/s/', sampleRate: 0.5 }, + } as unknown as DecideResponse) + const emitValues = [] + let lastSessionId = sessionRecording['sessionId'] + + for (let i = 0; i < 100; i++) { + // this will change the session id + checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ + sessionId: 'newSessionId' + i, + windowId: 'windowId', + })) + _emit(createIncrementalSnapshot({ data: { source: 1 } })) + + expect(sessionRecording['sessionId']).not.toBe(lastSessionId) + lastSessionId = sessionRecording['sessionId'] + + emitValues.push(sessionRecording.emit) + } + + // the random number generator won't always be exactly 0.5, but it should be close + expect(emitValues.filter((v) => v === 'sampled').length).toBeGreaterThan(40) + expect(emitValues.filter((v) => v === false).length).toBeGreaterThan(40) + }) + }) + it('calls rrweb.record with the right options', () => { console_log_enabled_server_side = false // access private method 🤯 @@ -273,7 +393,7 @@ describe('SessionRecording', () => { _emit(createIncrementalSnapshot({ data: { source: 1 } })) expect(posthog.capture).not.toHaveBeenCalled() - sessionRecording.afterDecideResponse({ endpoint: '/s/' } as unknown as DecideResponse) + sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/' } } as unknown as DecideResponse) _emit(createIncrementalSnapshot({ data: { source: 2 } })) // access private method 🤯 @@ -283,6 +403,8 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { + $emit_reason: 'active', + $sample_rate: undefined, $snapshot_bytes: 60, $snapshot_data: [ { type: 3, data: { source: 1 } }, @@ -303,7 +425,7 @@ describe('SessionRecording', () => { }) it('buffers emitted events', () => { - sessionRecording.afterDecideResponse({ endpoint: '/s/' } as unknown as DecideResponse) + sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/' } } as unknown as DecideResponse) sessionRecording.startRecordingIfEnabled() expect(loadScript).toHaveBeenCalled() @@ -320,6 +442,8 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { + $emit_reason: 'active', + $sample_rate: undefined, $session_id: 'sessionId', $window_id: 'windowId', $snapshot_bytes: 60, @@ -340,7 +464,7 @@ describe('SessionRecording', () => { }) it('flushes buffer if the size of the buffer hits the limit', () => { - sessionRecording.afterDecideResponse({ endpoint: '/s/' } as unknown as DecideResponse) + sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/' } } as unknown as DecideResponse) sessionRecording.startRecordingIfEnabled() expect(loadScript).toHaveBeenCalled() const bigData = 'a'.repeat(RECORDING_MAX_EVENT_SIZE * 0.8) @@ -360,7 +484,7 @@ describe('SessionRecording', () => { }) it('flushes buffer if the session_id changes', () => { - sessionRecording.afterDecideResponse({ endpoint: '/s/' } as unknown as DecideResponse) + sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/' } } as unknown as DecideResponse) sessionRecording.startRecordingIfEnabled() _emit(createIncrementalSnapshot()) @@ -373,6 +497,8 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { + $emit_reason: 'active', + $sample_rate: undefined, $session_id: 'otherSessionId', $window_id: 'windowId', $snapshot_data: [{ type: 3, data: { source: 1 } }], diff --git a/src/constants.ts b/src/constants.ts index 620aceb00..7e367d045 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -14,6 +14,8 @@ export const AUTOCAPTURE_DISABLED_SERVER_SIDE = '$autocapture_disabled_server_si export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side' export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side' export const SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE = '$session_recording_recorder_version_server_side' // follows rrweb versioning +export const SESSION_RECORDING_SAMPLE_RATE = '$session_recording_sample_rate' +export const SESSION_RECORDING_SAMPLING_EXCLUDED = '$session_recording_sampling_excluded' export const SESSION_ID = '$sesid' export const ENABLED_FEATURE_FLAGS = '$enabled_feature_flags' export const PERSISTENCE_EARLY_ACCESS_FEATURES = '$early_access_features' diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index e406c8411..2351f379e 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -2,6 +2,8 @@ import { CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE, SESSION_RECORDING_ENABLED_SERVER_SIDE, SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE, + SESSION_RECORDING_SAMPLE_RATE, + SESSION_RECORDING_SAMPLING_EXCLUDED, } from '../constants' import { ensureMaxMessageSize, @@ -62,6 +64,9 @@ const ACTIVE_SOURCES = [ ] export class SessionRecording { + get emit(): false | 'sampled' | 'active' | 'buffering' { + return this._emit + } get lastActivityTimestamp(): number { return this._lastActivityTimestamp } @@ -70,7 +75,13 @@ export class SessionRecording { } private instance: PostHog - private emit: boolean + /** + * Session recording starts in buffering mode while waiting for decide response + * Once the response is received it might be disabled (false), enabled (active) or sampled (sampled) + * When sampled that means a sample rate is set and the last time the session id rotated + * the sample rate determined this session should be sent to the server. + */ + private _emit: false | 'sampled' | 'active' | 'buffering' private _endpoint: string private windowId: string | null private sessionId: string | null @@ -96,7 +107,7 @@ export class SessionRecording { this.instance = instance this.captureStarted = false this.snapshots = [] - this.emit = false // Controls whether data is sent to the server or not + this._emit = 'buffering' // Controls whether data is sent to the server or not this._endpoint = BASE_ENDPOINT this.stopRrweb = undefined this.windowId = null @@ -155,15 +166,20 @@ export class SessionRecording { return recordingVersion_client_side || recordingVersion_server_side || 'v1' } + getSampleRate(): number | undefined { + return this.instance.get_property(SESSION_RECORDING_SAMPLE_RATE) + } + afterDecideResponse(response: DecideResponse) { - this.receivedDecide = true if (this.instance.persistence) { this.instance.persistence.register({ [SESSION_RECORDING_ENABLED_SERVER_SIDE]: !!response['sessionRecording'], [CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE]: response.sessionRecording?.consoleLogRecordingEnabled, [SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE]: response.sessionRecording?.recorderVersion, + [SESSION_RECORDING_SAMPLE_RATE]: response.sessionRecording?.sampleRate, }) } + if (response.sessionRecording?.endpoint) { this._endpoint = response.sessionRecording?.endpoint } @@ -171,6 +187,12 @@ export class SessionRecording { if (response.sessionRecording?.recorderVersion) { this.recorderVersion = response.sessionRecording.recorderVersion } + + this.receivedDecide = true + this._emit = this.isRecordingEnabled() ? 'active' : 'buffering' + // We call this to ensure that the first session is sampled if it should be + this._isSampled() + this.startRecordingIfEnabled() } @@ -182,7 +204,7 @@ export class SessionRecording { payload: { level, trace: [], - // Even though it is a string we stringify it as thats what rrweb expects + // Even though it is a string we stringify it as that's what rrweb expects payload: [JSON.stringify(message)], }, }, @@ -194,7 +216,6 @@ export class SessionRecording { // Only submit data after we've received a decide response to account for // changing endpoints and the feature being disabled on the server side. if (this.receivedDecide) { - this.emit = true this.snapshots.forEach((properties) => this._captureSnapshotBuffered(properties)) } this._startCapture() @@ -268,7 +289,7 @@ export class SessionRecording { if (isUserInteraction) { this._lastActivityTimestamp = event.timestamp if (this.isIdle) { - // Remove the idle state if set and trigger a full snapshot as we will have ingored previous mutations + // Remove the idle state if set and trigger a full snapshot as we will have ignored previous mutations this.isIdle = false this._tryTakeFullSnapshot() } @@ -284,6 +305,10 @@ export class SessionRecording { event.timestamp ) + if (this.sessionId !== sessionId) { + this._isSampled() + } + if ( [FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 && (this.windowId !== windowId || this.sessionId !== sessionId) @@ -377,7 +402,8 @@ export class SessionRecording { // :TRICKY: rrweb does not capture navigation within SPA-s, so hook into our $pageview events to get access to all events. // Dropping the initial event is fine (it's always captured by rrweb). this.instance._addCaptureHook((eventName) => { - // If anything could go wrong here it has the potential to block the main loop so we catch all errors. + // If anything could go wrong here it has the potential to block the main loop, + // so we catch all errors. try { if (eventName === '$pageview') { const href = this._maskUrl(window.location.href) @@ -433,9 +459,9 @@ export class SessionRecording { $window_id: this.windowId, } - if (this.emit) { + if (this._emit === 'sampled' || this._emit === 'active') { this._captureSnapshotBuffered(properties) - } else { + } else if (this._emit === 'buffering') { this.snapshots.push(properties) } } @@ -468,6 +494,10 @@ export class SessionRecording { $snapshot_data: this.buffer.data, $session_id: this.buffer.sessionId, $window_id: this.buffer.windowId, + $sample_rate: this.getSampleRate(), + // We use this to determine whether the session was sampled or not. + // it is logically impossible to get here without sampled or active as the state but 🤷️ + $emit_reason: this._emit === 'sampled' ? 'sampled' : this._emit ? 'active' : 'inactive', }) } @@ -514,4 +544,39 @@ export class SessionRecording { }, }) } + + /** + * Checks if a session should be sampled. + * a sample rate of 0.2 means only 20% of sessions will be sent to the server. + */ + private _isSampled() { + const sampleRate = this.getSampleRate() + if (sampleRate === undefined) { + return + } + + let shouldSample: boolean = false + // if the session has previously been marked as excluded by sample rate + // then we respect that setting + const excludedSession = this.instance.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED) + if (excludedSession !== this.sessionId) { + const randomNumber = Math.random() + shouldSample = randomNumber < sampleRate + } + + this._emit = shouldSample ? 'sampled' : false + + if (shouldSample) { + this.instance.persistence?.register({ + [SESSION_RECORDING_SAMPLING_EXCLUDED]: null, + }) + } else { + this.instance.persistence?.register({ + [SESSION_RECORDING_SAMPLING_EXCLUDED]: this.sessionId, + }) + logger.warn( + `Sample rate ${sampleRate} has determined that sessionId ${this.sessionId} will not be sent to the server.` + ) + } + } } diff --git a/src/types.ts b/src/types.ts index 7fd90f86e..3c1c309af 100644 --- a/src/types.ts +++ b/src/types.ts @@ -232,6 +232,7 @@ export interface DecideResponse { endpoint?: string consoleLogRecordingEnabled?: boolean recorderVersion?: 'v1' | 'v2' + sampleRate?: number } surveys?: boolean toolbarParams: ToolbarParams