Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [RUM-2720] Send logs without session id when session inactive #2578

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 54 additions & 38 deletions packages/core/src/domain/session/sessionManager.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -38,7 +38,7 @@ describe('startSessionManager', () => {
let clock: Clock

function expireSessionCookie() {
setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION)
expireCookie()
clock.tick(STORAGE_POLL_DELAY)
}

Expand All @@ -48,25 +48,25 @@ describe('startSessionManager', () => {
}

function expectSessionIdToBe(sessionManager: SessionManager<FakeTrackingType>, 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<FakeTrackingType>) {
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<FakeTrackingType>) {
expect(sessionManager.findActiveSession()).toBeUndefined()
expect(sessionManager.findSession()).toBeUndefined()
expect(getCookie(SESSION_STORE_KEY)).toContain('isExpired=1')
}

function expectSessionIdToNotBeDefined(sessionManager: SessionManager<FakeTrackingType>) {
expect(sessionManager.findActiveSession()!.id).toBeUndefined()
expect(sessionManager.findSession()!.id).toBeUndefined()
expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=')
}

Expand All @@ -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<FakeTrackingType>, productKey: string) {
expect(sessionManager.findActiveSession()?.trackingType).toBeUndefined()
expect(sessionManager.findSession()?.trackingType).toBeUndefined()
expect(getCookie(SESSION_STORE_KEY)).not.toContain(`${productKey}=`)
}

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -231,24 +231,24 @@ 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()
})
})

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)
})
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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)
Expand All @@ -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
})
Expand All @@ -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=')
})
})
Expand Down Expand Up @@ -509,23 +509,23 @@ 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', () => {
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
const firstSessionId = sessionManager.findSession()!.id
const firstSessionTrackingType = sessionManager.findSession()!.trackingType
expireSessionCookie()

// 10s to 20s: no session
Expand All @@ -534,24 +534,40 @@ 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
)
})

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

// 10s to 20s: no session
clock.tick(10 * ONE_SECOND)

expect(sessionManager.findSession((15 * ONE_SECOND) as RelativeTime, { returnInactive: true })).toBeDefined()

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()
Expand All @@ -564,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()
Expand Down Expand Up @@ -599,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)

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

Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/domain/session/sessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import { SESSION_TIME_OUT_DELAY } from './sessionConstants'
import { startSessionStore } from './sessionStore'

export interface SessionManager<TrackingType extends string> {
findActiveSession: (startTime?: RelativeTime) => SessionContext<TrackingType> | undefined
findSession: (
startTime?: RelativeTime,
options?: { returnInactive: boolean }
) => SessionContext<TrackingType> | undefined
renewObservable: Observable<void>
expireObservable: Observable<void>
expire: () => void
Expand Down Expand Up @@ -80,7 +83,7 @@ export function startSessionManager<TrackingType extends string>(
}

return {
findActiveSession: (startTime) => sessionContextHistory.find(startTime),
findSession: (startTime, options) => sessionContextHistory.find(startTime, options),
renewObservable,
expireObservable,
expire: sessionStore.expire,
Expand Down
28 changes: 14 additions & 14 deletions packages/core/src/domain/session/sessionStore.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -52,7 +52,7 @@ function getStoreExpiration() {
}

function resetSessionInStore() {
setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION)
expireCookie()
}

describe('session store', () => {
Expand Down Expand Up @@ -172,13 +172,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 }))

Expand All @@ -187,7 +187,7 @@ describe('session store', () => {
expect(sessionStoreManager.getSession().id).toBeUndefined()
expectNotTrackedSessionToBeInStore()
expect(expireSpy).not.toHaveBeenCalled()
expect(renewSpy).not.toHaveBeenCalled()
expect(renewSpy).toHaveBeenCalledTimes(1)
}
)

Expand All @@ -200,7 +200,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(
Expand All @@ -218,13 +218,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 }))
Expand All @@ -236,13 +236,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 }))
Expand All @@ -254,7 +254,7 @@ describe('session store', () => {
expect(sessionStoreManager.getSession()[PRODUCT_KEY]).toBeDefined()
expectNotTrackedSessionToBeInStore()
expect(expireSpy).toHaveBeenCalled()
expect(renewSpy).not.toHaveBeenCalled()
expect(renewSpy).toHaveBeenCalledTimes(1)
}
)

Expand Down Expand Up @@ -285,13 +285,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) => ({
Expand All @@ -305,7 +305,7 @@ describe('session store', () => {
expect(sessionStoreManager.getSession().id).toBeUndefined()
expectNotTrackedSessionToBeInStore()
expect(expireSpy).toHaveBeenCalled()
expect(renewSpy).not.toHaveBeenCalled()
expect(renewSpy).toHaveBeenCalledTimes(1)
}
)

Expand Down
Loading