From 8da441141d3161670f10c1f6ea7097861ebffcf5 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 30 Sep 2024 10:09:05 +0100 Subject: [PATCH] fix: no custom events when idle (#1438) --- .../replay/sessionrecording.test.ts | 43 ++-------------- src/extensions/replay/sessionrecording.ts | 51 +++++++++++++++---- src/sessionid.ts | 8 ++- src/types.ts | 6 +++ 4 files changed, 58 insertions(+), 50 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index f1024b9be..29592eb45 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -26,7 +26,7 @@ import { } from '../../../types' import { uuidv7 } from '../../../uuidv7' import { - RECORDING_IDLE_ACTIVITY_TIMEOUT_MS, + RECORDING_IDLE_THRESHOLD_MS, RECORDING_MAX_EVENT_SIZE, SessionRecording, } from '../../../extensions/replay/sessionrecording' @@ -136,10 +136,10 @@ const createIncrementalStyleSheetEvent = () => { }) } -const createCustomSnapshot = (event = {}, payload = {}): customEvent => ({ +const createCustomSnapshot = (event = {}, payload = {}, tag: string = 'custom'): customEvent => ({ type: EventType.Custom, data: { - tag: 'custom', + tag: tag, payload: { ...payload, }, @@ -1371,39 +1371,6 @@ describe('SessionRecording', () => { }) }) - it('buffers custom events without capturing while idle', () => { - // force idle state - sessionRecording['isIdle'] = true - // buffer is empty - expect(sessionRecording['buffer']).toEqual({ - ...EMPTY_BUFFER, - sessionId: sessionId, - windowId: 'windowId', - }) - - sessionRecording.onRRwebEmit(createCustomSnapshot({}) as eventWithTime) - - // custom event is buffered - expect(sessionRecording['buffer']).toEqual({ - data: [ - { - data: { - payload: {}, - tag: 'custom', - }, - type: 5, - }, - ], - sessionId: sessionId, - size: 47, - windowId: 'windowId', - }) - emitInactiveEvent(startingTimestamp + 100, true) - expect(posthog.capture).not.toHaveBeenCalled() - - expect(sessionRecording['flushBufferTimer']).toBeUndefined() - }) - it('does not emit buffered custom events while idle even when over buffer max size', () => { // force idle state sessionRecording['isIdle'] = true @@ -1494,8 +1461,8 @@ describe('SessionRecording', () => { it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { const firstActivityTimestamp = startingTimestamp + 100 const secondActivityTimestamp = startingTimestamp + 200 - const thirdActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000 - const fourthActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 + const thirdActivityTimestamp = startingTimestamp + RECORDING_IDLE_THRESHOLD_MS + 1000 + const fourthActivityTimestamp = startingTimestamp + RECORDING_IDLE_THRESHOLD_MS + 2000 const firstSnapshotEvent = emitActiveEvent(firstActivityTimestamp) // event was active so activity timestamp is updated diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 5d8b14008..f3218a90f 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -17,7 +17,14 @@ import { } from './sessionrecording-utils' import { PostHog } from '../../posthog-core' import { DecideResponse, FlagVariant, NetworkRecordOptions, NetworkRequest, Properties } from '../../types' -import { EventType, type eventWithTime, IncrementalSource, type listenerHandler, RecordPlugin } from '@rrweb/types' +import { + customEvent, + EventType, + type eventWithTime, + IncrementalSource, + type listenerHandler, + RecordPlugin, +} from '@rrweb/types' import { isBoolean, isFunction, isNullish, isNumber, isObject, isString, isUndefined } from '../../utils/type-utils' import { logger } from '../../utils/logger' @@ -31,7 +38,7 @@ const BASE_ENDPOINT = '/s/' const FIVE_MINUTES = 1000 * 60 * 5 const TWO_SECONDS = 2000 -export const RECORDING_IDLE_ACTIVITY_TIMEOUT_MS = FIVE_MINUTES +export const RECORDING_IDLE_THRESHOLD_MS = FIVE_MINUTES export const RECORDING_MAX_EVENT_SIZE = 1024 * 1024 * 0.9 // ~1mb (with some wiggle room) export const RECORDING_BUFFER_TIMEOUT = 2000 // 2 seconds export const SESSION_RECORDING_BATCH_KEY = 'recordings' @@ -169,7 +176,7 @@ function compressEvent(event: eventWithTime, ph: PostHog): eventWithTime | compr }, } } - } catch (e: unknown) { + } catch (e) { logger.error(LOGGER_PREFIX + ' could not compress event', e) ph.captureException((e as Error) || 'e was not an error', { attempted_event_type: event?.type || 'no event type', @@ -178,6 +185,10 @@ function compressEvent(event: eventWithTime, ph: PostHog): eventWithTime | compr return event } +function isSessionIdleEvent(e: eventWithTime): e is eventWithTime & customEvent { + return e.type === EventType.Custom && e.data.tag === 'sessionIdle' +} + export class SessionRecording { private _endpoint: string private flushBufferTimer?: any @@ -213,6 +224,10 @@ export class SessionRecording { // Util to help developers working on this feature manually override _forceAllowLocalhostNetworkCapture = false + private get sessionIdleThresholdMilliseconds(): number { + return this.instance.config.session_recording.session_idle_threshold_ms || RECORDING_IDLE_THRESHOLD_MS + } + private get rrwebRecord(): rrwebRecord | undefined { return assignableWindow?.__PosthogExtensions__?.rrweb?.record } @@ -345,6 +360,13 @@ export class SessionRecording { this.windowId = windowId this.buffer = this.clearBuffer() + + if (this.sessionIdleThresholdMilliseconds >= this.sessionManager.sessionTimeoutMs) { + logger.warn( + LOGGER_PREFIX + + ` session_idle_threshold_ms (${this.sessionIdleThresholdMilliseconds}) is greater than the session timeout (${this.sessionManager.sessionTimeoutMs}). Session will never be detected as idle` + ) + } } private _onBeforeUnload = (): void => { @@ -407,8 +429,6 @@ export class SessionRecording { } }) } - - logger.info(LOGGER_PREFIX + ' started') } else { this.stopRecording() } @@ -642,17 +662,24 @@ export class SessionRecording { if (!isUserInteraction && !this.isIdle) { // We check if the lastActivityTimestamp is old enough to go idle - if (event.timestamp - this._lastActivityTimestamp > RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) { + const timeSinceLastActivity = event.timestamp - this._lastActivityTimestamp + if (timeSinceLastActivity > this.sessionIdleThresholdMilliseconds) { + // we mark as idle right away, + // or else we get multiple idle events + // if there are lots of non-user activity events being emitted this.isIdle = true + // don't take full snapshots while idle clearInterval(this._fullSnapshotTimer) + this._tryAddCustomEvent('sessionIdle', { eventTimestamp: event.timestamp, lastActivityTimestamp: this._lastActivityTimestamp, - threshold: RECORDING_IDLE_ACTIVITY_TIMEOUT_MS, + threshold: this.sessionIdleThresholdMilliseconds, bufferLength: this.buffer.data.length, bufferSize: this.buffer.size, }) + // proactively flush the buffer in case the session is idle for a long time this._flushBuffer() } @@ -807,6 +834,11 @@ export class SessionRecording { this._tryAddCustomEvent('$posthog_config', { config: this.instance.config, }) + + logger.info(LOGGER_PREFIX + ' started', { + idleThreshold: this.sessionIdleThresholdMilliseconds, + maxIdleTime: this.sessionManager.sessionTimeoutMs, + }) } private _scheduleFullSnapshot(): void { @@ -889,12 +921,11 @@ export class SessionRecording { this._updateWindowAndSessionIds(event) // When in an idle state we keep recording, but don't capture the events, - // but we allow custom events even when idle - if (this.isIdle && event.type !== EventType.Custom) { + if (this.isIdle && !isSessionIdleEvent(event)) { return } - if (event.type === EventType.Custom && event.data.tag === 'sessionIdle') { + if (isSessionIdleEvent(event)) { // session idle events have a timestamp when rrweb sees them // which can artificially lengthen a session // we know when we detected it based on the payload and can correct the timestamp diff --git a/src/sessionid.ts b/src/sessionid.ts index 60a937179..b9667692a 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -24,8 +24,8 @@ export class SessionIdManager { private _sessionStartTimestamp: number | null private _sessionActivityTimestamp: number | null - private readonly _sessionTimeoutMs: number private _sessionIdChangedHandlers: SessionIdChangedCallback[] = [] + private readonly _sessionTimeoutMs: number constructor( config: Partial, @@ -88,6 +88,10 @@ export class SessionIdManager { this._listenToReloadWindow() } + get sessionTimeoutMs(): number { + return this._sessionTimeoutMs + } + onSessionId(callback: SessionIdChangedCallback): () => void { // KLUDGE: when running in tests the handlers array was always undefined // it's yucky but safe to set it here so that it's always definitely available @@ -219,7 +223,7 @@ export class SessionIdManager { let valuesChanged = false const noSessionId = !sessionId - const activityTimeout = !readOnly && Math.abs(timestamp - lastTimestamp) > this._sessionTimeoutMs + const activityTimeout = !readOnly && Math.abs(timestamp - lastTimestamp) > this.sessionTimeoutMs if (noSessionId || activityTimeout || sessionPastMaximumLength) { sessionId = this._sessionIdGenerator() windowId = this._windowIdGenerator() diff --git a/src/types.ts b/src/types.ts index a35993141..3cdeb3145 100644 --- a/src/types.ts +++ b/src/types.ts @@ -279,6 +279,12 @@ export interface SessionRecordingOptions { full_snapshot_interval_millis?: number // PREVIEW: whether to compress part of the events before sending them to the server, this is a preview feature and may change without notice compress_events?: boolean + /* + ADVANCED: alters the threshold before a recording considers a user has become idle. + Normally only altered alongside changes to session_idle_timeout_ms. + Default is 5 minutes. + */ + session_idle_threshold_ms?: number } export type SessionIdChangedCallback = (