From c36208334d2088c4528ab7652604f33e3dcdb026 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 19 Oct 2023 00:59:11 +0200 Subject: [PATCH] maybe nicer --- src/__tests__/extensions/sessionrecording.ts | 152 ++++++++----------- src/constants.ts | 1 - src/extensions/sessionrecording.ts | 74 ++++----- src/posthog-core.ts | 2 +- src/sessionid.ts | 60 ++++++-- src/utils.ts | 4 + 6 files changed, 139 insertions(+), 154 deletions(-) diff --git a/src/__tests__/extensions/sessionrecording.ts b/src/__tests__/extensions/sessionrecording.ts index 0e1ff6541..dac154f91 100644 --- a/src/__tests__/extensions/sessionrecording.ts +++ b/src/__tests__/extensions/sessionrecording.ts @@ -9,21 +9,17 @@ import { import { PostHogPersistence } from '../../posthog-persistence' import { CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE, + SESSION_ID, 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 { - INCREMENTAL_SNAPSHOT_EVENT_TYPE, - META_EVENT_TYPE, - MUTATION_SOURCE_TYPE, -} from '../../extensions/sessionrecording-utils' +import { INCREMENTAL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE } from '../../extensions/sessionrecording-utils' import { PostHog } from '../../posthog-core' import { DecideResponse, PostHogConfig, Property, SessionIdChangedCallback } from '../../types' -import Mock = jest.Mock import { uuidv7 } from '../../uuidv7' +import Mock = jest.Mock // Type and source defined here designate a non-user-generated recording event @@ -52,8 +48,8 @@ describe('SessionRecording', () => { 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 + let sessionIdGeneratorMock: Mock + let windowIdGeneratorMock: Mock beforeEach(() => { ;(window as any).rrwebRecord = jest.fn() @@ -66,7 +62,6 @@ describe('SessionRecording', () => { 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', @@ -79,15 +74,41 @@ describe('SessionRecording', () => { persistence: 'memory', } as unknown as PostHogConfig - checkAndGetSessionAndWindowIdMock = jest.fn() - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: sessionId, - windowId: 'windowId', - })) + sessionIdGeneratorMock = jest.fn().mockImplementation(() => sessionId) + windowIdGeneratorMock = jest.fn().mockImplementation(() => 'windowId') + + const fakePersistence: PostHogPersistence = { + props: { SESSION_ID: sessionId }, + 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_ID: + // eslint-disable-next-line no-case-declarations + const providedId = (>value)[1] + if (providedId !== null) { + sessionId = providedId + } + break + default: + throw new Error('config has not been mocked for this property key: ' + property_key) + } + }) + }), + } as unknown as PostHogPersistence - sessionManager = { - checkAndGetSessionAndWindowId: checkAndGetSessionAndWindowIdMock, - } as unknown as SessionIdManager + sessionManager = new SessionIdManager(config, fakePersistence, sessionIdGeneratorMock, windowIdGeneratorMock) posthog = { get_property: (property_key: string): Property | undefined => { @@ -100,39 +121,15 @@ describe('SessionRecording', () => { 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 + case SESSION_ID: + return sessionId default: throw new Error('config has not been mocked for this property key: ' + property_key) } }, config: config, capture: jest.fn(), - 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, + persistence: fakePersistence, sessionManager: sessionManager, _addCaptureHook: jest.fn(), @@ -314,13 +311,10 @@ describe('SessionRecording', () => { sessionRecording.afterDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: 0 }, } as unknown as DecideResponse) - expect(posthog.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED)).toStrictEqual(undefined) - _emit(createIncrementalSnapshot({ data: { source: 1 } })) - expect(posthog.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED)).toStrictEqual({ - sessionId: sessionId, - sampled: false, - }) + const { isSampled, sessionId: storedSessionId } = sessionManager.checkAndGetSessionAndWindowId(true) + expect(isSampled).toStrictEqual(false) + expect(storedSessionId).toStrictEqual(sessionId) }) it('does emit to capture if the sample rate is 1', () => { @@ -335,8 +329,8 @@ describe('SessionRecording', () => { _emit(createIncrementalSnapshot({ data: { source: 1 } })) expect(sessionRecording.emit).toBe('sampled') - expect(posthog.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED)).toStrictEqual({ - sampled: true, + expect(sessionManager.checkAndGetSessionAndWindowId(true)).toMatchObject({ + isSampled: true, sessionId: sessionId, }) @@ -357,11 +351,9 @@ describe('SessionRecording', () => { let lastSessionId = sessionRecording['sessionId'] for (let i = 0; i < 100; i++) { - // this will change the session id - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: 'newSessionId' + i, - windowId: 'windowId', - })) + // force change the session ID + sessionManager.resetSessionId() + sessionId = 'session-id-' + uuidv7() _emit(createIncrementalSnapshot({ data: { source: 1 } })) expect(sessionRecording['sessionId']).not.toBe(lastSessionId) @@ -590,14 +582,13 @@ describe('SessionRecording', () => { sessionRecording.startRecordingIfEnabled() - expect(sessionRecording.started()).toEqual(true) - expect(sessionRecording.captureStarted).toEqual(true) + expect(sessionRecording.started).toEqual(true) expect(sessionRecording.stopRrweb).not.toEqual(undefined) sessionRecording.stopRecording() expect(sessionRecording.stopRrweb).toEqual(undefined) - expect(sessionRecording.captureStarted).toEqual(false) + expect(sessionRecording.started).toEqual(false) }) it('session recording can be turned on after being turned off', () => { @@ -605,14 +596,13 @@ describe('SessionRecording', () => { sessionRecording.startRecordingIfEnabled() - expect(sessionRecording.started()).toEqual(true) - expect(sessionRecording.captureStarted).toEqual(true) + expect(sessionRecording.started).toEqual(true) expect(sessionRecording.stopRrweb).not.toEqual(undefined) sessionRecording.stopRecording() expect(sessionRecording.stopRrweb).toEqual(undefined) - expect(sessionRecording.captureStarted).toEqual(false) + expect(sessionRecording.started).toEqual(false) }) describe('console logs', () => { @@ -643,50 +633,32 @@ describe('SessionRecording', () => { }) it('sends a full snapshot if there is a new session/window id and the event is not type FullSnapshot or Meta', () => { - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: 'new-session-id', - windowId: 'new-window-id', - })) + sessionIdGeneratorMock.mockImplementation(() => 'newSessionId') + windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot()) expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalled() }) it('sends a full snapshot if there is a new window id and the event is not type FullSnapshot or Meta', () => { - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: 'old-session-id', - windowId: 'new-window-id', - })) + sessionIdGeneratorMock.mockImplementation(() => 'old-session-id') + windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot()) expect((window as any).rrwebRecord.takeFullSnapshot).toHaveBeenCalled() }) it('does not send a full snapshot if there is a new session/window id and the event is type FullSnapshot or Meta', () => { - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: 'new-session-id', - windowId: 'new-window-id', - })) + sessionIdGeneratorMock.mockImplementation(() => 'newSessionId') + windowIdGeneratorMock.mockImplementation(() => 'newWindowId') _emit(createIncrementalSnapshot({ type: META_EVENT_TYPE })) expect((window as any).rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() }) it('does not send a full snapshot if there is not a new session or window id', () => { - checkAndGetSessionAndWindowIdMock.mockImplementation(() => ({ - sessionId: 'old-session-id', - windowId: 'old-window-id', - })) + sessionIdGeneratorMock.mockImplementation(() => 'old-session-id') + windowIdGeneratorMock.mockImplementation(() => 'old-window-id') _emit(createIncrementalSnapshot()) expect((window as any).rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled() }) - - it('it calls checkAndGetSessionAndWindowId with readOnly as true if it not a user interaction', () => { - _emit(createIncrementalSnapshot({ data: { source: MUTATION_SOURCE_TYPE, adds: [{ id: 1 }] } })) - expect(checkAndGetSessionAndWindowIdMock).toHaveBeenCalledWith(true, undefined) - }) - - it('it calls checkAndGetSessionAndWindowId with readOnly as false if it is a user interaction', () => { - _emit(createIncrementalSnapshot()) - expect(checkAndGetSessionAndWindowIdMock).toHaveBeenCalledWith(false, undefined) - }) }) describe('the session id manager', () => { diff --git a/src/constants.ts b/src/constants.ts index 7e367d045..9ceb9cd25 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -15,7 +15,6 @@ export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled 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 29310345f..cf605c662 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -3,7 +3,6 @@ import { SESSION_RECORDING_ENABLED_SERVER_SIDE, SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE, SESSION_RECORDING_SAMPLE_RATE, - SESSION_RECORDING_SAMPLING_EXCLUDED, } from '../constants' import { ensureMaxMessageSize, @@ -19,7 +18,7 @@ import { PostHog } from '../posthog-core' import { DecideResponse, NetworkRequest, Properties } from '../types' import { EventType, type eventWithTime, type listenerHandler } from '@rrweb/types' import Config from '../config' -import { logger, loadScript, _timestamp, window } from '../utils' +import { logger, loadScript, _timestamp, window, _isBoolean } from '../utils' const BASE_ENDPOINT = '/s/' @@ -71,6 +70,17 @@ const ACTIVE_SOURCES = [ */ type SessionRecordingStatus = false | 'sampled' | 'active' | 'buffering' +function currySamplingChecked(rate: number) { + return () => { + const randomNumber = Math.random() + const shouldSample = randomNumber < rate + if (!shouldSample) { + logger.warn(`Sample rate ${rate} has determined that this sessionId will not be sent to the server.`) + } + return shouldSample + } +} + export class SessionRecording { get emit(): SessionRecordingStatus { return this._emit @@ -81,6 +91,9 @@ export class SessionRecording { get endpoint(): string { return this._endpoint } + get started(): boolean { + return this.captureStarted + } private instance: PostHog private _emit: SessionRecordingStatus @@ -96,8 +109,8 @@ export class SessionRecording { windowId: string | null } private mutationRateLimiter?: MutationRateLimiter + private captureStarted: boolean - captureStarted: boolean snapshots: any[] stopRrweb: listenerHandler | undefined receivedDecide: boolean @@ -138,10 +151,6 @@ export class SessionRecording { } } - started() { - return this.captureStarted - } - stopRecording() { if (this.captureStarted && this.stopRrweb) { this.stopRrweb() @@ -182,6 +191,14 @@ export class SessionRecording { }) } + if (response.sessionRecording?.sampleRate !== undefined) { + const rate = response.sessionRecording?.sampleRate + const sessionManager = this.getSessionManager() + if (sessionManager !== undefined) { + sessionManager.checkSampling = currySamplingChecked(rate) + } + } + if (response.sessionRecording?.endpoint) { this._endpoint = response.sessionRecording?.endpoint } @@ -300,7 +317,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId, isSampled } = sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) @@ -310,8 +327,8 @@ export class SessionRecording { this.windowId = windowId this.sessionId = sessionId - if (sessionIdChanged) { - this._isSampled() + if (_isBoolean(isSampled)) { + this._emit = isSampled ? 'sampled' : false } if ( @@ -449,9 +466,6 @@ export class SessionRecording { const { event, size } = ensureMaxMessageSize(truncateLargeConsoleLogs(throttledEvent)) this._updateWindowAndSessionIds(event) - // this is the earliest point that there is a session ID available, - // and we can check whether to sample this recording - this._isSampled() if (this.isIdle) { // When in an idle state we keep recording, but don't capture the events @@ -550,38 +564,4 @@ 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 - // if the session has previously been marked as excluded by sample rate - // then we respect that setting - const sessionStatus = this.instance.get_property(SESSION_RECORDING_SAMPLING_EXCLUDED) - if (sessionStatus?.sessionId !== this.sessionId) { - const randomNumber = Math.random() - shouldSample = randomNumber < sampleRate - } else { - shouldSample = sessionStatus?.sampled - } - - this._emit = shouldSample ? 'sampled' : false - - this.instance.persistence?.register({ - [SESSION_RECORDING_SAMPLING_EXCLUDED]: { sessionId: this.sessionId, sampled: shouldSample }, - }) - - if (!shouldSample) { - logger.warn( - `Sample rate ${sampleRate} has determined that sessionId ${this.sessionId} will not be sent to the server.` - ) - } - } } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 5cc9d94f7..16c464692 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1737,7 +1737,7 @@ export class PostHog { * is currently running */ sessionRecordingStarted(): boolean { - return !!this.sessionRecording?.started() + return !!this.sessionRecording?.started } /** diff --git a/src/sessionid.ts b/src/sessionid.ts index 2f4ae5203..036cdef1b 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -3,13 +3,23 @@ import { SESSION_ID } from './constants' import { sessionStore } from './storage' import { PostHogConfig, SessionIdChangedCallback } from './types' import { uuidv7 } from './uuidv7' -import { logger } from './utils' +import { logger, window } from './utils' const MAX_SESSION_IDLE_TIMEOUT = 30 * 60 // 30 mins const MIN_SESSION_IDLE_TIMEOUT = 60 // 1 mins const SESSION_LENGTH_LIMIT = 24 * 3600 * 1000 // 24 hours export class SessionIdManager { + get checkSampling(): () => boolean | null { + return this._checkSampling + } + + set checkSampling(value: () => boolean | null) { + this._checkSampling = value + } + + private _sessionIdGenerator: () => string + private _windowIdGenerator: () => string private config: Partial private persistence: PostHogPersistence private _windowId: string | null | undefined @@ -17,17 +27,31 @@ export class SessionIdManager { private _window_id_storage_key: string private _primary_window_exists_storage_key: string private _sessionStartTimestamp: number | null + + // when sampling is active this is set to true or false + // true means a sessionId can be sent to the API, false that it cannot + private _isSampled: boolean | null + private _checkSampling: () => boolean | null = () => null + private _sessionActivityTimestamp: number | null private _sessionTimeoutMs: number private _sessionIdChangedHandlers: SessionIdChangedCallback[] = [] - constructor(config: Partial, persistence: PostHogPersistence) { + constructor( + config: Partial, + persistence: PostHogPersistence, + sessionIdGenerator?: () => string, + windowIdGenerator?: () => string + ) { this.config = config this.persistence = persistence this._windowId = undefined this._sessionId = undefined this._sessionStartTimestamp = null this._sessionActivityTimestamp = null + this._isSampled = null + this._sessionIdGenerator = sessionIdGenerator || uuidv7 + this._windowIdGenerator = windowIdGenerator || uuidv7 const persistenceName = config['persistence_name'] || config['token'] let desiredTimeout = config['session_idle_timeout_seconds'] || MAX_SESSION_IDLE_TIMEOUT @@ -116,25 +140,29 @@ export class SessionIdManager { private _setSessionId( sessionId: string | null, sessionActivityTimestamp: number | null, - sessionStartTimestamp: number | null + sessionStartTimestamp: number | null, + isSampled: boolean | null ): void { if ( sessionId !== this._sessionId || sessionActivityTimestamp !== this._sessionActivityTimestamp || - sessionStartTimestamp !== this._sessionStartTimestamp + sessionStartTimestamp !== this._sessionStartTimestamp || + isSampled !== this._isSampled ) { this._sessionStartTimestamp = sessionStartTimestamp this._sessionActivityTimestamp = sessionActivityTimestamp this._sessionId = sessionId + this._isSampled = isSampled + this.persistence.register({ - [SESSION_ID]: [sessionActivityTimestamp, sessionId, sessionStartTimestamp], + [SESSION_ID]: [sessionActivityTimestamp, sessionId, sessionStartTimestamp, isSampled], }) } } - private _getSessionId(): [number, string, number] { - if (this._sessionId && this._sessionActivityTimestamp && this._sessionStartTimestamp) { - return [this._sessionActivityTimestamp, this._sessionId, this._sessionStartTimestamp] + private _getSessionId(): [number, string, number, boolean | null] { + if (this._sessionId && this._sessionActivityTimestamp && this._sessionStartTimestamp && this._isSampled) { + return [this._sessionActivityTimestamp, this._sessionId, this._sessionStartTimestamp, this._isSampled] } const sessionId = this.persistence.props[SESSION_ID] @@ -143,13 +171,13 @@ export class SessionIdManager { sessionId.push(sessionId[0]) } - return sessionId || [0, null, 0] + return sessionId || [0, null, 0, null] } // Resets the session id by setting it to null. On the subsequent call to checkAndGetSessionAndWindowId, // new ids will be generated. resetSessionId(): void { - this._setSessionId(null, null, null) + this._setSessionId(null, null, null, null) } /* @@ -186,7 +214,7 @@ export class SessionIdManager { const timestamp = _timestamp || new Date().getTime() // eslint-disable-next-line prefer-const - let [lastTimestamp, sessionId, startTimestamp] = this._getSessionId() + let [lastTimestamp, sessionId, startTimestamp, isSampled] = this._getSessionId() let windowId = this._getWindowId() const sessionPastMaximumLength = @@ -198,12 +226,13 @@ export class SessionIdManager { (!readOnly && Math.abs(timestamp - lastTimestamp) > this._sessionTimeoutMs) || sessionPastMaximumLength ) { - sessionId = uuidv7() - windowId = uuidv7() + sessionId = this._sessionIdGenerator() + windowId = this._windowIdGenerator() startTimestamp = timestamp valuesChanged = true + isSampled = this._checkSampling() } else if (!windowId) { - windowId = uuidv7() + windowId = this._windowIdGenerator() valuesChanged = true } @@ -211,7 +240,7 @@ export class SessionIdManager { const sessionStartTimestamp = startTimestamp === 0 ? new Date().getTime() : startTimestamp this._setWindowId(windowId) - this._setSessionId(sessionId, newTimestamp, sessionStartTimestamp) + this._setSessionId(sessionId, newTimestamp, sessionStartTimestamp, isSampled) if (valuesChanged) { this._sessionIdChangedHandlers.forEach((handler) => handler(sessionId, windowId)) @@ -221,6 +250,7 @@ export class SessionIdManager { sessionId, windowId, sessionStartTimestamp, + isSampled, } } } diff --git a/src/utils.ts b/src/utils.ts index 8884e0eee..21c499ad5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -212,6 +212,10 @@ export const _isNumber = function (obj: any): obj is number { return toString.call(obj) == '[object Number]' } +export const _isBoolean = function (candidate: unknown): candidate is boolean { + return typeof candidate === 'boolean' +} + export const _isValidRegex = function (str: string): boolean { try { new RegExp(str)