From e708f8dd82724d50dbf13000a8334c0d63844e70 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 1 Feb 2024 11:41:03 +0100 Subject: [PATCH 01/16] Keep untracked session in sessionContextHistory --- .../src/domain/session/sessionStore.spec.ts | 46 ++++++++++++++----- .../core/src/domain/session/sessionStore.ts | 11 +++-- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index a897357103..a5af5b8e51 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -135,13 +135,13 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeDefined() expectTrackedSessionToBeInStore() expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) it( 'when session not in cache, session not in store and new session not tracked, ' + - 'should store not tracked session', + 'should store not tracked session and trigger renew session', () => { setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) @@ -150,7 +150,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expectNotTrackedSessionToBeInStore() expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) @@ -163,7 +163,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBe(FIRST_ID) expectTrackedSessionToBeInStore(FIRST_ID) expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) }) it( @@ -181,13 +181,13 @@ describe('session store', () => { expect(sessionId).not.toBe(FIRST_ID) expectTrackedSessionToBeInStore(sessionId) expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) it( 'when session in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', + 'should expire session, store not tracked session and trigger renew session', () => { setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) @@ -199,13 +199,13 @@ describe('session store', () => { expect(sessionStoreManager.getSession()[PRODUCT_KEY]).toBeDefined() expectNotTrackedSessionToBeInStore() expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) it( 'when session not tracked in cache, session not in store and new session not tracked, ' + - 'should expire session and store not tracked session', + 'should expire session, store not tracked session and trigger renew session', () => { setSessionInStore(FakeTrackingType.NOT_TRACKED) setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) @@ -217,7 +217,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession()[PRODUCT_KEY]).toBeDefined() expectNotTrackedSessionToBeInStore() expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) @@ -248,13 +248,13 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBe(SECOND_ID) expectTrackedSessionToBeInStore(SECOND_ID) expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) it( 'when session in cache is different session than in store and store session is not tracked, ' + - 'should expire session and store not tracked session', + 'should expire session, store not tracked session and trigger renew', () => { setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) setupSessionStore((rawTrackingType) => ({ @@ -268,7 +268,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expectNotTrackedSessionToBeInStore() expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) } ) }) @@ -281,6 +281,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() }) it('when session not in cache and session in store, should do nothing', () => { @@ -291,6 +292,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache and session not in store, should expire session', () => { @@ -302,6 +304,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache is same session than in store, should expand session', () => { @@ -314,6 +317,7 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBe(FIRST_ID) expect(sessionStoreManager.getSession().expire).toBe(getStoreExpiration()) expect(expireSpy).not.toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache is different session than in store, should expire session', () => { @@ -326,7 +330,25 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expectTrackedSessionToBeInStore(SECOND_ID) expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).not.toHaveBeenCalled() }) + + it( + 'when session in cache is different session than in store and store session is not tracked, ' + + 'should expire session and trigger renew', + () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) + setSessionInStore(FakeTrackingType.NOT_TRACKED) + + sessionStoreManager.expandSession() + + expect(sessionStoreManager.getSession().id).toBeUndefined() + expectNotTrackedSessionToBeInStore() + expect(expireSpy).toHaveBeenCalled() + expect(renewSpy).toHaveBeenCalledTimes(1) + } + ) }) describe('regular watch', () => { diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 449cca08d3..5f6e94be85 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -66,16 +66,15 @@ export function startSessionStore( let sessionCache: SessionState = retrieveActiveSession() function expandOrRenewSession() { - let isTracked: boolean processSessionStoreOperations( { process: (sessionState) => { const synchronizedSession = synchronizeSession(sessionState) - isTracked = expandOrRenewSessionState(synchronizedSession) + expandOrRenewSessionState(synchronizedSession) return synchronizedSession }, after: (sessionState) => { - if (isTracked && !hasSessionInCache()) { + if (!hasSessionInCache()) { renewSessionInCache(sessionState) } sessionCache = sessionState @@ -116,6 +115,11 @@ export function startSessionStore( if (hasSessionInCache()) { if (isSessionInCacheOutdated(sessionState)) { expireSessionInCache() + + // Only renew if the session is not tracked, because we want tracked session to be renewed on user activity + if (sessionState[productKey] && !computeSessionState(sessionState[productKey]).isTracked) { + renewSessionInCache(sessionState) + } } else { sessionCache = sessionState } @@ -130,7 +134,6 @@ export function startSessionStore( sessionState.id = generateUUID() sessionState.created = String(dateNow()) } - return isTracked } function hasSessionInCache() { From 34305c660935cb6dcc73f402c32a753141319d90 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 1 Feb 2024 11:56:25 +0100 Subject: [PATCH 02/16] Allow to find expired session --- .../src/domain/session/sessionManager.spec.ts | 91 ++++++++++--------- .../core/src/domain/session/sessionManager.ts | 18 +++- packages/core/src/tools/valueHistory.spec.ts | 11 ++- packages/core/src/tools/valueHistory.ts | 13 ++- .../src/domain/logsSessionManager.spec.ts | 30 +++++- .../logs/src/domain/logsSessionManager.ts | 16 ++-- 6 files changed, 117 insertions(+), 62 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 6c47cf9527..ebcb1cf1cd 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -446,47 +446,56 @@ describe('startSessionManager', () => { }) describe('session history', () => { - it('should return undefined when there is no current session and no startTime', () => { - const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) - expireSessionCookie() - - expect(sessionManager.findActiveSession()).toBeUndefined() - }) - - it('should return the current session context when there is no start time', () => { - const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) - - expect(sessionManager.findActiveSession()!.id).toBeDefined() - expect(sessionManager.findActiveSession()!.trackingType).toBeDefined() - }) - - it('should return the session context corresponding to startTime', () => { - const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) - - // 0s to 10s: first session - clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) - const firstSessionId = sessionManager.findActiveSession()!.id - const firstSessionTrackingType = sessionManager.findActiveSession()!.trackingType - expireSessionCookie() - - // 10s to 20s: no session - clock.tick(10 * ONE_SECOND) - - // 20s to end: second session - document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) - clock.tick(10 * ONE_SECOND) - const secondSessionId = sessionManager.findActiveSession()!.id - const secondSessionTrackingType = sessionManager.findActiveSession()!.trackingType - - expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) - expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( - firstSessionTrackingType - ) - expect(sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime)).toBeUndefined() - expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) - expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( - secondSessionTrackingType - ) + ;['findActiveSession' as const, 'findActiveOrExpiredSession' as const].forEach((method) => { + describe(method, () => { + it('should return the current session context when there is no start time', () => { + const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) + + expect(sessionManager[method]()!.id).toBeDefined() + expect(sessionManager[method]()!.trackingType).toBeDefined() + }) + + it('should return the session context corresponding to startTime', () => { + const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) + + // 0s to 10s: first session + clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) + const firstSessionId = sessionManager[method]()!.id + const firstSessionTrackingType = sessionManager[method]()!.trackingType + expireSessionCookie() + + // 10s to end: second session + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + clock.tick(10 * ONE_SECOND) + const secondSessionId = sessionManager[method]()!.id + const secondSessionTrackingType = sessionManager[method]()!.trackingType + + expect(sessionManager[method]((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) + expect(sessionManager[method]((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe(firstSessionTrackingType) + expect(sessionManager[method]((15 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) + expect(sessionManager[method]((15 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( + secondSessionTrackingType + ) + }) + }) + }) + + describe('findActiveSession', () => { + it('should return undefined when the session is expired', () => { + const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) + expireSessionCookie() + + expect(sessionManager.findActiveSession()).toBeUndefined() + }) + }) + + describe('findActiveOrExpiredSession', () => { + it('should return the session context when the session is expired', () => { + const sessionManager = startSessionManager(configuration, FIRST_PRODUCT_KEY, () => TRACKED_SESSION_STATE) + expireSessionCookie() + + expect(sessionManager.findActiveOrExpiredSession()).toBeDefined() + }) }) }) }) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 68af45ff1f..bfcf30ca21 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -1,6 +1,6 @@ import type { Observable } from '../../tools/observable' import type { Context } from '../../tools/serialisation/context' -import { ValueHistory } from '../../tools/valueHistory' +import { AFTER_ENTRY_START, ValueHistory } from '../../tools/valueHistory' import type { RelativeTime } from '../../tools/utils/timeUtils' import { relativeNow, clocksOrigin, ONE_MINUTE } from '../../tools/utils/timeUtils' import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener' @@ -11,6 +11,8 @@ import { startSessionStore } from './sessionStore' export interface SessionManager { findActiveSession: (startTime?: RelativeTime) => SessionContext | undefined + findActiveOrExpiredSession: (startTime?: RelativeTime) => SessionContext | undefined + renewObservable: Observable expireObservable: Observable expire: () => void @@ -19,6 +21,7 @@ export interface SessionManager { export interface SessionContext extends Context { id: string trackingType: TrackingType + endTime?: RelativeTime } export const VISIBILITY_CHECK_DELAY = ONE_MINUTE @@ -37,20 +40,24 @@ export function startSessionManager( const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) stopCallbacks.push(() => sessionContextHistory.stop()) + let currentSessionContext = buildSessionContext() + sessionStore.renewObservable.subscribe(() => { - sessionContextHistory.add(buildSessionContext(), relativeNow()) + currentSessionContext = buildSessionContext() + sessionContextHistory.add(currentSessionContext, relativeNow()) }) sessionStore.expireObservable.subscribe(() => { - sessionContextHistory.closeActive(relativeNow()) + currentSessionContext.endTime = relativeNow() + sessionContextHistory.closeActive(currentSessionContext.endTime) }) sessionStore.expandOrRenewSession() - sessionContextHistory.add(buildSessionContext(), clocksOrigin().relative) + sessionContextHistory.add(currentSessionContext, clocksOrigin().relative) trackActivity(configuration, () => sessionStore.expandOrRenewSession()) trackVisibility(configuration, () => sessionStore.expandSession()) - function buildSessionContext() { + function buildSessionContext(): SessionContext { return { id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, @@ -59,6 +66,7 @@ export function startSessionManager( return { findActiveSession: (startTime) => sessionContextHistory.find(startTime), + findActiveOrExpiredSession: (startTime) => sessionContextHistory.find(startTime, AFTER_ENTRY_START), renewObservable: sessionStore.renewObservable, expireObservable: sessionStore.expireObservable, expire: sessionStore.expire, diff --git a/packages/core/src/tools/valueHistory.spec.ts b/packages/core/src/tools/valueHistory.spec.ts index 02a8f9f81d..165adb8698 100644 --- a/packages/core/src/tools/valueHistory.spec.ts +++ b/packages/core/src/tools/valueHistory.spec.ts @@ -2,7 +2,7 @@ import type { Clock } from '../../test' import { mockClock } from '../../test' import type { Duration, RelativeTime } from './utils/timeUtils' import { addDuration, ONE_MINUTE } from './utils/timeUtils' -import { CLEAR_OLD_VALUES_INTERVAL, ValueHistory } from './valueHistory' +import { AFTER_ENTRY_START, CLEAR_OLD_VALUES_INTERVAL, ValueHistory } from './valueHistory' const EXPIRE_DELAY = 10 * ONE_MINUTE const MAX_ENTRIES = 5 @@ -54,6 +54,15 @@ describe('valueHistory', () => { expect(valueHistory.find(15 as RelativeTime)).toBeUndefined() }) + + describe('with AFTER_ENTRY_START predicate', () => { + it('should return the value of the closest entry regardless of the being closed', () => { + valueHistory.add('foo', 0 as RelativeTime).close(10 as RelativeTime) + valueHistory.add('bar', 20 as RelativeTime) + + expect(valueHistory.find(15 as RelativeTime, AFTER_ENTRY_START)).toEqual('foo') + }) + }) }) describe('findAll', () => { diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index 6107fa9c21..cbd2623d50 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -5,6 +5,10 @@ import { addDuration, relativeNow, ONE_MINUTE } from './utils/timeUtils' const END_OF_TIMES = Infinity as RelativeTime +export const AFTER_ENTRY_START = () => true +const DURING_ENTRY_LIFESPAN = (entry: ValueHistoryEntry, startTime: RelativeTime) => + startTime <= entry.endTime + export interface ValueHistoryEntry { startTime: RelativeTime endTime: RelativeTime @@ -60,13 +64,14 @@ export class ValueHistory { } /** - * Return the latest value that was active during `startTime`, or the currently active value - * if no `startTime` is provided. This method assumes that entries are not overlapping. + * Return the latest value matching the startTime and the predicate, + * if no `startTime` is provided, return the most recent value + * if no `predicate` is provided, return the active value */ - find(startTime: RelativeTime = END_OF_TIMES): Value | undefined { + find(startTime: RelativeTime = END_OF_TIMES, predicate = DURING_ENTRY_LIFESPAN): Value | undefined { for (const entry of this.entries) { if (entry.startTime <= startTime) { - if (startTime <= entry.endTime) { + if (predicate(entry, startTime)) { return entry.value } break diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index 47cd85ab21..929c653e9b 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -91,8 +91,8 @@ describe('logs session manager', () => { expect(getCookie(SESSION_STORE_KEY)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) }) - describe('findSession', () => { - it('should return the current session', () => { + describe('findTrackedSession', () => { + it('should return the current active session', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) expect(logsSessionManager.findTrackedSession()!.id).toBe('abcdef') @@ -101,17 +101,37 @@ describe('logs session manager', () => { it('should return undefined if the session is not tracked', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=0', DURATION) const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) - expect(logsSessionManager.findTrackedSession()).toBe(undefined) + expect(logsSessionManager.findTrackedSession()).toBeUndefined() + }) + + it('should not return the current session if it has expired by default', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) + clock.tick(10 * ONE_SECOND) + setCookie(SESSION_STORE_KEY, '', DURATION) + clock.tick(STORAGE_POLL_DELAY) + expect(logsSessionManager.findTrackedSession()).toBeUndefined() }) - it('should return undefined if the session has expired', () => { + it('should return the current session if it has expired when returnExpired = true', () => { const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) setCookie(SESSION_STORE_KEY, '', DURATION) clock.tick(STORAGE_POLL_DELAY) - expect(logsSessionManager.findTrackedSession()).toBe(undefined) + expect(logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })).toBeDefined() }) it('should return session corresponding to start time', () => { + setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', DURATION) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) + clock.tick(10 * ONE_SECOND) + setCookie(SESSION_STORE_KEY, 'id=bar&logs=1', DURATION) + // simulate a click to renew the session + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + clock.tick(STORAGE_POLL_DELAY) + expect(logsSessionManager.findTrackedSession(0 as RelativeTime)!.id).toEqual('foo') + expect(logsSessionManager.findTrackedSession()!.id).toEqual('bar') + }) + }) setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) clock.tick(10 * ONE_SECOND) diff --git a/packages/logs/src/domain/logsSessionManager.ts b/packages/logs/src/domain/logsSessionManager.ts index acbb7d0abb..6a867ae28c 100644 --- a/packages/logs/src/domain/logsSessionManager.ts +++ b/packages/logs/src/domain/logsSessionManager.ts @@ -5,7 +5,7 @@ import type { LogsConfiguration } from './configuration' export const LOGS_SESSION_KEY = 'logs' export interface LogsSessionManager { - findTrackedSession: (startTime?: RelativeTime) => LogsSession | undefined + findTrackedSession: (startTime?: RelativeTime, options?: { returnExpired: boolean }) => LogsSession | undefined expireObservable: Observable } @@ -22,14 +22,18 @@ export function startLogsSessionManager(configuration: LogsConfiguration): LogsS const sessionManager = startSessionManager(configuration, LOGS_SESSION_KEY, (rawTrackingType) => computeSessionState(configuration, rawTrackingType) ) + return { - findTrackedSession: (startTime) => { - const session = sessionManager.findActiveSession(startTime) - return session && session.trackingType === LoggerTrackingType.TRACKED - ? { + findTrackedSession: (startTime?: RelativeTime, { returnExpired } = { returnExpired: false }) => { + const session = returnExpired + ? sessionManager.findActiveOrExpiredSession(startTime) + : sessionManager.findActiveSession(startTime) + + if (session && session.trackingType === LoggerTrackingType.TRACKED) { + return { id: session.id, } - : undefined + } }, expireObservable: sessionManager.expireObservable, } From 2e162b59f919507282b94c0f9793e0e0a11b808c Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 1 Feb 2024 11:57:37 +0100 Subject: [PATCH 03/16] Send logs when a session is expired - Send logs when a session is expired - Only set session id when the session is active --- packages/logs/src/domain/assembly.spec.ts | 66 +++++++++++++------ packages/logs/src/domain/assembly.ts | 4 +- .../domain/contexts/internalContext.spec.ts | 1 + .../src/domain/logsSessionManager.spec.ts | 10 ++- .../logs/src/domain/logsSessionManager.ts | 10 +-- 5 files changed, 62 insertions(+), 29 deletions(-) diff --git a/packages/logs/src/domain/assembly.spec.ts b/packages/logs/src/domain/assembly.spec.ts index 970dd3ebc9..df4772f493 100644 --- a/packages/logs/src/domain/assembly.spec.ts +++ b/packages/logs/src/domain/assembly.spec.ts @@ -41,11 +41,12 @@ const COMMON_CONTEXT_WITH_USER: CommonContext = { describe('startLogsAssembly', () => { const sessionManager: LogsSessionManager = { - findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID } : undefined), + findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID, isActiveAt: () => sessionIsActive } : undefined), expireObservable: new Observable(), } let beforeSend: (event: LogsEvent) => void | boolean + let sessionIsActive: boolean let sessionIsTracked: boolean let lifeCycle: LifeCycle let serverLogs: Array = [] @@ -53,6 +54,7 @@ describe('startLogsAssembly', () => { beforeEach(() => { sessionIsTracked = true + sessionIsActive = true lifeCycle = new LifeCycle() lifeCycle.subscribe(LifeCycleEventType.LOG_COLLECTED, (serverRumEvent) => serverLogs.push(serverRumEvent)) const configuration = { @@ -96,31 +98,53 @@ describe('startLogsAssembly', () => { expect(serverLogs.length).toEqual(0) }) - it('should not send if session is not tracked', () => { - sessionIsTracked = false - lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { - rawLogsEvent: DEFAULT_MESSAGE, + describe('event generation condition', () => { + it('should not send if session is not tracked', () => { + sessionIsTracked = false + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(0) }) - expect(serverLogs.length).toEqual(0) - }) - it('should enable/disable the sending when the tracking type change', () => { - lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { - rawLogsEvent: DEFAULT_MESSAGE, + it('should send log with session id if session is active', () => { + sessionIsTracked = true + sessionIsActive = true + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(1) + expect(serverLogs[0].session_id).toEqual(SESSION_ID) }) - expect(serverLogs.length).toEqual(1) - sessionIsTracked = false - lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { - rawLogsEvent: DEFAULT_MESSAGE, + it('should send log without session id if session has expired', () => { + sessionIsTracked = true + sessionIsActive = false + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(1) + expect(serverLogs[0].session_id).toBeUndefined() }) - expect(serverLogs.length).toEqual(1) - sessionIsTracked = true - lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { - rawLogsEvent: DEFAULT_MESSAGE, + it('should enable/disable the sending when the tracking type change', () => { + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(1) + + sessionIsTracked = false + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(1) + + sessionIsTracked = true + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { + rawLogsEvent: DEFAULT_MESSAGE, + }) + expect(serverLogs.length).toEqual(2) }) - expect(serverLogs.length).toEqual(2) }) describe('contexts inclusion', () => { @@ -283,7 +307,7 @@ describe('startLogsAssembly', () => { describe('user management', () => { const sessionManager: LogsSessionManager = { - findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID } : undefined), + findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID, isActiveAt: () => true } : undefined), expireObservable: new Observable(), } @@ -351,7 +375,7 @@ describe('user management', () => { describe('logs limitation', () => { let clock: Clock const sessionManager: LogsSessionManager = { - findTrackedSession: () => ({ id: SESSION_ID }), + findTrackedSession: () => ({ id: SESSION_ID, isActiveAt: () => true }), expireObservable: new Observable(), } diff --git a/packages/logs/src/domain/assembly.ts b/packages/logs/src/domain/assembly.ts index b038938677..2ebaf86d97 100644 --- a/packages/logs/src/domain/assembly.ts +++ b/packages/logs/src/domain/assembly.ts @@ -25,7 +25,7 @@ export function startLogsAssembly( LifeCycleEventType.RAW_LOG_COLLECTED, ({ rawLogsEvent, messageContext = undefined, savedCommonContext = undefined }) => { const startTime = getRelativeTime(rawLogsEvent.date) - const session = sessionManager.findTrackedSession(startTime) + const session = sessionManager.findTrackedSession(startTime, { returnExpired: true }) if (!session) { return @@ -35,7 +35,7 @@ export function startLogsAssembly( const log = combine( { service: configuration.service, - session_id: session.id, + session_id: session.isActiveAt(startTime) ? session.id : undefined, // Insert user first to allow overrides from global context usr: !isEmptyObject(commonContext.user) ? commonContext.user : undefined, view: commonContext.view, diff --git a/packages/logs/src/domain/contexts/internalContext.spec.ts b/packages/logs/src/domain/contexts/internalContext.spec.ts index 8e20ecdd72..2da152dae1 100644 --- a/packages/logs/src/domain/contexts/internalContext.spec.ts +++ b/packages/logs/src/domain/contexts/internalContext.spec.ts @@ -15,6 +15,7 @@ describe('internal context', () => { const sessionManagerMock = { findTrackedSession: () => ({ id: sessionIdMock, + isActiveAt: () => true, }), expireObservable: new Observable(), } diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index 929c653e9b..e1d9774ec6 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -7,6 +7,7 @@ import { stopSessionManager, ONE_SECOND, DOM_EVENT, + relativeNow, } from '@datadog/browser-core' import type { Clock } from '@datadog/browser-core/test' import { createNewEvent, mockClock } from '@datadog/browser-core/test' @@ -132,13 +133,18 @@ describe('logs session manager', () => { expect(logsSessionManager.findTrackedSession()!.id).toEqual('bar') }) }) + + describe('isActiveAt', () => { + it('should return true when the session is active and false when the session has expired', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) clock.tick(10 * ONE_SECOND) setCookie(SESSION_STORE_KEY, '', DURATION) clock.tick(STORAGE_POLL_DELAY) - expect(logsSessionManager.findTrackedSession()).toBeUndefined() - expect(logsSessionManager.findTrackedSession(0 as RelativeTime)!.id).toBe('abcdef') + + const session = logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })! + expect(session.isActiveAt(0 as RelativeTime)).toEqual(true) + expect(session.isActiveAt((11 * ONE_SECOND) as RelativeTime)).toEqual(false) }) }) }) diff --git a/packages/logs/src/domain/logsSessionManager.ts b/packages/logs/src/domain/logsSessionManager.ts index 6a867ae28c..fb25b1740d 100644 --- a/packages/logs/src/domain/logsSessionManager.ts +++ b/packages/logs/src/domain/logsSessionManager.ts @@ -1,5 +1,5 @@ import type { RelativeTime } from '@datadog/browser-core' -import { Observable, performDraw, startSessionManager } from '@datadog/browser-core' +import { Observable, performDraw, relativeNow, startSessionManager } from '@datadog/browser-core' import type { LogsConfiguration } from './configuration' export const LOGS_SESSION_KEY = 'logs' @@ -11,6 +11,7 @@ export interface LogsSessionManager { export type LogsSession = { id?: string // session can be tracked without id + isActiveAt: (startTime?: RelativeTime) => boolean } export const enum LoggerTrackingType { @@ -31,8 +32,9 @@ export function startLogsSessionManager(configuration: LogsConfiguration): LogsS if (session && session.trackingType === LoggerTrackingType.TRACKED) { return { - id: session.id, - } + id: session.id, + isActiveAt: (startTime = relativeNow()) => (session.endTime || Infinity) > startTime, + } } }, expireObservable: sessionManager.expireObservable, @@ -41,7 +43,7 @@ export function startLogsSessionManager(configuration: LogsConfiguration): LogsS export function startLogsSessionManagerStub(configuration: LogsConfiguration): LogsSessionManager { const isTracked = computeTrackingType(configuration) === LoggerTrackingType.TRACKED - const session = isTracked ? {} : undefined + const session = isTracked ? { isActiveAt: () => true } : undefined return { findTrackedSession: () => session, expireObservable: new Observable(), From 4fc6bf61ce0989106f55892ea876e49f6c386b02 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 15 Feb 2024 16:57:12 +0100 Subject: [PATCH 04/16] Add integration tests --- packages/logs/src/boot/startLogs.spec.ts | 86 +++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index d86fc661ec..2d051e7ac2 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -9,8 +9,11 @@ import { noop, createTrackingConsentState, TrackingConsent, + setCookie, + STORAGE_POLL_DELAY, + ONE_MINUTE, } from '@datadog/browser-core' -import type { Request } from '@datadog/browser-core/test' +import type { Clock, Request } from '@datadog/browser-core/test' import { interceptRequests, stubEndpointBuilder, @@ -19,8 +22,11 @@ import { cleanupSyntheticsWorkerValues, mockSyntheticsWorkerValues, registerCleanupTask, + mockClock, + createNewEvent, } from '@datadog/browser-core/test' +import { SESSION_EXPIRATION_DELAY } from 'packages/core/src/domain/session/sessionConstants' import type { LogsConfiguration } from '../domain/configuration' import { validateAndBuildLogsConfiguration } from '../domain/configuration' import { HandlerType, Logger, StatusType } from '../domain/logger' @@ -238,4 +244,82 @@ describe('logs', () => { expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() }) }) + + // it('if new session is not tracked, renew the session so that logs sdk stop sending events', () => { + // ;({ handleLog, stop: stopLogs } = startLogs( + // initConfiguration, + // baseConfiguration, + // () => COMMON_CONTEXT, + // createTrackingConsentState(TrackingConsent.GRANTED) + // )) + // registerCleanupTask(stopLogs) + + // expect(getCookie(SESSION_STORE_KEY)).not.toBeUndefined() + // }) + + describe('session lifecycle', () => { + let clock: Clock + beforeEach(() => { + clock = mockClock() + }) + afterEach(() => { + clock.cleanup() + }) + + it('when the renewed session is not tracked, stop sending events', () => { + setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) + ;({ handleLog, stop: stopLogs } = startLogs( + initConfiguration, + baseConfiguration, + () => COMMON_CONTEXT, + createTrackingConsentState(TrackingConsent.GRANTED) + )) + registerCleanupTask(stopLogs) + + handleLog({ status: StatusType.info, message: 'message 1' }, logger) + + // renew untracked session + setCookie(SESSION_STORE_KEY, 'id=bar&logs=0', 1000) + clock.tick(STORAGE_POLL_DELAY) + + handleLog({ status: StatusType.info, message: 'message 2' }, logger) + + expect(requests.length).toEqual(1) + expect(getLoggedMessage(requests, 0)).toEqual( + jasmine.objectContaining({ + message: 'message 1', + session_id: jasmine.any(String), + }) + ) + }) + + it('when the session expires keep sending logs without session id', () => { + setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) + ;({ handleLog, stop: stopLogs } = startLogs( + initConfiguration, + baseConfiguration, + () => COMMON_CONTEXT, + createTrackingConsentState(TrackingConsent.GRANTED) + )) + registerCleanupTask(stopLogs) + + handleLog({ status: StatusType.info, message: 'message 1' }, logger) + + // expire session + setCookie(SESSION_STORE_KEY, '', ONE_MINUTE) + clock.tick(STORAGE_POLL_DELAY) + + handleLog({ status: StatusType.info, message: 'message 2' }, logger) + + const firstRequest = getLoggedMessage(requests, 0) + const secondRequest = getLoggedMessage(requests, 1) + + expect(requests.length).toEqual(2) + expect(firstRequest.message).toEqual('message 1') + expect(firstRequest.session_id).toEqual('foo') + + expect(secondRequest.message).toEqual('message 2') + expect(secondRequest.session_id).toBeUndefined() + }) + }) }) From d581d92cd05962619898c6feb80d492d89211228 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 16 Feb 2024 14:48:26 +0100 Subject: [PATCH 05/16] test --- .../core/src/domain/session/sessionManager.ts | 25 ++++++++++++++----- .../src/domain/session/sessionStore.spec.ts | 2 +- .../core/src/domain/session/sessionStore.ts | 16 ++++++------ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index b4715c3808..f7a762a237 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -9,13 +9,14 @@ import type { Configuration } from '../configuration' import type { TrackingConsentState } from '../trackingConsent' import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { startSessionStore } from './sessionStore' +import type { SessionState } from './sessionState' export interface SessionManager { findActiveSession: (startTime?: RelativeTime) => SessionContext | undefined findActiveOrExpiredSession: (startTime?: RelativeTime) => SessionContext | undefined renewObservable: Observable - expireObservable: Observable + expireObservable: Observable expire: () => void } @@ -35,6 +36,7 @@ export function startSessionManager( computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean }, trackingConsentState: TrackingConsentState ): SessionManager { + const renewOnUserActivity // TODO - Improve configuration type and remove assertion const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState) stopCallbacks.push(() => sessionStore.stop()) @@ -42,15 +44,25 @@ export function startSessionManager( const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) stopCallbacks.push(() => sessionContextHistory.stop()) - let currentSessionContext = buildSessionContext() + let currentSessionContext = buildSessionContext(sessionStore.getSession()) - sessionStore.renewObservable.subscribe(() => { - currentSessionContext = buildSessionContext() + function addSessionContext(sessionState: SessionState) { + currentSessionContext = buildSessionContext(sessionState) sessionContextHistory.add(currentSessionContext, relativeNow()) + } + + sessionStore.renewObservable.subscribe(() => { + addSessionContext(sessionStore.getSession()) }) - sessionStore.expireObservable.subscribe(() => { + + sessionStore.expireObservable.subscribe((nextSession: SessionState) => { currentSessionContext.endTime = relativeNow() sessionContextHistory.closeActive(currentSessionContext.endTime) + + // If next session is not tracked add it to the history + if (nextSession[productKey] && !computeSessionState(nextSession[productKey]).isTracked) { + addSessionContext(nextSession) + } }) // We expand/renew session unconditionally as tracking consent is always granted when the session @@ -73,7 +85,8 @@ export function startSessionManager( }) trackVisibility(configuration, () => sessionStore.expandSession()) - function buildSessionContext(): SessionContext { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + function buildSessionContext(sessionState: SessionState): SessionContext { return { id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index a5af5b8e51..d0e5f0c550 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -44,7 +44,7 @@ function resetSessionInStore() { setCookie(SESSION_STORE_KEY, '', DURATION) } -describe('session store', () => { +fdescribe('session store', () => { describe('getSessionStoreStrategyType', () => { it('should return a type cookie when cookies are available', () => { const sessionStoreStrategyType = selectSessionStoreStrategyType({ diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 5f6e94be85..71d188397e 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -16,7 +16,7 @@ export interface SessionStore { expandSession: () => void getSession: () => SessionState renewObservable: Observable - expireObservable: Observable + expireObservable: Observable expire: () => void stop: () => void } @@ -54,7 +54,7 @@ export function startSessionStore( computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean } ): SessionStore { const renewObservable = new Observable() - const expireObservable = new Observable() + const expireObservable = new Observable() const sessionStoreStrategy = sessionStoreStrategyType.type === 'Cookie' @@ -114,12 +114,12 @@ export function startSessionStore( } if (hasSessionInCache()) { if (isSessionInCacheOutdated(sessionState)) { - expireSessionInCache() + expireSessionInCache(sessionState) // Only renew if the session is not tracked, because we want tracked session to be renewed on user activity - if (sessionState[productKey] && !computeSessionState(sessionState[productKey]).isTracked) { - renewSessionInCache(sessionState) - } + // if (sessionState[productKey] && !computeSessionState(sessionState[productKey]).isTracked) { + // renewSessionInCache(sessionState) + // } } else { sessionCache = sessionState } @@ -144,9 +144,9 @@ export function startSessionStore( return sessionCache.id !== sessionState.id || sessionCache[productKey] !== sessionState[productKey] } - function expireSessionInCache() { + function expireSessionInCache(sessionState: SessionState) { sessionCache = {} - expireObservable.notify() + expireObservable.notify(sessionState) } function renewSessionInCache(sessionState: SessionState) { From bfb62bdd66ca728d3ef0b31462947adbb2517426 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Mon, 13 May 2024 16:09:43 +0200 Subject: [PATCH 06/16] Fixes --- .../core/src/domain/session/sessionManager.ts | 21 +++++++------------ .../src/domain/session/sessionStore.spec.ts | 2 +- .../core/src/domain/session/sessionStore.ts | 9 ++++++-- packages/logs/src/boot/startLogs.spec.ts | 2 -- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 91976c91ac..e500706d54 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -9,7 +9,6 @@ import type { Configuration } from '../configuration' import type { TrackingConsentState } from '../trackingConsent' import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { startSessionStore } from './sessionStore' -import type { SessionState } from './sessionState' export interface SessionManager { findActiveSession: (startTime?: RelativeTime) => SessionContext | undefined @@ -46,20 +45,17 @@ export function startSessionManager( const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) stopCallbacks.push(() => sessionContextHistory.stop()) - let currentSessionContext = buildSessionContext(sessionStore.getSession()) - - function addSessionContext(sessionState: SessionState) { - currentSessionContext = buildSessionContext(sessionState) - sessionContextHistory.add(currentSessionContext, relativeNow()) - } + let currentSessionContext = buildSessionContext() sessionStore.renewObservable.subscribe(() => { - addSessionContext(sessionStore.getSession()) + currentSessionContext = buildSessionContext() + sessionContextHistory.add(currentSessionContext, relativeNow()) renewObservable.notify() }) sessionStore.expireObservable.subscribe(() => { expireObservable.notify() - sessionContextHistory.closeActive(relativeNow()) + currentSessionContext.endTime = relativeNow() + sessionContextHistory.closeActive(currentSessionContext.endTime) }) // We expand/renew session unconditionally as tracking consent is always granted when the session @@ -83,8 +79,7 @@ export function startSessionManager( trackVisibility(configuration, () => sessionStore.expandSession()) trackResume(configuration, () => sessionStore.restartSession()) - // eslint-disable-next-line @typescript-eslint/no-unused-vars - function buildSessionContext(sessionState: SessionState): SessionContext { + function buildSessionContext(): SessionContext { return { id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, @@ -94,8 +89,8 @@ export function startSessionManager( return { findActiveSession: (startTime) => sessionContextHistory.find(startTime), findActiveOrExpiredSession: (startTime) => sessionContextHistory.find(startTime, AFTER_ENTRY_START), - renewObservable: sessionStore.renewObservable, - expireObservable: sessionStore.expireObservable, + renewObservable, + expireObservable, expire: sessionStore.expire, } } diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 07263def2f..a478f5d5d1 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -55,7 +55,7 @@ function resetSessionInStore() { setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) } -fdescribe('session store', () => { +describe('session store', () => { describe('getSessionStoreStrategyType', () => { it('should return a type cookie when cookies are available', () => { const sessionStoreStrategyType = selectSessionStoreStrategyType({ diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 58a3a7ba1d..e7a7ffe647 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -6,7 +6,12 @@ import { generateUUID } from '../../tools/utils/stringUtils' import type { InitConfiguration } from '../configuration' import { selectCookieStrategy, initCookieStrategy } from './storeStrategies/sessionInCookie' import type { SessionStoreStrategyType } from './storeStrategies/sessionStoreStrategy' -import { getExpiredSessionState, isSessionInExpiredState, isSessionInNotStartedState } from './sessionState' +import { + getExpiredSessionState, + isSessionInExpiredState, + isSessionInNotStartedState, + isSessionStarted, +} from './sessionState' import type { SessionState } from './sessionState' import { initLocalStorageStrategy, selectLocalStorageStrategy } from './storeStrategies/sessionInLocalStorage' import { processSessionStoreOperations } from './sessionStoreOperations' @@ -81,7 +86,7 @@ export function startSessionStore( return synchronizedSession }, after: (sessionState) => { - if (!hasSessionInCache()) { + if (isSessionStarted(sessionState) && !hasSessionInCache()) { renewSessionInCache(sessionState) } sessionCache = sessionState diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 25aeb443fb..74d61d699c 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -22,10 +22,8 @@ import { mockSyntheticsWorkerValues, registerCleanupTask, mockClock, - createNewEvent, } from '@datadog/browser-core/test' -import { SESSION_EXPIRATION_DELAY } from 'packages/core/src/domain/session/sessionConstants' import type { LogsConfiguration } from '../domain/configuration' import { validateAndBuildLogsConfiguration } from '../domain/configuration' import { HandlerType, Logger, StatusType } from '../domain/logger' From 00666dce754ff8288971fa699482ddb23cb74d45 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Tue, 14 May 2024 16:10:43 +0200 Subject: [PATCH 07/16] Remove condition to stop logging on untracked new session --- .../src/domain/session/sessionStore.spec.ts | 17 ------- .../core/src/domain/session/sessionStore.ts | 5 --- packages/logs/src/boot/startLogs.spec.ts | 45 ++----------------- 3 files changed, 3 insertions(+), 64 deletions(-) diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index a478f5d5d1..c60bf05833 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -387,23 +387,6 @@ describe('session store', () => { expect(expireSpy).toHaveBeenCalled() expect(renewSpy).not.toHaveBeenCalled() }) - - it( - 'when session in cache is different session than in store and store session is not tracked, ' + - 'should expire session and trigger renew', - () => { - setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) - setupSessionStore(() => ({ isTracked: false, trackingType: FakeTrackingType.NOT_TRACKED })) - setSessionInStore(FakeTrackingType.NOT_TRACKED) - - sessionStoreManager.expandSession() - - expect(sessionStoreManager.getSession().id).toBeUndefined() - expectNotTrackedSessionToBeInStore() - expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).toHaveBeenCalledTimes(1) - } - ) }) describe('regular watch', () => { diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index e7a7ffe647..6bcf730c68 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -127,11 +127,6 @@ export function startSessionStore( if (hasSessionInCache()) { if (isSessionInCacheOutdated(sessionState)) { expireSessionInCache() - - // Only renew if the session is not tracked, because we want tracked session to be renewed on user activity - if (sessionState[productKey] && !computeSessionState(sessionState[productKey]).isTracked) { - renewSessionInCache(sessionState) - } } else { sessionCache = sessionState } diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 74d61d699c..ef8c40ac78 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -241,18 +241,6 @@ describe('logs', () => { }) }) - // it('if new session is not tracked, renew the session so that logs sdk stop sending events', () => { - // ;({ handleLog, stop: stopLogs } = startLogs( - // initConfiguration, - // baseConfiguration, - // () => COMMON_CONTEXT, - // createTrackingConsentState(TrackingConsent.GRANTED) - // )) - // registerCleanupTask(stopLogs) - - // expect(getCookie(SESSION_STORE_KEY)).not.toBeUndefined() - // }) - describe('session lifecycle', () => { let clock: Clock beforeEach(() => { @@ -262,34 +250,7 @@ describe('logs', () => { clock.cleanup() }) - it('when the renewed session is not tracked, stop sending events', () => { - setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) - ;({ handleLog, stop: stopLogs } = startLogs( - initConfiguration, - baseConfiguration, - () => COMMON_CONTEXT, - createTrackingConsentState(TrackingConsent.GRANTED) - )) - registerCleanupTask(stopLogs) - - handleLog({ status: StatusType.info, message: 'message 1' }, logger) - - // renew untracked session - setCookie(SESSION_STORE_KEY, 'id=bar&logs=0', 1000) - clock.tick(STORAGE_POLL_DELAY) - - handleLog({ status: StatusType.info, message: 'message 2' }, logger) - - expect(requests.length).toEqual(1) - expect(getLoggedMessage(requests, 0)).toEqual( - jasmine.objectContaining({ - message: 'message 1', - session_id: jasmine.any(String), - }) - ) - }) - - it('when the session expires keep sending logs without session id', () => { + it('sends logs without session id when the session expires ', () => { setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) ;({ handleLog, stop: stopLogs } = startLogs( initConfiguration, @@ -302,8 +263,8 @@ describe('logs', () => { handleLog({ status: StatusType.info, message: 'message 1' }, logger) // expire session - setCookie(SESSION_STORE_KEY, '', ONE_MINUTE) - clock.tick(STORAGE_POLL_DELAY) + setCookie(SESSION_STORE_KEY, 'isExpired=1', ONE_MINUTE) + clock.tick(STORAGE_POLL_DELAY * 2) handleLog({ status: StatusType.info, message: 'message 2' }, logger) From 7cea6971f9da4b759427874e3daca7a9dbd77510 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Tue, 14 May 2024 18:00:49 +0200 Subject: [PATCH 08/16] Add sendLogsAfterSessionExpiration config --- .../domain/telemetry/telemetryEvent.types.ts | 5 ++++ packages/logs/src/boot/startLogs.spec.ts | 25 +++++++++++++++++++ packages/logs/src/domain/assembly.ts | 4 ++- .../logs/src/domain/configuration.spec.ts | 2 ++ packages/logs/src/domain/configuration.ts | 4 +++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index b389f1d7d2..8d2ffa8c45 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -353,6 +353,11 @@ export type TelemetryConfigurationEvent = CommonTelemetryProperties & { * The version of the tracer API used by the SDK. Eg. '0.1.0' */ tracer_api_version?: string + + /* + * Keeps sending logs after tracked session expiration + */ + send_logs_after_session_expiration?: boolean [k: string]: unknown } [k: string]: unknown diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index ef8c40ac78..ae3641ef09 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -278,5 +278,30 @@ describe('logs', () => { expect(secondRequest.message).toEqual('message 2') expect(secondRequest.session_id).toBeUndefined() }) + + it('does not send logs with session id when session is expired and sendLogsAfterSessionExpiration is false', () => { + setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) + ;({ handleLog, stop: stopLogs } = startLogs( + initConfiguration, + { ...baseConfiguration, sendLogsAfterSessionExpiration: false }, + () => COMMON_CONTEXT, + createTrackingConsentState(TrackingConsent.GRANTED) + )) + registerCleanupTask(stopLogs) + + handleLog({ status: StatusType.info, message: 'message 1' }, logger) + + // expire session + setCookie(SESSION_STORE_KEY, 'isExpired=1', ONE_MINUTE) + clock.tick(STORAGE_POLL_DELAY * 2) + + handleLog({ status: StatusType.info, message: 'message 2' }, logger) + + const firstRequest = getLoggedMessage(requests, 0) + + expect(requests.length).toEqual(1) + expect(firstRequest.message).toEqual('message 1') + expect(firstRequest.session_id).toEqual('foo') + }) }) }) diff --git a/packages/logs/src/domain/assembly.ts b/packages/logs/src/domain/assembly.ts index 2720267619..c75386edd2 100644 --- a/packages/logs/src/domain/assembly.ts +++ b/packages/logs/src/domain/assembly.ts @@ -25,7 +25,9 @@ export function startLogsAssembly( LifeCycleEventType.RAW_LOG_COLLECTED, ({ rawLogsEvent, messageContext = undefined, savedCommonContext = undefined, domainContext }) => { const startTime = getRelativeTime(rawLogsEvent.date) - const session = sessionManager.findTrackedSession(startTime, { returnExpired: true }) + const session = sessionManager.findTrackedSession(startTime, { + returnExpired: configuration.sendLogsAfterSessionExpiration, + }) if (!session) { return diff --git a/packages/logs/src/domain/configuration.spec.ts b/packages/logs/src/domain/configuration.spec.ts index 1701bb427b..97ebc95f4f 100644 --- a/packages/logs/src/domain/configuration.spec.ts +++ b/packages/logs/src/domain/configuration.spec.ts @@ -138,6 +138,7 @@ describe('serializeLogsConfiguration', () => { forwardConsoleLogs: 'all', forwardReports: 'all', usePciIntake: false, + sendLogsAfterSessionExpiration: false, } type MapLogsInitConfigurationKey = Key extends keyof InitConfiguration @@ -156,6 +157,7 @@ describe('serializeLogsConfiguration', () => { forward_console_logs: 'all', forward_reports: 'all', use_pci_intake: false, + send_logs_after_session_expiration: false, }) }) }) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 07271b94ce..3a0794e94e 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -19,6 +19,7 @@ export interface LogsInitConfiguration extends InitConfiguration { forwardErrorsToLogs?: boolean | undefined forwardConsoleLogs?: ConsoleApiName[] | 'all' | undefined forwardReports?: RawReportType[] | 'all' | undefined + sendLogsAfterSessionExpiration?: boolean usePciIntake?: boolean } @@ -29,6 +30,7 @@ export interface LogsConfiguration extends Configuration { forwardConsoleLogs: ConsoleApiName[] forwardReports: RawReportType[] requestErrorResponseLengthLimit: number + sendLogsAfterSessionExpiration: boolean } /** @@ -73,6 +75,7 @@ export function validateAndBuildLogsConfiguration( forwardConsoleLogs, forwardReports, requestErrorResponseLengthLimit: DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT, + sendLogsAfterSessionExpiration: initConfiguration.sendLogsAfterSessionExpiration !== false, }, baseConfiguration ) @@ -104,6 +107,7 @@ export function serializeLogsConfiguration(configuration: LogsInitConfiguration) forward_console_logs: configuration.forwardConsoleLogs, forward_reports: configuration.forwardReports, use_pci_intake: configuration.usePciIntake, + send_logs_after_session_expiration: configuration.sendLogsAfterSessionExpiration, }, baseSerializedInitConfiguration ) satisfies RawTelemetryConfiguration From 3d75e6c6168dc671897af3a61cc8ed67b1f7325c Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Wed, 15 May 2024 14:09:38 +0200 Subject: [PATCH 09/16] Update rum-events-format --- .../domain/telemetry/telemetryEvent.types.ts | 112 ++++++++++++++++-- rum-events-format | 2 +- 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index 8d2ffa8c45..f3d20cf44b 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -353,9 +353,8 @@ export type TelemetryConfigurationEvent = CommonTelemetryProperties & { * The version of the tracer API used by the SDK. Eg. '0.1.0' */ tracer_api_version?: string - - /* - * Keeps sending logs after tracked session expiration + /** + * Whether logs are sent after the session expiration */ send_logs_after_session_expiration?: boolean [k: string]: unknown @@ -403,16 +402,70 @@ export type TelemetryCommonFeaturesUsage = feature: 'stop-session' [k: string]: unknown } + | { + /** + * startView API + */ + feature: 'start-view' + [k: string]: unknown + } + | { + /** + * addAction API + */ + feature: 'add-action' + [k: string]: unknown + } + | { + /** + * addError API + */ + feature: 'add-error' + [k: string]: unknown + } + | { + /** + * setGlobalContext, setGlobalContextProperty, addAttribute APIs + */ + feature: 'set-global-context' + [k: string]: unknown + } + | { + /** + * setUser, setUserProperty, setUserInfo APIs + */ + feature: 'set-user' + [k: string]: unknown + } + | { + /** + * addFeatureFlagEvaluation API + */ + feature: 'add-feature-flag-evaluation' + [k: string]: unknown + } /** * Schema of browser specific features usage */ -export type TelemetryBrowserFeaturesUsage = { - /** - * startSessionReplayRecording API - */ - feature: 'start-session-replay-recording' - [k: string]: unknown -} +export type TelemetryBrowserFeaturesUsage = + | { + /** + * startSessionReplayRecording API + */ + feature: 'start-session-replay-recording' + /** + * Whether the recording is allowed to start even on sessions sampled out of replay + */ + is_forced?: boolean + [k: string]: unknown + } + | { + /** + * startDurationVital API + */ + feature: 'start-duration-vital' + [k: string]: unknown + } /** * Schema of common properties of Telemetry events @@ -492,5 +545,44 @@ export interface CommonTelemetryProperties { * Enabled experimental features */ readonly experimental_features?: string[] + telemetry?: { + /** + * Device properties + */ + device?: { + /** + * Architecture of the device + */ + architecture?: string + /** + * Brand of the device + */ + brand?: string + /** + * Model of the device + */ + model?: string + [k: string]: unknown + } + /** + * OS properties + */ + os?: { + /** + * Build of the OS + */ + build?: string + /** + * Name of the OS + */ + name?: string + /** + * Version of the OS + */ + version?: string + [k: string]: unknown + } + [k: string]: unknown + } [k: string]: unknown } diff --git a/rum-events-format b/rum-events-format index 402cd50670..dcd62e5856 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit 402cd50670b5452368a8016f7428fbdb81b24e98 +Subproject commit dcd62e58566b9d158c404f3588edc62c041262dd From f1c07e04b49ba13bfd14c254157573bc065c82c9 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Wed, 15 May 2024 15:03:18 +0200 Subject: [PATCH 10/16] Cleaning test --- packages/core/src/domain/session/sessionStore.spec.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index c60bf05833..27c7999293 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -335,7 +335,6 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() }) it('when session not in cache and session in store, should do nothing', () => { @@ -346,7 +345,6 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache and session not in store, should expire session', () => { @@ -359,7 +357,6 @@ describe('session store', () => { expectSessionToBeExpiredInStore() expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache is same session than in store, should expand session', () => { @@ -372,7 +369,6 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBe(FIRST_ID) expect(sessionStoreManager.getSession().expire).toBe(getStoreExpiration()) expect(expireSpy).not.toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() }) it('when session in cache is different session than in store, should expire session', () => { @@ -385,7 +381,6 @@ describe('session store', () => { expect(sessionStoreManager.getSession().id).toBeUndefined() expectTrackedSessionToBeInStore(SECOND_ID) expect(expireSpy).toHaveBeenCalled() - expect(renewSpy).not.toHaveBeenCalled() }) }) From bc167dfcd91972eda3df45fe2585bf526e881190 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Thu, 16 May 2024 13:50:51 +0200 Subject: [PATCH 11/16] Add todo comment to cleanup option in next major --- packages/logs/src/domain/configuration.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 3a0794e94e..966e006146 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -19,7 +19,8 @@ export interface LogsInitConfiguration extends InitConfiguration { forwardErrorsToLogs?: boolean | undefined forwardConsoleLogs?: ConsoleApiName[] | 'all' | undefined forwardReports?: RawReportType[] | 'all' | undefined - sendLogsAfterSessionExpiration?: boolean + // TODO next major: remove this option and make it the default behaviour + sendLogsAfterSessionExpiration?: boolean | undefined usePciIntake?: boolean } From 3782d12e3c881206d3f83e06142e31220fe5b24a Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Thu, 16 May 2024 13:51:37 +0200 Subject: [PATCH 12/16] Option should be false by default --- packages/logs/src/boot/startLogs.spec.ts | 4 ++-- packages/logs/src/domain/configuration.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index ae3641ef09..0b4e50fa8a 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -254,7 +254,7 @@ describe('logs', () => { setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) ;({ handleLog, stop: stopLogs } = startLogs( initConfiguration, - baseConfiguration, + { ...baseConfiguration, sendLogsAfterSessionExpiration: true }, () => COMMON_CONTEXT, createTrackingConsentState(TrackingConsent.GRANTED) )) @@ -283,7 +283,7 @@ describe('logs', () => { setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) ;({ handleLog, stop: stopLogs } = startLogs( initConfiguration, - { ...baseConfiguration, sendLogsAfterSessionExpiration: false }, + baseConfiguration, () => COMMON_CONTEXT, createTrackingConsentState(TrackingConsent.GRANTED) )) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 966e006146..91084fa7fa 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -76,7 +76,7 @@ export function validateAndBuildLogsConfiguration( forwardConsoleLogs, forwardReports, requestErrorResponseLengthLimit: DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT, - sendLogsAfterSessionExpiration: initConfiguration.sendLogsAfterSessionExpiration !== false, + sendLogsAfterSessionExpiration: !!initConfiguration.sendLogsAfterSessionExpiration, }, baseConfiguration ) From d3ea09d6a1a0ada45ff96d42ad9f95a2f5990b16 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Thu, 23 May 2024 15:18:20 +0200 Subject: [PATCH 13/16] Create test utils expireCookie --- .../core/src/domain/session/sessionManager.spec.ts | 4 ++-- packages/core/src/domain/session/sessionStore.spec.ts | 4 ++-- packages/core/test/cookie.ts | 7 +++++++ packages/core/test/index.ts | 1 + packages/logs/src/boot/startLogs.spec.ts | 7 +++---- packages/logs/src/domain/logsSessionManager.spec.ts | 10 +++++----- packages/rum-core/src/domain/rumSessionManager.spec.ts | 8 ++++---- 7 files changed, 24 insertions(+), 17 deletions(-) create mode 100644 packages/core/test/cookie.ts diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 7c7f25dc34..c63218b219 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -1,4 +1,4 @@ -import { createNewEvent, mockClock, restorePageVisibility, setPageVisibility } from '../../../test' +import { createNewEvent, expireCookie, mockClock, restorePageVisibility, setPageVisibility } from '../../../test' import type { Clock } from '../../../test' import { getCookie, setCookie } from '../../browser/cookie' import type { RelativeTime } from '../../tools/utils/timeUtils' @@ -38,7 +38,7 @@ describe('startSessionManager', () => { let clock: Clock function expireSessionCookie() { - setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) } diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 27c7999293..a245dac603 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -1,5 +1,5 @@ import type { Clock } from '../../../test' -import { mockClock } from '../../../test' +import { expireCookie, mockClock } from '../../../test' import { getCookie, setCookie } from '../../browser/cookie' import type { SessionStore } from './sessionStore' import { STORAGE_POLL_DELAY, startSessionStore, selectSessionStoreStrategyType } from './sessionStore' @@ -52,7 +52,7 @@ function getStoreExpiration() { } function resetSessionInStore() { - setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expireCookie() } describe('session store', () => { diff --git a/packages/core/test/cookie.ts b/packages/core/test/cookie.ts new file mode 100644 index 0000000000..a791cc3e24 --- /dev/null +++ b/packages/core/test/cookie.ts @@ -0,0 +1,7 @@ +import { setCookie } from '../src/browser/cookie' +import { SESSION_STORE_KEY } from '../src/domain/session/storeStrategies/sessionStoreStrategy' +import { ONE_MINUTE } from '../src/tools/utils/timeUtils' + +export function expireCookie() { + setCookie(SESSION_STORE_KEY, 'isExpired=1', ONE_MINUTE) +} diff --git a/packages/core/test/index.ts b/packages/core/test/index.ts index c1ad3e201b..9ef178cec5 100644 --- a/packages/core/test/index.ts +++ b/packages/core/test/index.ts @@ -1,6 +1,7 @@ export * from './browserChecks' export * from './buildEnv' export * from './collectAsyncCalls' +export * from './cookie' export * from './registerCleanupTask' export * from './requests' export * from './emulate/createNewEvent' diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 0b4e50fa8a..73dac10083 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -22,6 +22,7 @@ import { mockSyntheticsWorkerValues, registerCleanupTask, mockClock, + expireCookie, } from '@datadog/browser-core/test' import type { LogsConfiguration } from '../domain/configuration' @@ -262,8 +263,7 @@ describe('logs', () => { handleLog({ status: StatusType.info, message: 'message 1' }, logger) - // expire session - setCookie(SESSION_STORE_KEY, 'isExpired=1', ONE_MINUTE) + expireCookie() clock.tick(STORAGE_POLL_DELAY * 2) handleLog({ status: StatusType.info, message: 'message 2' }, logger) @@ -291,8 +291,7 @@ describe('logs', () => { handleLog({ status: StatusType.info, message: 'message 1' }, logger) - // expire session - setCookie(SESSION_STORE_KEY, 'isExpired=1', ONE_MINUTE) + expireCookie() clock.tick(STORAGE_POLL_DELAY * 2) handleLog({ status: StatusType.info, message: 'message 2' }, logger) diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index 456a0b1e0b..a1c57ed0a6 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -12,7 +12,7 @@ import { TrackingConsent, } from '@datadog/browser-core' import type { Clock } from '@datadog/browser-core/test' -import { createNewEvent, mockClock } from '@datadog/browser-core/test' +import { createNewEvent, expireCookie, mockClock } from '@datadog/browser-core/test' import type { LogsConfiguration } from './configuration' import { @@ -72,7 +72,7 @@ describe('logs session manager', () => { it('should renew on activity after expiration', () => { startLogsSessionManagerWithDefaults() - setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expireCookie() expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1') clock.tick(STORAGE_POLL_DELAY) @@ -99,14 +99,14 @@ describe('logs session manager', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) - setCookie(SESSION_STORE_KEY, '', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) expect(logsSessionManager.findTrackedSession()).toBeUndefined() }) it('should return the current session if it has expired when returnExpired = true', () => { const logsSessionManager = startLogsSessionManagerWithDefaults() - setCookie(SESSION_STORE_KEY, '', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) expect(logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })).toBeDefined() }) @@ -129,7 +129,7 @@ describe('logs session manager', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) const logsSessionManager = startLogsSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) - setCookie(SESSION_STORE_KEY, '', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) const session = logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })! diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 22d7d1df09..56bd043bd0 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -13,7 +13,7 @@ import { BridgeCapability, } from '@datadog/browser-core' import type { Clock } from '@datadog/browser-core/test' -import { createNewEvent, initEventBridgeStub, mockClock } from '@datadog/browser-core/test' +import { createNewEvent, expireCookie, initEventBridgeStub, mockClock } from '@datadog/browser-core/test' import type { RumConfiguration } from './configuration' import { validateAndBuildRumConfiguration } from './configuration' @@ -113,7 +113,7 @@ describe('rum session manager', () => { startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100 } }) - setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expireCookie() expect(getCookie(SESSION_STORE_KEY)).toEqual('isExpired=1') expect(expireSessionSpy).not.toHaveBeenCalled() expect(renewSessionSpy).not.toHaveBeenCalled() @@ -145,7 +145,7 @@ describe('rum session manager', () => { it('should return undefined if the session has expired', () => { const rumSessionManager = startRumSessionManagerWithDefaults() - setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) expect(rumSessionManager.findTrackedSession()).toBe(undefined) }) @@ -154,7 +154,7 @@ describe('rum session manager', () => { setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION) const rumSessionManager = startRumSessionManagerWithDefaults() clock.tick(10 * ONE_SECOND) - setCookie(SESSION_STORE_KEY, '', DURATION) + expireCookie() clock.tick(STORAGE_POLL_DELAY) expect(rumSessionManager.findTrackedSession()).toBeUndefined() expect(rumSessionManager.findTrackedSession(0 as RelativeTime)!.id).toBe('abcdef') From 7dec32e376da0993aff3cd3989854c5e23134e3e Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Thu, 23 May 2024 20:32:49 +0200 Subject: [PATCH 14/16] Introduce `returnInactive` option to findActiveSession --- .../src/domain/session/sessionManager.spec.ts | 95 +++++++++++-------- .../core/src/domain/session/sessionManager.ts | 25 ++--- packages/core/src/tools/valueHistory.spec.ts | 7 +- packages/core/src/tools/valueHistory.ts | 15 ++- packages/logs/src/domain/assembly.spec.ts | 24 ++++- packages/logs/src/domain/assembly.ts | 12 ++- .../domain/contexts/internalContext.spec.ts | 1 - .../src/domain/logsSessionManager.spec.ts | 16 +--- .../logs/src/domain/logsSessionManager.ts | 26 ++--- 9 files changed, 110 insertions(+), 111 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index c63218b219..b06b881f68 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -505,55 +505,68 @@ describe('startSessionManager', () => { }) describe('session history', () => { - ;['findActiveSession' as const, 'findActiveOrExpiredSession' as const].forEach((method) => { - describe(method, () => { - it('should return the current session context when there is no start time', () => { - const sessionManager = startSessionManagerWithDefaults() - - expect(sessionManager[method]()!.id).toBeDefined() - expect(sessionManager[method]()!.trackingType).toBeDefined() - }) - - it('should return the session context corresponding to startTime', () => { - const sessionManager = startSessionManagerWithDefaults() - - // 0s to 10s: first session - clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) - const firstSessionId = sessionManager[method]()!.id - const firstSessionTrackingType = sessionManager[method]()!.trackingType - expireSessionCookie() - - // 10s to end: second session - document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) - clock.tick(10 * ONE_SECOND) - const secondSessionId = sessionManager[method]()!.id - const secondSessionTrackingType = sessionManager[method]()!.trackingType - - expect(sessionManager[method]((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) - expect(sessionManager[method]((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe(firstSessionTrackingType) - expect(sessionManager[method]((15 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) - expect(sessionManager[method]((15 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( - secondSessionTrackingType - ) - }) - }) + it('should return undefined when there is no current session and no startTime', () => { + const sessionManager = startSessionManagerWithDefaults() + expireSessionCookie() + + expect(sessionManager.findActiveSession()).toBeUndefined() }) - describe('findActiveSession', () => { - it('should return undefined when the session is expired', () => { - const sessionManager = startSessionManagerWithDefaults() - expireSessionCookie() + it('should return the current session context when there is no start time', () => { + const sessionManager = startSessionManagerWithDefaults() - expect(sessionManager.findActiveSession()).toBeUndefined() - }) + expect(sessionManager.findActiveSession()!.id).toBeDefined() + expect(sessionManager.findActiveSession()!.trackingType).toBeDefined() }) - describe('findActiveOrExpiredSession', () => { - it('should return the session context when the session is expired', () => { + it('should return the session context corresponding to startTime', () => { + const sessionManager = startSessionManagerWithDefaults() + + // 0s to 10s: first session + clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) + const firstSessionId = sessionManager.findActiveSession()!.id + const firstSessionTrackingType = sessionManager.findActiveSession()!.trackingType + expireSessionCookie() + + // 10s to 20s: no session + clock.tick(10 * ONE_SECOND) + + // 20s to end: second session + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + clock.tick(10 * ONE_SECOND) + const secondSessionId = sessionManager.findActiveSession()!.id + const secondSessionTrackingType = sessionManager.findActiveSession()!.trackingType + + expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) + expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( + firstSessionTrackingType + ) + expect(sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime)).toBeUndefined() + expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) + expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( + secondSessionTrackingType + ) + }) + + describe('option `returnInactive` is true', () => { + it('should return the session context even when the session is expired', () => { const sessionManager = startSessionManagerWithDefaults() + + // 0s to 10s: first session + clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) + expireSessionCookie() - expect(sessionManager.findActiveOrExpiredSession()).toBeDefined() + // 10s to 20s: no session + clock.tick(10 * ONE_SECOND) + + expect( + sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: true }) + ).toBeDefined() + + expect( + sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: false }) + ).toBeUndefined() }) }) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index e500706d54..d9ac971c72 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -1,6 +1,6 @@ import { Observable } from '../../tools/observable' import type { Context } from '../../tools/serialisation/context' -import { AFTER_ENTRY_START, ValueHistory } from '../../tools/valueHistory' +import { ValueHistory } from '../../tools/valueHistory' import type { RelativeTime } from '../../tools/utils/timeUtils' import { relativeNow, clocksOrigin, ONE_MINUTE } from '../../tools/utils/timeUtils' import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener' @@ -11,9 +11,10 @@ import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { startSessionStore } from './sessionStore' export interface SessionManager { - findActiveSession: (startTime?: RelativeTime) => SessionContext | undefined - findActiveOrExpiredSession: (startTime?: RelativeTime) => SessionContext | undefined - + findActiveSession: ( + startTime?: RelativeTime, + options?: { returnInactive: boolean } + ) => SessionContext | undefined renewObservable: Observable expireObservable: Observable expire: () => void @@ -22,7 +23,6 @@ export interface SessionManager { export interface SessionContext extends Context { id: string trackingType: TrackingType - endTime?: RelativeTime } export const VISIBILITY_CHECK_DELAY = ONE_MINUTE @@ -45,23 +45,19 @@ export function startSessionManager( const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) stopCallbacks.push(() => sessionContextHistory.stop()) - let currentSessionContext = buildSessionContext() - sessionStore.renewObservable.subscribe(() => { - currentSessionContext = buildSessionContext() - sessionContextHistory.add(currentSessionContext, relativeNow()) + sessionContextHistory.add(buildSessionContext(), relativeNow()) renewObservable.notify() }) sessionStore.expireObservable.subscribe(() => { expireObservable.notify() - currentSessionContext.endTime = relativeNow() - sessionContextHistory.closeActive(currentSessionContext.endTime) + sessionContextHistory.closeActive(relativeNow()) }) // We expand/renew session unconditionally as tracking consent is always granted when the session // manager is started. sessionStore.expandOrRenewSession() - sessionContextHistory.add(currentSessionContext, clocksOrigin().relative) + sessionContextHistory.add(buildSessionContext(), clocksOrigin().relative) trackingConsentState.observable.subscribe(() => { if (trackingConsentState.isGranted()) { @@ -79,7 +75,7 @@ export function startSessionManager( trackVisibility(configuration, () => sessionStore.expandSession()) trackResume(configuration, () => sessionStore.restartSession()) - function buildSessionContext(): SessionContext { + function buildSessionContext() { return { id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, @@ -87,8 +83,7 @@ export function startSessionManager( } return { - findActiveSession: (startTime) => sessionContextHistory.find(startTime), - findActiveOrExpiredSession: (startTime) => sessionContextHistory.find(startTime, AFTER_ENTRY_START), + findActiveSession: (startTime, options) => sessionContextHistory.find(startTime, options), renewObservable, expireObservable, expire: sessionStore.expire, diff --git a/packages/core/src/tools/valueHistory.spec.ts b/packages/core/src/tools/valueHistory.spec.ts index 165adb8698..9f368b7eab 100644 --- a/packages/core/src/tools/valueHistory.spec.ts +++ b/packages/core/src/tools/valueHistory.spec.ts @@ -2,7 +2,7 @@ import type { Clock } from '../../test' import { mockClock } from '../../test' import type { Duration, RelativeTime } from './utils/timeUtils' import { addDuration, ONE_MINUTE } from './utils/timeUtils' -import { AFTER_ENTRY_START, CLEAR_OLD_VALUES_INTERVAL, ValueHistory } from './valueHistory' +import { CLEAR_OLD_VALUES_INTERVAL, ValueHistory } from './valueHistory' const EXPIRE_DELAY = 10 * ONE_MINUTE const MAX_ENTRIES = 5 @@ -55,12 +55,11 @@ describe('valueHistory', () => { expect(valueHistory.find(15 as RelativeTime)).toBeUndefined() }) - describe('with AFTER_ENTRY_START predicate', () => { + describe('with `option.returnInactive` true', () => { it('should return the value of the closest entry regardless of the being closed', () => { valueHistory.add('foo', 0 as RelativeTime).close(10 as RelativeTime) valueHistory.add('bar', 20 as RelativeTime) - - expect(valueHistory.find(15 as RelativeTime, AFTER_ENTRY_START)).toEqual('foo') + expect(valueHistory.find(15 as RelativeTime, { returnInactive: true })).toEqual('foo') }) }) }) diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index f896f5c82b..269d100f1b 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -6,10 +6,6 @@ import { addDuration, relativeNow, ONE_MINUTE } from './utils/timeUtils' const END_OF_TIMES = Infinity as RelativeTime -export const AFTER_ENTRY_START = () => true -const DURING_ENTRY_LIFESPAN = (entry: ValueHistoryEntry, startTime: RelativeTime) => - startTime <= entry.endTime - export interface ValueHistoryEntry { startTime: RelativeTime endTime: RelativeTime @@ -62,14 +58,15 @@ export class ValueHistory { } /** - * Return the latest value matching the startTime and the predicate, - * if no `startTime` is provided, return the most recent value - * if no `predicate` is provided, return the active value + * Return the latest value that was active during `startTime`, or the currently active value + * if no `startTime` is provided. This method assumes that entries are not overlapping. + * + * If `option.returnInactive` is true, returns the value at `startTime` (active or not). */ - find(startTime: RelativeTime = END_OF_TIMES, predicate = DURING_ENTRY_LIFESPAN): Value | undefined { + find(startTime: RelativeTime = END_OF_TIMES, options?: { returnInactive: boolean }): Value | undefined { for (const entry of this.entries) { if (entry.startTime <= startTime) { - if (predicate(entry, startTime)) { + if (options?.returnInactive || startTime <= entry.endTime) { return entry.value } break diff --git a/packages/logs/src/domain/assembly.spec.ts b/packages/logs/src/domain/assembly.spec.ts index df4772f493..ad320f731d 100644 --- a/packages/logs/src/domain/assembly.spec.ts +++ b/packages/logs/src/domain/assembly.spec.ts @@ -12,6 +12,7 @@ import { mockClock } from '@datadog/browser-core/test' import type { LogsEvent } from '../logsEvent.types' import type { CommonContext } from '../rawLogsEvent.types' import { startLogsAssembly } from './assembly' +import type { LogsConfiguration } from './configuration' import { validateAndBuildLogsConfiguration } from './configuration' import { Logger, StatusType } from './logger' import type { LogsSessionManager } from './logsSessionManager' @@ -41,7 +42,10 @@ const COMMON_CONTEXT_WITH_USER: CommonContext = { describe('startLogsAssembly', () => { const sessionManager: LogsSessionManager = { - findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID, isActiveAt: () => sessionIsActive } : undefined), + findTrackedSession: (_startTime, options) => + (sessionIsActive && sessionIsTracked) || options?.returnInactive + ? { id: sessionIsTracked ? SESSION_ID : undefined } + : undefined, expireObservable: new Observable(), } @@ -49,6 +53,7 @@ describe('startLogsAssembly', () => { let sessionIsActive: boolean let sessionIsTracked: boolean let lifeCycle: LifeCycle + let configuration: LogsConfiguration let serverLogs: Array = [] let mainLogger: Logger @@ -57,8 +62,8 @@ describe('startLogsAssembly', () => { sessionIsActive = true lifeCycle = new LifeCycle() lifeCycle.subscribe(LifeCycleEventType.LOG_COLLECTED, (serverRumEvent) => serverLogs.push(serverRumEvent)) - const configuration = { - ...validateAndBuildLogsConfiguration(initConfiguration)!, + configuration = { + ...validateAndBuildLogsConfiguration({ ...initConfiguration })!, beforeSend: (x: LogsEvent) => beforeSend(x), } beforeSend = noop @@ -118,8 +123,17 @@ describe('startLogsAssembly', () => { }) it('should send log without session id if session has expired', () => { + startLogsAssembly( + sessionManager, + { ...configuration, sendLogsAfterSessionExpiration: true }, + lifeCycle, + () => COMMON_CONTEXT, + noop + ) + sessionIsTracked = true sessionIsActive = false + lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { rawLogsEvent: DEFAULT_MESSAGE, }) @@ -307,7 +321,7 @@ describe('startLogsAssembly', () => { describe('user management', () => { const sessionManager: LogsSessionManager = { - findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID, isActiveAt: () => true } : undefined), + findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID } : undefined), expireObservable: new Observable(), } @@ -375,7 +389,7 @@ describe('user management', () => { describe('logs limitation', () => { let clock: Clock const sessionManager: LogsSessionManager = { - findTrackedSession: () => ({ id: SESSION_ID, isActiveAt: () => true }), + findTrackedSession: () => ({ id: SESSION_ID }), expireObservable: new Observable(), } diff --git a/packages/logs/src/domain/assembly.ts b/packages/logs/src/domain/assembly.ts index c75386edd2..cf352491a0 100644 --- a/packages/logs/src/domain/assembly.ts +++ b/packages/logs/src/domain/assembly.ts @@ -25,11 +25,13 @@ export function startLogsAssembly( LifeCycleEventType.RAW_LOG_COLLECTED, ({ rawLogsEvent, messageContext = undefined, savedCommonContext = undefined, domainContext }) => { const startTime = getRelativeTime(rawLogsEvent.date) - const session = sessionManager.findTrackedSession(startTime, { - returnExpired: configuration.sendLogsAfterSessionExpiration, - }) + const session = sessionManager.findTrackedSession(startTime) - if (!session) { + if ( + !session && + (!configuration.sendLogsAfterSessionExpiration || + !sessionManager.findTrackedSession(startTime, { returnInactive: true })) + ) { return } @@ -37,7 +39,7 @@ export function startLogsAssembly( const log = combine( { service: configuration.service, - session_id: session.isActiveAt(startTime) ? session.id : undefined, + session_id: session?.id, // Insert user first to allow overrides from global context usr: !isEmptyObject(commonContext.user) ? commonContext.user : undefined, view: commonContext.view, diff --git a/packages/logs/src/domain/contexts/internalContext.spec.ts b/packages/logs/src/domain/contexts/internalContext.spec.ts index 2da152dae1..8e20ecdd72 100644 --- a/packages/logs/src/domain/contexts/internalContext.spec.ts +++ b/packages/logs/src/domain/contexts/internalContext.spec.ts @@ -15,7 +15,6 @@ describe('internal context', () => { const sessionManagerMock = { findTrackedSession: () => ({ id: sessionIdMock, - isActiveAt: () => true, }), expireObservable: new Observable(), } diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index a1c57ed0a6..35360e5ca7 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -108,7 +108,7 @@ describe('logs session manager', () => { const logsSessionManager = startLogsSessionManagerWithDefaults() expireCookie() clock.tick(STORAGE_POLL_DELAY) - expect(logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })).toBeDefined() + expect(logsSessionManager.findTrackedSession(relativeNow(), { returnInactive: true })).toBeDefined() }) it('should return session corresponding to start time', () => { @@ -124,20 +124,6 @@ describe('logs session manager', () => { }) }) - describe('isActiveAt', () => { - it('should return true when the session is active and false when the session has expired', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&logs=1', DURATION) - const logsSessionManager = startLogsSessionManagerWithDefaults() - clock.tick(10 * ONE_SECOND) - expireCookie() - clock.tick(STORAGE_POLL_DELAY) - - const session = logsSessionManager.findTrackedSession(relativeNow(), { returnExpired: true })! - expect(session.isActiveAt(0 as RelativeTime)).toEqual(true) - expect(session.isActiveAt((11 * ONE_SECOND) as RelativeTime)).toEqual(false) - }) - }) - function startLogsSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { return startLogsSessionManager( { diff --git a/packages/logs/src/domain/logsSessionManager.ts b/packages/logs/src/domain/logsSessionManager.ts index 9b57e954fd..89a9e6b81d 100644 --- a/packages/logs/src/domain/logsSessionManager.ts +++ b/packages/logs/src/domain/logsSessionManager.ts @@ -1,17 +1,16 @@ import type { RelativeTime, TrackingConsentState } from '@datadog/browser-core' -import { Observable, performDraw, relativeNow, startSessionManager } from '@datadog/browser-core' +import { Observable, performDraw, startSessionManager } from '@datadog/browser-core' import type { LogsConfiguration } from './configuration' export const LOGS_SESSION_KEY = 'logs' export interface LogsSessionManager { - findTrackedSession: (startTime?: RelativeTime, options?: { returnExpired: boolean }) => LogsSession | undefined + findTrackedSession: (startTime?: RelativeTime, options?: { returnInactive: boolean }) => LogsSession | undefined expireObservable: Observable } export type LogsSession = { id?: string // session can be tracked without id - isActiveAt: (startTime?: RelativeTime) => boolean } export const enum LoggerTrackingType { @@ -29,19 +28,14 @@ export function startLogsSessionManager( (rawTrackingType) => computeSessionState(configuration, rawTrackingType), trackingConsentState ) - return { - findTrackedSession: (startTime?: RelativeTime, { returnExpired } = { returnExpired: false }) => { - const session = returnExpired - ? sessionManager.findActiveOrExpiredSession(startTime) - : sessionManager.findActiveSession(startTime) - - if (session && session.trackingType === LoggerTrackingType.TRACKED) { - return { - id: session.id, - isActiveAt: (startTime = relativeNow()) => (session.endTime || Infinity) > startTime, - } - } + findTrackedSession: (startTime?: RelativeTime, options = { returnInactive: false }) => { + const session = sessionManager.findActiveSession(startTime, options) + return session && session.trackingType === LoggerTrackingType.TRACKED + ? { + id: session.id, + } + : undefined }, expireObservable: sessionManager.expireObservable, } @@ -49,7 +43,7 @@ export function startLogsSessionManager( export function startLogsSessionManagerStub(configuration: LogsConfiguration): LogsSessionManager { const isTracked = computeTrackingType(configuration) === LoggerTrackingType.TRACKED - const session = isTracked ? { isActiveAt: () => true } : undefined + const session = isTracked ? {} : undefined return { findTrackedSession: () => session, expireObservable: new Observable(), From ff9931593665e404cc66399d108010c575a58a63 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Fri, 24 May 2024 09:51:17 +0200 Subject: [PATCH 15/16] Set default options --- packages/core/src/tools/valueHistory.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index 269d100f1b..1f14deaab6 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -63,10 +63,13 @@ export class ValueHistory { * * If `option.returnInactive` is true, returns the value at `startTime` (active or not). */ - find(startTime: RelativeTime = END_OF_TIMES, options?: { returnInactive: boolean }): Value | undefined { + find( + startTime: RelativeTime = END_OF_TIMES, + options: { returnInactive: boolean } = { returnInactive: false } + ): Value | undefined { for (const entry of this.entries) { if (entry.startTime <= startTime) { - if (options?.returnInactive || startTime <= entry.endTime) { + if (options.returnInactive || startTime <= entry.endTime) { return entry.value } break From ff8b19650be25499a1a8ed9684bf0e5153019835 Mon Sep 17 00:00:00 2001 From: Nicolas Ulrich Date: Fri, 24 May 2024 09:52:58 +0200 Subject: [PATCH 16/16] Rename findActiveSession to findSession --- .../src/domain/session/sessionManager.spec.ts | 78 +++++++++---------- .../core/src/domain/session/sessionManager.ts | 4 +- .../logs/src/domain/logsSessionManager.ts | 2 +- .../rum-core/src/domain/rumSessionManager.ts | 2 +- 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index b06b881f68..39880f463f 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -48,25 +48,25 @@ describe('startSessionManager', () => { } function expectSessionIdToBe(sessionManager: SessionManager, sessionId: string) { - expect(sessionManager.findActiveSession()!.id).toBe(sessionId) + expect(sessionManager.findSession()!.id).toBe(sessionId) expect(getCookie(SESSION_STORE_KEY)).toContain(`id=${sessionId}`) } function expectSessionIdToBeDefined(sessionManager: SessionManager) { - expect(sessionManager.findActiveSession()!.id).toMatch(/^[a-f0-9-]+$/) - expect(sessionManager.findActiveSession()?.isExpired).toBeUndefined() + expect(sessionManager.findSession()!.id).toMatch(/^[a-f0-9-]+$/) + expect(sessionManager.findSession()?.isExpired).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).toMatch(/id=[a-f0-9-]+/) expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') } function expectSessionToBeExpired(sessionManager: SessionManager) { - expect(sessionManager.findActiveSession()).toBeUndefined() + expect(sessionManager.findSession()).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).toContain('isExpired=1') } function expectSessionIdToNotBeDefined(sessionManager: SessionManager) { - expect(sessionManager.findActiveSession()!.id).toBeUndefined() + expect(sessionManager.findSession()!.id).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') } @@ -75,12 +75,12 @@ describe('startSessionManager', () => { productKey: string, trackingType: FakeTrackingType ) { - expect(sessionManager.findActiveSession()!.trackingType).toEqual(trackingType) + expect(sessionManager.findSession()!.trackingType).toEqual(trackingType) expect(getCookie(SESSION_STORE_KEY)).toContain(`${productKey}=${trackingType}`) } function expectTrackingTypeToNotBeDefined(sessionManager: SessionManager, productKey: string) { - expect(sessionManager.findActiveSession()?.trackingType).toBeUndefined() + expect(sessionManager.findSession()?.trackingType).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).not.toContain(`${productKey}=`) } @@ -115,7 +115,7 @@ describe('startSessionManager', () => { deleteSessionCookie() - expect(sessionManager.findActiveSession()).toBeUndefined() + expect(sessionManager.findSession()).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() window.dispatchEvent(createNewEvent(DOM_EVENT.RESUME)) @@ -231,13 +231,13 @@ describe('startSessionManager', () => { expect(renewSessionSpy).not.toHaveBeenCalled() - expect(sessionManager.findActiveSession()).toBeUndefined() + expect(sessionManager.findSession()).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) expect(renewSessionSpy).not.toHaveBeenCalled() - expect(sessionManager.findActiveSession()).toBeUndefined() + expect(sessionManager.findSession()).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() }) }) @@ -245,10 +245,10 @@ describe('startSessionManager', () => { describe('multiple startSessionManager calls', () => { it('should re-use the same session id', () => { const firstSessionManager = startSessionManagerWithDefaults({ productKey: FIRST_PRODUCT_KEY }) - const idA = firstSessionManager.findActiveSession()!.id + const idA = firstSessionManager.findSession()!.id const secondSessionManager = startSessionManagerWithDefaults({ productKey: SECOND_PRODUCT_KEY }) - const idB = secondSessionManager.findActiveSession()!.id + const idB = secondSessionManager.findSession()!.id expect(idA).toBe(idB) }) @@ -287,8 +287,8 @@ describe('startSessionManager', () => { computeSessionState: () => NOT_TRACKED_SESSION_STATE, }) - expect(firstSessionManager.findActiveSession()!.trackingType).toEqual(FakeTrackingType.TRACKED) - expect(secondSessionManager.findActiveSession()!.trackingType).toEqual(FakeTrackingType.NOT_TRACKED) + expect(firstSessionManager.findSession()!.trackingType).toEqual(FakeTrackingType.TRACKED) + expect(secondSessionManager.findSession()!.trackingType).toEqual(FakeTrackingType.NOT_TRACKED) }) it('should notify each expire and renew observables', () => { @@ -324,7 +324,7 @@ describe('startSessionManager', () => { const expireSessionSpy = jasmine.createSpy() sessionManager.expireObservable.subscribe(expireSessionSpy) - expect(sessionManager.findActiveSession()).toBeDefined() + expect(sessionManager.findSession()).toBeDefined() expect(getCookie(SESSION_STORE_KEY)).toBeDefined() clock.tick(SESSION_TIME_OUT_DELAY) @@ -339,7 +339,7 @@ describe('startSessionManager', () => { const expireSessionSpy = jasmine.createSpy() sessionManager.expireObservable.subscribe(expireSessionSpy) - expect(sessionManager.findActiveSession()!.id).not.toBe('abcde') + expect(sessionManager.findSession()!.id).not.toBe('abcde') expect(getCookie(SESSION_STORE_KEY)).toContain(`created=${Date.now()}`) expect(expireSessionSpy).not.toHaveBeenCalled() // the session has not been active from the start }) @@ -349,7 +349,7 @@ describe('startSessionManager', () => { const sessionManager = startSessionManagerWithDefaults() - expect(sessionManager.findActiveSession()!.id).toBe('abcde') + expect(sessionManager.findSession()!.id).toBe('abcde') expect(getCookie(SESSION_STORE_KEY)).not.toContain('created=') }) }) @@ -509,14 +509,14 @@ describe('startSessionManager', () => { const sessionManager = startSessionManagerWithDefaults() expireSessionCookie() - expect(sessionManager.findActiveSession()).toBeUndefined() + expect(sessionManager.findSession()).toBeUndefined() }) it('should return the current session context when there is no start time', () => { const sessionManager = startSessionManagerWithDefaults() - expect(sessionManager.findActiveSession()!.id).toBeDefined() - expect(sessionManager.findActiveSession()!.trackingType).toBeDefined() + expect(sessionManager.findSession()!.id).toBeDefined() + expect(sessionManager.findSession()!.trackingType).toBeDefined() }) it('should return the session context corresponding to startTime', () => { @@ -524,8 +524,8 @@ describe('startSessionManager', () => { // 0s to 10s: first session clock.tick(10 * ONE_SECOND - STORAGE_POLL_DELAY) - const firstSessionId = sessionManager.findActiveSession()!.id - const firstSessionTrackingType = sessionManager.findActiveSession()!.trackingType + const firstSessionId = sessionManager.findSession()!.id + const firstSessionTrackingType = sessionManager.findSession()!.trackingType expireSessionCookie() // 10s to 20s: no session @@ -534,16 +534,14 @@ describe('startSessionManager', () => { // 20s to end: second session document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) clock.tick(10 * ONE_SECOND) - const secondSessionId = sessionManager.findActiveSession()!.id - const secondSessionTrackingType = sessionManager.findActiveSession()!.trackingType - - expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) - expect(sessionManager.findActiveSession((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( - firstSessionTrackingType - ) - expect(sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime)).toBeUndefined() - expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) - expect(sessionManager.findActiveSession((25 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( + const secondSessionId = sessionManager.findSession()!.id + const secondSessionTrackingType = sessionManager.findSession()!.trackingType + + expect(sessionManager.findSession((5 * ONE_SECOND) as RelativeTime)!.id).toBe(firstSessionId) + expect(sessionManager.findSession((5 * ONE_SECOND) as RelativeTime)!.trackingType).toBe(firstSessionTrackingType) + expect(sessionManager.findSession((15 * ONE_SECOND) as RelativeTime)).toBeUndefined() + expect(sessionManager.findSession((25 * ONE_SECOND) as RelativeTime)!.id).toBe(secondSessionId) + expect(sessionManager.findSession((25 * ONE_SECOND) as RelativeTime)!.trackingType).toBe( secondSessionTrackingType ) }) @@ -560,20 +558,16 @@ describe('startSessionManager', () => { // 10s to 20s: no session clock.tick(10 * ONE_SECOND) - expect( - sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: true }) - ).toBeDefined() + expect(sessionManager.findSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: true })).toBeDefined() - expect( - sessionManager.findActiveSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: false }) - ).toBeUndefined() + expect(sessionManager.findSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: false })).toBeUndefined() }) }) it('should return the current session context in the renewObservable callback', () => { const sessionManager = startSessionManagerWithDefaults() let currentSession - sessionManager.renewObservable.subscribe(() => (currentSession = sessionManager.findActiveSession())) + sessionManager.renewObservable.subscribe(() => (currentSession = sessionManager.findSession())) // new session expireSessionCookie() @@ -586,7 +580,7 @@ describe('startSessionManager', () => { it('should return the current session context in the expireObservable callback', () => { const sessionManager = startSessionManagerWithDefaults() let currentSession - sessionManager.expireObservable.subscribe(() => (currentSession = sessionManager.findActiveSession())) + sessionManager.expireObservable.subscribe(() => (currentSession = sessionManager.findSession())) // new session expireSessionCookie() @@ -621,7 +615,7 @@ describe('startSessionManager', () => { it('renews the session when tracking consent is granted', () => { const trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED) const sessionManager = startSessionManagerWithDefaults({ trackingConsentState }) - const initialSessionId = sessionManager.findActiveSession()!.id + const initialSessionId = sessionManager.findSession()!.id trackingConsentState.update(TrackingConsent.NOT_GRANTED) @@ -632,7 +626,7 @@ describe('startSessionManager', () => { clock.tick(STORAGE_POLL_DELAY) expectSessionIdToBeDefined(sessionManager) - expect(sessionManager.findActiveSession()!.id).not.toBe(initialSessionId) + expect(sessionManager.findSession()!.id).not.toBe(initialSessionId) }) }) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index d9ac971c72..3bdbfe3d0d 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -11,7 +11,7 @@ import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { startSessionStore } from './sessionStore' export interface SessionManager { - findActiveSession: ( + findSession: ( startTime?: RelativeTime, options?: { returnInactive: boolean } ) => SessionContext | undefined @@ -83,7 +83,7 @@ export function startSessionManager( } return { - findActiveSession: (startTime, options) => sessionContextHistory.find(startTime, options), + findSession: (startTime, options) => sessionContextHistory.find(startTime, options), renewObservable, expireObservable, expire: sessionStore.expire, diff --git a/packages/logs/src/domain/logsSessionManager.ts b/packages/logs/src/domain/logsSessionManager.ts index 89a9e6b81d..def6986bda 100644 --- a/packages/logs/src/domain/logsSessionManager.ts +++ b/packages/logs/src/domain/logsSessionManager.ts @@ -30,7 +30,7 @@ export function startLogsSessionManager( ) return { findTrackedSession: (startTime?: RelativeTime, options = { returnInactive: false }) => { - const session = sessionManager.findActiveSession(startTime, options) + const session = sessionManager.findSession(startTime, options) return session && session.trackingType === LoggerTrackingType.TRACKED ? { id: session.id, diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index f4e8d06f10..6448e8e84b 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -52,7 +52,7 @@ export function startRumSessionManager( return { findTrackedSession: (startTime) => { - const session = sessionManager.findActiveSession(startTime) + const session = sessionManager.findSession(startTime) if (!session || !isTypeTracked(session.trackingType)) { return }