Skip to content

Commit

Permalink
feat: different default and max idle period (#1558)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Nov 28, 2024
1 parent 1e8b54f commit 9de949e
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 42 deletions.
18 changes: 15 additions & 3 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ describe('SessionRecording', () => {
const postHogPersistence = new PostHogPersistence(config)
postHogPersistence.clear()

sessionManager = new SessionIdManager(config, postHogPersistence, sessionIdGeneratorMock, windowIdGeneratorMock)
sessionManager = new SessionIdManager(
{ config, persistence: postHogPersistence, register: jest.fn() } as unknown as PostHog,
sessionIdGeneratorMock,
windowIdGeneratorMock
)

// add capture hook returns an unsubscribe function
removeCaptureHookMock = jest.fn()
Expand Down Expand Up @@ -1130,7 +1134,11 @@ describe('SessionRecording', () => {
let unsubscribeCallback: () => void

beforeEach(() => {
sessionManager = new SessionIdManager(config, new PostHogPersistence(config))
sessionManager = new SessionIdManager({
config,
persistence: new PostHogPersistence(config),
register: jest.fn(),
} as unknown as PostHog)
posthog.sessionManager = sessionManager

mockCallback = jest.fn()
Expand Down Expand Up @@ -1216,7 +1224,11 @@ describe('SessionRecording', () => {

describe('with a real session id manager', () => {
beforeEach(() => {
sessionManager = new SessionIdManager(config, new PostHogPersistence(config))
sessionManager = new SessionIdManager({
config,
persistence: new PostHogPersistence(config),
register: jest.fn(),
} as unknown as PostHog)
posthog.sessionManager = sessionManager

sessionRecording.startIfEnabledOrStop()
Expand Down
47 changes: 37 additions & 10 deletions src/__tests__/sessionid.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { SessionIdManager } from '../sessionid'
import { DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS, MAX_SESSION_IDLE_TIMEOUT_SECONDS, SessionIdManager } from '../sessionid'
import { SESSION_ID } from '../constants'
import { sessionStore } from '../storage'
import { uuid7ToTimestampMs, uuidv7 } from '../uuidv7'
import { BootstrapConfig, PostHogConfig, Properties } from '../types'
import { PostHogPersistence } from '../posthog-persistence'
import { assignableWindow } from '../utils/globals'
import { PostHog } from '../posthog-core'

jest.mock('../uuidv7')
jest.mock('../storage')
Expand All @@ -13,14 +14,22 @@ describe('Session ID manager', () => {
let timestamp: number | undefined
let now: number
let timestampOfSessionStart: number
let registerMock: jest.Mock

const config: Partial<PostHogConfig> = {
persistence_name: 'persistance-name',
}

let persistence: { props: Properties } & Partial<PostHogPersistence>

const sessionIdMgr = (phPersistence: Partial<PostHogPersistence>) =>
new SessionIdManager(config, phPersistence as PostHogPersistence)
const sessionIdMgr = (phPersistence: Partial<PostHogPersistence>) => {
registerMock = jest.fn()
return new SessionIdManager({
config,
persistence: phPersistence as PostHogPersistence,
register: registerMock,
} as unknown as PostHog)
}

const originalDate = Date

Expand Down Expand Up @@ -70,7 +79,11 @@ describe('Session ID manager', () => {
const bootstrap: BootstrapConfig = {
sessionID: bootstrapSessionId,
}
const sessionIdManager = new SessionIdManager({ ...config, bootstrap }, persistence as PostHogPersistence)
const sessionIdManager = new SessionIdManager({
config: { ...config, bootstrap },
persistence: persistence as PostHogPersistence,
register: jest.fn(),
} as unknown as PostHog)

// act
const { sessionId, sessionStartTimestamp } = sessionIdManager.checkAndGetSessionAndWindowId(false, now)
Expand All @@ -79,6 +92,15 @@ describe('Session ID manager', () => {
expect(sessionId).toEqual(bootstrapSessionId)
expect(sessionStartTimestamp).toEqual(timestamp)
})

it('registers the session timeout as an event property', () => {
config.session_idle_timeout_seconds = 8 * 60 * 60
const sessionIdManager = sessionIdMgr(persistence)
sessionIdManager.checkAndGetSessionAndWindowId(undefined, timestamp)
expect(registerMock).toHaveBeenCalledWith({
$configured_session_timeout_ms: config.session_idle_timeout_seconds * 1000,
})
})
})

describe('stored session data', () => {
Expand Down Expand Up @@ -317,12 +339,13 @@ describe('Session ID manager', () => {

describe('custom session_idle_timeout_seconds', () => {
const mockSessionManager = (timeout: number | undefined) =>
new SessionIdManager(
{
new SessionIdManager({
config: {
session_idle_timeout_seconds: timeout,
},
persistence as PostHogPersistence
)
persistence: persistence as PostHogPersistence,
register: jest.fn(),
} as unknown as PostHog)

beforeEach(() => {
console.warn = jest.fn()
Expand All @@ -336,10 +359,14 @@ describe('Session ID manager', () => {
expect(console.warn).toBeCalledTimes(1)
expect(mockSessionManager(30 * 60 - 1)['_sessionTimeoutMs']).toEqual((30 * 60 - 1) * 1000)
expect(console.warn).toBeCalledTimes(1)
expect(mockSessionManager(30 * 60 + 1)['_sessionTimeoutMs']).toEqual(30 * 60 * 1000)
expect(mockSessionManager(MAX_SESSION_IDLE_TIMEOUT_SECONDS + 1)['_sessionTimeoutMs']).toEqual(
MAX_SESSION_IDLE_TIMEOUT_SECONDS * 1000
)
expect(console.warn).toBeCalledTimes(2)
// @ts-expect-error - test invalid input
expect(mockSessionManager('foobar')['_sessionTimeoutMs']).toEqual(30 * 60 * 1000)
expect(mockSessionManager('foobar')['_sessionTimeoutMs']).toEqual(
DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS * 1000
)
expect(console.warn).toBeCalledTimes(3)
})
})
Expand Down
80 changes: 72 additions & 8 deletions src/__tests__/utils/number-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,78 @@ jest.mock('../../utils/logger', () => ({
describe('number-utils', () => {
describe('clampToRange', () => {
it.each([
// [value, result, min, max, expected result, test description]
['returns max when value is not a number', null, 10, 100, 100],
['returns max when value is not a number', 'not-a-number', 10, 100, 100],
['returns max when value is greater than max', 150, 10, 100, 100],
['returns min when value is less than min', 5, 10, 100, 10],
['returns the value when it is within the range', 50, 10, 100, 50],
])('%s', (_description, value, min, max, expected) => {
const result = clampToRange(value, min, max, 'Test Label')
[
'returns max when value is not a number',
{
value: null,
min: 10,
max: 100,
expected: 100,
fallback: undefined,
},
],
[
'returns max when value is not a number',
{
value: 'not-a-number',
min: 10,
max: 100,
expected: 100,
fallback: undefined,
},
],
[
'returns max when value is greater than max',
{
value: 150,
min: 10,
max: 100,
expected: 100,
fallback: undefined,
},
],
[
'returns min when value is less than min',
{
value: 5,
min: 10,
max: 100,
expected: 10,
fallback: undefined,
},
],
[
'returns the value when it is within the range',
{
value: 50,
min: 10,
max: 100,
expected: 50,
fallback: undefined,
},
],
[
'returns the fallback value when provided is not valid',
{
value: 'invalid',
min: 10,
max: 100,
expected: 20,
fallback: 20,
},
],
[
'returns the max value when fallback is not valid',
{
value: 'invalid',
min: 10,
max: 75,
expected: 75,
fallback: '20',
},
],
])('%s', (_description, { value, min, max, expected, fallback }) => {
const result = clampToRange(value, min, max, 'Test Label', fallback)
expect(result).toBe(expected)
})

Expand Down
2 changes: 1 addition & 1 deletion src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export class PostHog {
this._retryQueue = new RetryQueue(this)
this.__request_queue = []

this.sessionManager = new SessionIdManager(this.config, this.persistence)
this.sessionManager = new SessionIdManager(this)
this.sessionPropsManager = new SessionPropsManager(this.sessionManager, this.persistence)

new TracingHeaders(this).startIfEnabledOrStop()
Expand Down
38 changes: 21 additions & 17 deletions src/sessionid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import { isArray, isNumber, isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

import { clampToRange } from './utils/number-utils'
import { PostHog } from './posthog-core'

const MAX_SESSION_IDLE_TIMEOUT = 30 * 60 // 30 minutes
const MIN_SESSION_IDLE_TIMEOUT = 60 // 1 minute
const SESSION_LENGTH_LIMIT = 24 * 3600 * 1000 // 24 hours
export const DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS = 30 * 60 // 30 minutes
export const MAX_SESSION_IDLE_TIMEOUT_SECONDS = 10 * 60 * 60 // 10 hours
const MIN_SESSION_IDLE_TIMEOUT_SECONDS = 60 // 1 minute
const SESSION_LENGTH_LIMIT_MILLISECONDS = 24 * 3600 * 1000 // 24 hours

export class SessionIdManager {
private readonly _sessionIdGenerator: () => string
Expand All @@ -29,32 +31,34 @@ export class SessionIdManager {
private _sessionIdChangedHandlers: SessionIdChangedCallback[] = []
private readonly _sessionTimeoutMs: number

constructor(
config: Partial<PostHogConfig>,
persistence: PostHogPersistence,
sessionIdGenerator?: () => string,
windowIdGenerator?: () => string
) {
this.config = config
this.persistence = persistence
constructor(instance: PostHog, sessionIdGenerator?: () => string, windowIdGenerator?: () => string) {
if (!instance.persistence) {
throw new Error('SessionIdManager requires a PostHogPersistence instance')
}

this.config = instance.config
this.persistence = instance.persistence
this._windowId = undefined
this._sessionId = undefined
this._sessionStartTimestamp = null
this._sessionActivityTimestamp = null
this._sessionIdGenerator = sessionIdGenerator || uuidv7
this._windowIdGenerator = windowIdGenerator || uuidv7

const persistenceName = config['persistence_name'] || config['token']
const persistenceName = this.config['persistence_name'] || this.config['token']

const desiredTimeout = config['session_idle_timeout_seconds'] || MAX_SESSION_IDLE_TIMEOUT
const desiredTimeout = this.config['session_idle_timeout_seconds'] || DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS
this._sessionTimeoutMs =
clampToRange(
desiredTimeout,
MIN_SESSION_IDLE_TIMEOUT,
MAX_SESSION_IDLE_TIMEOUT,
'session_idle_timeout_seconds'
MIN_SESSION_IDLE_TIMEOUT_SECONDS,
MAX_SESSION_IDLE_TIMEOUT_SECONDS,
'session_idle_timeout_seconds',
DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS
) * 1000

instance.register({ $configured_session_timeout_ms: this._sessionTimeoutMs })

this._window_id_storage_key = 'ph_' + persistenceName + '_window_id'
this._primary_window_exists_storage_key = 'ph_' + persistenceName + '_primary_window_exists'

Expand Down Expand Up @@ -218,7 +222,7 @@ export class SessionIdManager {
const sessionPastMaximumLength =
isNumber(startTimestamp) &&
startTimestamp > 0 &&
Math.abs(timestamp - startTimestamp) > SESSION_LENGTH_LIMIT
Math.abs(timestamp - startTimestamp) > SESSION_LENGTH_LIMIT_MILLISECONDS

let valuesChanged = false
const noSessionId = !sessionId
Expand Down
17 changes: 14 additions & 3 deletions src/utils/number-utils.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
import { isNumber } from './type-utils'
import { logger } from './logger'

export function clampToRange(value: unknown, min: number, max: number, label?: string): number {
/**
* Clamps a value to a range.
* @param value the value to clamp
* @param min the minimum value
* @param max the maximum value
* @param label if provided then enables logging and prefixes all logs with labels
* @param fallbackValue if provided then returns this value if the value is not a valid number
*/
export function clampToRange(value: unknown, min: number, max: number, label?: string, fallbackValue?: number): number {
if (min > max) {
logger.warn('min cannot be greater than max.')
min = max
}

if (!isNumber(value)) {
label && logger.warn(label + ' must be a number. Defaulting to max value:' + max)
return max
label &&
logger.warn(
label + ' must be a number. using max or fallback. max: ' + max + ', fallback: ' + fallbackValue
)
return clampToRange(fallbackValue || max, min, max, label)
} else if (value > max) {
label && logger.warn(label + ' cannot be greater than max: ' + max + '. Using max value instead.')
return max
Expand Down

0 comments on commit 9de949e

Please sign in to comment.