Skip to content

Commit

Permalink
feat: add a session length maximum time limit (#405)
Browse files Browse the repository at this point in the history 
* make session id recycle after 24 hours

* less pythony variables in test

* add the start time if the client doesn't already have it

* put session start time at end of storage so you can rollback if needed

* add tests that show that clock moving forward by 24 hours resets session id

* and a test for inactivity threshold
  • Loading branch information
@pauldambra
pauldambra authored Jun 20, 2022
1 parent 6929034 commit 8804cb8
Show file tree
Hide file tree
Showing 3 changed files with 261 additions and 40 deletions.
136 changes: 132 additions & 4 deletions src/__tests__/extensions/sessionrecording.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }
Expand All @@ -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(),
}))

Expand All @@ -41,6 +43,7 @@ describe('SessionRecording', () => {
recordCanvas: true,
someUnregisteredProp: 'abc',
},
persistence: 'memory',
}))
given('$session_recording_enabled_server_side', () => true)
given('disabled', () => false)
Expand Down Expand Up @@ -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)
})
})
})
})
117 changes: 93 additions & 24 deletions src/__tests__/sessionid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,22 @@ 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',
}))

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)
})

Expand All @@ -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')
})

Expand All @@ -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')
})
Expand All @@ -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', () => {
Expand All @@ -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],
})
})
})

Expand All @@ -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()
Expand All @@ -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',
Expand Down
Loading

0 comments on commit 8804cb8

Please sign in to comment.