Skip to content

Commit

Permalink
fix: no custom events when idle (#1438)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Sep 30, 2024
1 parent 8f590a5 commit 8da4411
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 50 deletions.
43 changes: 5 additions & 38 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
51 changes: 41 additions & 10 deletions src/extensions/replay/sessionrecording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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',
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -407,8 +429,6 @@ export class SessionRecording {
}
})
}

logger.info(LOGGER_PREFIX + ' started')
} else {
this.stopRecording()
}
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions src/sessionid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PostHogConfig>,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 6 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down

0 comments on commit 8da4411

Please sign in to comment.