diff --git a/src/__tests__/extensions/sessionrecording.js b/src/__tests__/extensions/sessionrecording.js index 83d11a5bc..7979e6d10 100644 --- a/src/__tests__/extensions/sessionrecording.js +++ b/src/__tests__/extensions/sessionrecording.js @@ -5,7 +5,8 @@ import { MUTATION_SOURCE_TYPE, SessionRecording, } from '../../extensions/sessionrecording' -import { SESSION_RECORDING_ENABLED_SERVER_SIDE } from '../../posthog-persistence' +import { PostHogPersistence, SESSION_RECORDING_ENABLED_SERVER_SIDE } from '../../posthog-persistence' +import { SessionIdManager } from '../../sessionid' // Type and source defined here designate a non-user-generated recording event const NON_USER_GENERATED_EVENT = { type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: MUTATION_SOURCE_TYPE } } @@ -19,15 +20,16 @@ describe('SessionRecording', () => { given('sessionRecording', () => new SessionRecording(given.posthog)) given('incomingSessionAndWindowId', () => ({ sessionId: 'sessionId', windowId: 'windowId' })) + given('sessionManager', () => ({ + checkAndGetSessionAndWindowId: jest.fn().mockImplementation(() => given.incomingSessionAndWindowId), + })) given('posthog', () => ({ get_property: () => given.$session_recording_enabled_server_side, get_config: jest.fn().mockImplementation((key) => given.config[key]), capture: jest.fn(), persistence: { register: jest.fn() }, _captureMetrics: { incr: jest.fn() }, - sessionManager: { - checkAndGetSessionAndWindowId: jest.fn().mockImplementation(() => given.incomingSessionAndWindowId), - }, + sessionManager: given.sessionManager, _addCaptureHook: jest.fn(), })) @@ -41,6 +43,7 @@ describe('SessionRecording', () => { recordCanvas: true, someUnregisteredProp: 'abc', }, + persistence: 'memory', })) given('$session_recording_enabled_server_side', () => true) given('disabled', () => false) @@ -338,5 +341,130 @@ describe('SessionRecording', () => { ) }) }) + + describe('with a real session id manager', () => { + const startingDate = new Date() + + beforeEach(() => { + given('sessionManager', () => new SessionIdManager(given.config, new PostHogPersistence(given.config))) + given.sessionRecording.startRecordingIfEnabled() + given.sessionRecording.startCaptureAndTrySendingQueuedSnapshots() + }) + + const emitAtDateTime = (date) => + _emit({ event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, timestamp: date.getTime() }) + + it('takes a full snapshot for the first _emit', () => { + emitAtDateTime(startingDate) + expect(window.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + }) + + it('does not take a full snapshot for the second _emit', () => { + emitAtDateTime(startingDate) + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 1 + ) + ) + expect(window.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + }) + + it('does not change session id for a second _emit', () => { + const startingSessionId = given.sessionManager._getSessionId()[1] + + emitAtDateTime(startingDate) + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 1 + ) + ) + + expect(given.sessionManager._getSessionId()[1]).toEqual(startingSessionId) + }) + + it('does not take a full snapshot for the third _emit', () => { + emitAtDateTime(startingDate) + + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 1 + ) + ) + + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 2 + ) + ) + expect(window.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + }) + + it('sends a full snapshot if the session is rotated because session has been inactive for 30 minutess', () => { + const startingSessionId = given.sessionManager._getSessionId()[1] + emitAtDateTime(startingDate) + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 1 + ) + ) + + const inactivityThresholdLater = new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 32 + ) + emitAtDateTime(inactivityThresholdLater) + + expect(given.sessionManager._getSessionId()[1]).not.toEqual(startingSessionId) + expect(window.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + }) + + it('sends a full snapshot if the session is rotated because max time has passed', () => { + const startingSessionId = given.sessionManager._getSessionId()[1] + emitAtDateTime(startingDate) + emitAtDateTime( + new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate(), + startingDate.getHours(), + startingDate.getMinutes() + 1 + ) + ) + + const moreThanADayLater = new Date( + startingDate.getFullYear(), + startingDate.getMonth(), + startingDate.getDate() + 1, + startingDate.getHours() + 1 + ) + emitAtDateTime(moreThanADayLater) + + expect(given.sessionManager._getSessionId()[1]).not.toEqual(startingSessionId) + expect(window.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + }) + }) }) }) diff --git a/src/__tests__/sessionid.js b/src/__tests__/sessionid.js index dab10ff7e..3b286305a 100644 --- a/src/__tests__/sessionid.js +++ b/src/__tests__/sessionid.js @@ -13,9 +13,9 @@ describe('Session ID manager', () => { given('persistence', () => ({ props: { [SESSION_ID]: given.storedSessionIdData }, register: jest.fn(), - disabled: given.disable_persistence, + disabled: given.disablePersistence, })) - given('disable_persistence', () => false) + given('disablePersistence', () => false) given('config', () => ({ persistence_name: 'persistance-name', @@ -23,10 +23,12 @@ describe('Session ID manager', () => { given('timestamp', () => 1603107479471) + given('now', () => given.timestamp + 1000) + beforeEach(() => { _.UUID.mockReturnValue('newUUID') sessionStore.is_supported.mockReturnValue(true) - const mockDate = new Date(1603107460000) + const mockDate = new Date(given.now) jest.spyOn(global, 'Date').mockImplementation(() => mockDate) }) @@ -36,7 +38,9 @@ describe('Session ID manager', () => { windowId: 'newUUID', sessionId: 'newUUID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'newUUID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'newUUID', given.timestamp], + }) expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') }) @@ -46,13 +50,17 @@ describe('Session ID manager', () => { windowId: 'newUUID', sessionId: 'newUUID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'newUUID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'newUUID', given.timestamp], + }) expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') }) }) describe('stored session data', () => { - given('storedSessionIdData', () => [1603107460000, 'oldSessionID']) + given('timestampOfSessionStart', () => given.now - 3600) + + given('storedSessionIdData', () => [given.now, 'oldSessionID', given.timestampOfSessionStart]) beforeEach(() => { sessionStore.parse.mockReturnValue('oldWindowID') }) @@ -62,19 +70,26 @@ describe('Session ID manager', () => { windowId: 'oldWindowID', sessionId: 'oldSessionID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'oldSessionID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'oldSessionID', given.timestampOfSessionStart], + }) }) it('reuses old ids and does not update the session timestamp if > 30m pass and readOnly is true', () => { - const old_timestamp = 1602107460000 - given('storedSessionIdData', () => [old_timestamp, 'oldSessionID']) + let thirtyMinutesAndOneSecond = 60 * 60 * 30 + 1 + const oldTimestamp = given.now - thirtyMinutesAndOneSecond + const sessionStart = oldTimestamp - 1000 + + given('storedSessionIdData', () => [oldTimestamp, 'oldSessionID', sessionStart]) given('readOnly', () => true) expect(given.subject).toEqual({ windowId: 'oldWindowID', sessionId: 'oldSessionID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [old_timestamp, 'oldSessionID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [oldTimestamp, 'oldSessionID', sessionStart], + }) }) it('generates only a new window id, and saves it when there is no previous window id set', () => { @@ -83,31 +98,83 @@ describe('Session ID manager', () => { windowId: 'newUUID', sessionId: 'oldSessionID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'oldSessionID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'oldSessionID', given.timestampOfSessionStart], + }) expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') }) it('generates a new session id and window id, and saves it when >30m since last event', () => { - const old_timestamp = 1602107460000 - given('storedSessionIdData', () => [old_timestamp, 'oldSessionID']) + const oldTimestamp = 1602107460000 + given('storedSessionIdData', () => [oldTimestamp, 'oldSessionID', given.timestampOfSessionStart]) + + expect(given.subject).toEqual({ + windowId: 'newUUID', + sessionId: 'newUUID', + }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'newUUID', given.timestamp], + }) + expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') + }) + + it('generates a new session id and window id, and saves it when >24 hours since start timestamp', () => { + const oldTimestamp = 1602107460000 + const twentyFourHours = 3600 * 24 + given('storedSessionIdData', () => [oldTimestamp, 'oldSessionID', given.timestampOfSessionStart]) + given('timestamp', () => given.timestampOfSessionStart + twentyFourHours) expect(given.subject).toEqual({ windowId: 'newUUID', sessionId: 'newUUID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'newUUID'] }) + + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'newUUID', given.timestamp], + }) + expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') + }) + + it('generates a new session id and window id, and saves it when >24 hours since start timestamp even when readonly is true', () => { + const oldTimestamp = 1602107460000 + const twentyFourHoursAndOneSecond = (3600 * 24 + 1) * 1000 + given('storedSessionIdData', () => [oldTimestamp, 'oldSessionID', given.timestampOfSessionStart]) + given('timestamp', () => given.timestampOfSessionStart + twentyFourHoursAndOneSecond) + given('readOnly', () => true) + + expect(given.subject).toEqual({ + windowId: 'newUUID', + sessionId: 'newUUID', + }) + + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'newUUID', given.timestamp], + }) expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID') }) it('uses the current time if no timestamp is provided', () => { - const old_timestamp = 1601107460000 - given('storedSessionIdData', () => [old_timestamp, 'oldSessionID']) + const oldTimestamp = 1601107460000 + given('storedSessionIdData', () => [oldTimestamp, 'oldSessionID', given.timestampOfSessionStart]) given('timestamp', () => undefined) expect(given.subject).toEqual({ windowId: 'newUUID', sessionId: 'newUUID', }) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [1603107460000, 'newUUID'] }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.now, 'newUUID', given.now], + }) + }) + + it('populates the session start timestamp for a browser with no start time stored', () => { + given('storedSessionIdData', () => [given.timestamp, 'oldSessionID']) + expect(given.subject).toEqual({ + windowId: 'oldWindowID', + sessionId: 'oldSessionID', + }) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [given.timestamp, 'oldSessionID', given.timestamp], + }) }) }) @@ -118,7 +185,7 @@ describe('Session ID manager', () => { expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newWindowId') }) it('stores and retrieves a window_id if persistance is disabled and storage is not used', () => { - given('disable_persistence', () => true) + given('disablePersistence', () => true) given.sessionIdManager._setWindowId('newWindowId') expect(given.sessionIdManager._getWindowId()).toEqual('newWindowId') expect(sessionStore.set).not.toHaveBeenCalled() @@ -133,20 +200,22 @@ describe('Session ID manager', () => { describe('session id storage', () => { it('stores and retrieves a session id and timestamp', () => { - given.sessionIdManager._setSessionId('newSessionId', 1603107460000) - expect(given.sessionIdManager._getSessionId()).toEqual([1603107460000, 'newSessionId']) - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [1603107460000, 'newSessionId'] }) + given.sessionIdManager._setSessionId('newSessionId', 1603107460000, 1603107460000) + expect(given.persistence.register).toHaveBeenCalledWith({ + [SESSION_ID]: [1603107460000, 'newSessionId', 1603107460000], + }) + expect(given.sessionIdManager._getSessionId()).toEqual([1603107460000, 'newSessionId', 1603107460000]) }) }) describe('reset session id', () => { it('clears the existing session id', () => { given.sessionIdManager.resetSessionId() - expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [null, null] }) + expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [null, null, null] }) }) it('a new session id is generated when called', () => { - given('storedSessionIdData', () => [null, null]) - expect(given.sessionIdManager._getSessionId()).toEqual([null, null]) + given('storedSessionIdData', () => [null, null, null]) + expect(given.sessionIdManager._getSessionId()).toEqual([null, null, null]) expect(given.subject).toMatchObject({ windowId: 'newUUID', sessionId: 'newUUID', diff --git a/src/sessionid.js b/src/sessionid.js index b5eaaa94a..d79f33ea8 100644 --- a/src/sessionid.js +++ b/src/sessionid.js @@ -3,6 +3,7 @@ import { sessionStore } from './storage' import { _ } from './utils' const SESSION_CHANGE_THRESHOLD = 30 * 60 * 1000 // 30 mins +const SESSION_LENGTH_LIMIT = 24 * 3600 * 1000 // 24 hours export class SessionIdManager { constructor(config, persistence) { @@ -40,25 +41,39 @@ export class SessionIdManager { // Note: 'this.persistence.register' can be disabled in the config. // In that case, this works by storing sessionId and the timestamp in memory. - _setSessionId(sessionId, timestamp) { - if (sessionId !== this._sessionId || timestamp !== this._timestamp) { - this._timestamp = timestamp + _setSessionId(sessionId, sessionActivityTimestamp, sessionStartTimestamp) { + if ( + sessionId !== this._sessionId || + sessionActivityTimestamp !== this._sessionActivityTimestamp || + sessionStartTimestamp !== this._sessionStartTimestamp + ) { + this._sessionStartTimestamp = sessionStartTimestamp + this._sessionActivityTimestamp = sessionActivityTimestamp this._sessionId = sessionId - this.persistence.register({ [SESSION_ID]: [timestamp, sessionId] }) + this.persistence.register({ + [SESSION_ID]: [sessionActivityTimestamp, sessionId, sessionStartTimestamp], + }) } } _getSessionId() { - if (this._sessionId && this._timestamp) { - return [this._timestamp, this._sessionId] + if (this._sessionId && this._sessionActivityTimestamp && this._sessionStartTimestamp) { + return [this._sessionActivityTimestamp, this._sessionId, this._sessionStartTimestamp] } - return this.persistence['props'][SESSION_ID] || [0, null] + const sessionId = this.persistence['props'][SESSION_ID] + + if (Array.isArray(sessionId) && sessionId.length === 2) { + // Storage does not yet have a session start time. Add the last activity timestamp as the start time + sessionId.push(sessionId[0]) + } + + return sessionId || [0, null, 0] } // Resets the session id by setting it to null. On the subsequent call to checkAndGetSessionAndWindowId, // new ids will be generated. resetSessionId() { - this._setSessionId(null, null) + this._setSessionId(null, null, null) } /* @@ -80,20 +95,29 @@ export class SessionIdManager { checkAndGetSessionAndWindowId(readOnly = false, timestamp = null) { timestamp = timestamp || new Date().getTime() - let [lastTimestamp, sessionId] = this._getSessionId() + let [lastTimestamp, sessionId, startTimestamp] = this._getSessionId() let windowId = this._getWindowId() - if (!sessionId || (!readOnly && Math.abs(timestamp - lastTimestamp) > SESSION_CHANGE_THRESHOLD)) { + const sessionPastMaximumLength = + startTimestamp && startTimestamp > 0 && Math.abs(timestamp - startTimestamp) > SESSION_LENGTH_LIMIT + + if ( + !sessionId || + (!readOnly && Math.abs(timestamp - lastTimestamp) > SESSION_CHANGE_THRESHOLD) || + sessionPastMaximumLength + ) { sessionId = _.UUID() windowId = _.UUID() + startTimestamp = timestamp } else if (!windowId) { windowId = _.UUID() } - const newTimestamp = lastTimestamp === 0 || !readOnly ? timestamp : lastTimestamp + const newTimestamp = lastTimestamp === 0 || !readOnly || sessionPastMaximumLength ? timestamp : lastTimestamp + const sessionStartTimestamp = startTimestamp === 0 ? new Date().getTime() : startTimestamp this._setWindowId(windowId) - this._setSessionId(sessionId, newTimestamp) + this._setSessionId(sessionId, newTimestamp, sessionStartTimestamp) return { sessionId: sessionId,