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-3837] Force Replay recording on sampled-out sessions #2777

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c5e1614
Force Replay recording on sampled-out sessions
N-Boutaib May 23, 2024
488177d
[RUM-3887] Add unit and E2E tests
N-Boutaib May 23, 2024
ece231f
[RUM-3837] Expose method to update session state for forced replay
N-Boutaib May 23, 2024
637814d
[RUM-3837] Separate sampling state from replay state
N-Boutaib May 23, 2024
bcbebc4
Update session context history when replay is forced
N-Boutaib May 24, 2024
d48ac9f
Add unit tests and rename function
N-Boutaib May 24, 2024
88c335a
Format code
N-Boutaib May 24, 2024
edd6cb4
Merge branch 'main' into najib.boutaib/RUM-3837-recording-of-sampled-…
N-Boutaib May 28, 2024
ed446dd
Sumbodule sync
N-Boutaib May 28, 2024
6c4705e
Update spec file after refactoring
N-Boutaib May 28, 2024
2f8b40e
Update types after refactoring
N-Boutaib May 28, 2024
bea862d
Make param optional in public api types
N-Boutaib May 28, 2024
9e203bd
Update browser extension
N-Boutaib May 28, 2024
2ccf155
Apply reviews
N-Boutaib May 29, 2024
e8d6cb1
Add extra check in e2e test
N-Boutaib May 29, 2024
b595827
Format
N-Boutaib May 29, 2024
1be6164
Format
N-Boutaib May 29, 2024
1887627
Move history update context to higher level
N-Boutaib May 29, 2024
74ff0ec
Move rum logic to rumSessionManager
N-Boutaib May 30, 2024
58927d7
Remove forgotten code
N-Boutaib May 31, 2024
3e8657b
Probably the last commit :sweat_smile:
N-Boutaib May 31, 2024
2e6a761
Format last commit
N-Boutaib May 31, 2024
0dac36d
Merge branch 'main' into najib.boutaib/RUM-3837-recording-of-sampled-…
N-Boutaib Jun 3, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function InfosTab() {
)
}
/>
{infos.cookie.forcedReplay && <Entry name="Is Replay Forced" value={'True'} />}
<Entry name="Created" value={infos.cookie.created && formatDate(Number(infos.cookie.created))} />
<Entry name="Expire" value={infos.cookie.expire && formatDate(Number(infos.cookie.expire))} />
<Button color="violet" variant="light" onClick={endSession} className="dd-privacy-allow">
Expand Down
4 changes: 2 additions & 2 deletions developer-extension/src/panel/components/tabs/replayTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export function ReplayTab() {
return <Alert level="error" message="RUM session sampled out." />
}

if (infos.cookie.rum === '2') {
if (infos.cookie.rum === '2' && infos.cookie.forcedReplay !== '1') {
return <Alert level="error" message="RUM session plan does not include replay." />
}

Expand Down Expand Up @@ -66,7 +66,7 @@ function generateFullSnapshot() {
// Restart to make sure we have a fresh Full Snapshot
evalInWindow(`
DD_RUM.stopSessionReplayRecording()
DD_RUM.startSessionReplayRecording()
DD_RUM.startSessionReplayRecording({ force: true })
`).catch((error) => {
logger.error('While restarting recording:', error)
})
Expand Down
1 change: 1 addition & 0 deletions developer-extension/src/panel/hooks/useSdkInfos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface SdkInfos {
expire?: string
logs?: string
rum?: string
forcedReplay?: '1'
}
}

Expand Down
17 changes: 17 additions & 0 deletions packages/core/src/domain/session/sessionManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,23 @@ describe('startSessionManager', () => {
})
})

describe('session state update', () => {
it('should notify session manager update observable', () => {
const sessionStateUpdateSpy = jasmine.createSpy()
const sessionManager = startSessionManagerWithDefaults()
sessionManager.sessionStateUpdateObservable.subscribe(sessionStateUpdateSpy)

sessionManager.updateSessionState({ extra: 'extra' })

expectSessionIdToBeDefined(sessionManager)
expect(sessionStateUpdateSpy).toHaveBeenCalledTimes(1)

const callArgs = sessionStateUpdateSpy.calls.argsFor(0)[0]
expect(callArgs.previousState.extra).toBeUndefined()
expect(callArgs.newState.extra).toBe('extra')
})
})

function startSessionManagerWithDefaults({
configuration,
productKey = FIRST_PRODUCT_KEY,
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/domain/session/sessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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<TrackingType extends string> {
findSession: (
Expand All @@ -17,12 +18,15 @@ export interface SessionManager<TrackingType extends string> {
) => SessionContext<TrackingType> | undefined
renewObservable: Observable<void>
expireObservable: Observable<void>
sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }>
expire: () => void
updateSessionState: (state: Partial<SessionState>) => void
}

export interface SessionContext<TrackingType extends string> extends Context {
id: string
trackingType: TrackingType
isReplayForced: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 thought: ‏Ideally we should remove any notion of "replay" from @datadog/browser-core. But I understand this last bit is not trivial to fix, so let's keep it like this for now, and we'll re-explore this later (I opened https://datadoghq.atlassian.net/browse/RUM-4740 for that)

}

export const VISIBILITY_CHECK_DELAY = ONE_MINUTE
Expand Down Expand Up @@ -79,14 +83,17 @@ export function startSessionManager<TrackingType extends string>(
return {
id: sessionStore.getSession().id!,
trackingType: sessionStore.getSession()[productKey] as TrackingType,
isReplayForced: !!sessionStore.getSession().forcedReplay,
}
}

return {
findSession: (startTime, options) => sessionContextHistory.find(startTime, options),
renewObservable,
expireObservable,
sessionStateUpdateObservable: sessionStore.sessionStateUpdateObservable,
expire: sessionStore.expire,
updateSessionState: sessionStore.updateSessionState,
}
}

Expand Down
60 changes: 60 additions & 0 deletions packages/core/src/domain/session/sessionStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,4 +472,64 @@ describe('session store', () => {
})
})
})

describe('session update and synchronisation', () => {
let updateSpy: jasmine.Spy<jasmine.Func>
let otherUpdateSpy: jasmine.Spy<jasmine.Func>
let clock: Clock

function setupSessionStore(updateSpy: () => void) {
const computeSessionState: (rawTrackingType?: string) => {
trackingType: FakeTrackingType
isTracked: boolean
} = () => ({
isTracked: true,
trackingType: FakeTrackingType.TRACKED,
})
const sessionStoreStrategyType = selectSessionStoreStrategyType({
clientToken: 'abc',
allowFallbackToLocalStorage: false,
})

const sessionStoreManager = startSessionStore(sessionStoreStrategyType!, PRODUCT_KEY, computeSessionState)
sessionStoreManager.sessionStateUpdateObservable.subscribe(updateSpy)

return sessionStoreManager
}

let sessionStoreManager: SessionStore
let otherSessionStoreManager: SessionStore

beforeEach(() => {
updateSpy = jasmine.createSpy()
otherUpdateSpy = jasmine.createSpy()
clock = mockClock()
})

afterEach(() => {
resetSessionInStore()
clock.cleanup()
sessionStoreManager.stop()
otherSessionStoreManager.stop()
})

it('should synchronise all stores and notify update observables of all stores', () => {
setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID)

sessionStoreManager = setupSessionStore(updateSpy)
otherSessionStoreManager = setupSessionStore(otherUpdateSpy)

sessionStoreManager.updateSessionState({ extra: 'extra' })

expect(updateSpy).toHaveBeenCalledTimes(1)

const callArgs = updateSpy.calls.argsFor(0)[0]
expect(callArgs!.previousState.extra).toBeUndefined()
expect(callArgs.newState.extra).toBe('extra')

// Need to wait until watch is triggered
clock.tick(STORAGE_POLL_DELAY)
expect(otherUpdateSpy).toHaveBeenCalled()
})
})
})
17 changes: 17 additions & 0 deletions packages/core/src/domain/session/sessionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ONE_SECOND, dateNow } from '../../tools/utils/timeUtils'
import { throttle } from '../../tools/utils/functionUtils'
import { generateUUID } from '../../tools/utils/stringUtils'
import type { InitConfiguration } from '../configuration'
import { assign } from '../../tools/utils/polyfills'
import { selectCookieStrategy, initCookieStrategy } from './storeStrategies/sessionInCookie'
import type { SessionStoreStrategyType } from './storeStrategies/sessionStoreStrategy'
import {
Expand All @@ -23,8 +24,10 @@ export interface SessionStore {
restartSession: () => void
renewObservable: Observable<void>
expireObservable: Observable<void>
sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }>
expire: () => void
stop: () => void
updateSessionState: (state: Partial<SessionState>) => void
}

/**
Expand Down Expand Up @@ -61,6 +64,7 @@ export function startSessionStore<TrackingType extends string>(
): SessionStore {
const renewObservable = new Observable<void>()
const expireObservable = new Observable<void>()
const sessionStateUpdateObservable = new Observable<{ previousState: SessionState; newState: SessionState }>()

const sessionStoreStrategy =
sessionStoreStrategyType.type === 'Cookie'
Expand Down Expand Up @@ -128,6 +132,7 @@ export function startSessionStore<TrackingType extends string>(
if (isSessionInCacheOutdated(sessionState)) {
expireSessionInCache()
} else {
sessionStateUpdateObservable.notify({ previousState: sessionCache, newState: sessionState })
sessionCache = sessionState
}
}
Expand Down Expand Up @@ -182,12 +187,23 @@ export function startSessionStore<TrackingType extends string>(
renewObservable.notify()
}

function updateSessionState(partialSessionState: Partial<SessionState>) {
processSessionStoreOperations(
{
process: (sessionState) => assign({}, sessionState, partialSessionState),
after: synchronizeSession,
},
sessionStoreStrategy
)
}

return {
expandOrRenewSession: throttledExpandOrRenewSession,
expandSession,
getSession: () => sessionCache,
renewObservable,
expireObservable,
sessionStateUpdateObservable,
restartSession: startSession,
expire: () => {
cancelExpandOrRenewSession()
Expand All @@ -197,5 +213,6 @@ export function startSessionStore<TrackingType extends string>(
stop: () => {
clearInterval(watchSessionTimeoutId)
},
updateSessionState,
}
}
14 changes: 8 additions & 6 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ import type { InternalContext } from '../domain/contexts/internalContext'
import { createPreStartStrategy } from './preStartRum'
import type { StartRum, StartRumResult } from './startRum'

export interface StartRecordingOptions {
force: boolean
}
export interface RumPublicApi extends PublicApi {
/**
* Init the RUM browser SDK.
Expand Down Expand Up @@ -234,7 +237,7 @@ export interface RumPublicApi extends PublicApi {
*
* See [Browser Session Replay](https://docs.datadoghq.com/real_user_monitoring/session_replay/browser) for further information.
*/
startSessionReplayRecording: () => void
startSessionReplayRecording: (options?: StartRecordingOptions) => void

/**
* Stop Session Replay recording.
Expand All @@ -245,7 +248,7 @@ export interface RumPublicApi extends PublicApi {
}

export interface RecorderApi {
start: () => void
start: (options?: StartRecordingOptions) => void
stop: () => void
onRumStart: (
lifeCycle: LifeCycle,
Expand Down Expand Up @@ -487,10 +490,9 @@ export function makeRumPublicApi(
}),

getSessionReplayLink: monitor(() => recorderApi.getSessionReplayLink()),

startSessionReplayRecording: monitor(() => {
recorderApi.start()
addTelemetryUsage({ feature: 'start-session-replay-recording' })
startSessionReplayRecording: monitor((options?: StartRecordingOptions) => {
recorderApi.start(options)
addTelemetryUsage({ feature: 'start-session-replay-recording', force: options?.force })
}),

stopSessionReplayRecording: monitor(() => recorderApi.stop()),
Expand Down
5 changes: 3 additions & 2 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import type { CiVisibilityContext } from './contexts/ciVisibilityContext'
import type { LifeCycle } from './lifeCycle'
import { LifeCycleEventType } from './lifeCycle'
import type { ViewContexts } from './contexts/viewContexts'
import type { RumSessionManager } from './rumSessionManager'
import { SessionReplayState, type RumSessionManager } from './rumSessionManager'
import type { UrlContexts } from './contexts/urlContexts'
import type { RumConfiguration } from './configuration'
import type { ActionContexts } from './action/actionCollection'
Expand Down Expand Up @@ -178,7 +178,8 @@ export function startRumAssembly(
;(serverRumEvent.session as Mutable<RumEvent['session']>).has_replay = commonContext.hasReplay
}
if (serverRumEvent.type === 'view') {
;(serverRumEvent.session as Mutable<RumEvent['session']>).sampled_for_replay = session.sessionReplayAllowed
;(serverRumEvent.session as Mutable<RumEvent['session']>).sampled_for_replay =
session.sessionReplay === SessionReplayState.SAMPLED
}

if (!isEmptyObject(commonContext.user)) {
Expand Down
25 changes: 17 additions & 8 deletions packages/rum-core/src/domain/rumSessionManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { LifeCycle, LifeCycleEventType } from './lifeCycle'
import {
RUM_SESSION_KEY,
RumTrackingType,
SessionReplayState,
startRumSessionManager,
startRumSessionManagerStub,
} from './rumSessionManager'
Expand Down Expand Up @@ -163,13 +164,21 @@ describe('rum session manager', () => {
it('should return session TRACKED_WITH_SESSION_REPLAY', () => {
setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=1', DURATION)
const rumSessionManager = startRumSessionManagerWithDefaults()
expect(rumSessionManager.findTrackedSession()!.sessionReplayAllowed).toBe(true)
expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED)
})

it('should return session TRACKED_WITHOUT_SESSION_REPLAY', () => {
setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=2', DURATION)
const rumSessionManager = startRumSessionManagerWithDefaults()
expect(rumSessionManager.findTrackedSession()!.sessionReplayAllowed).toBe(false)
expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF)
})

it('should update current entity when replay recording is forced', () => {
setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=2', DURATION)
const rumSessionManager = startRumSessionManagerWithDefaults()
rumSessionManager.setForcedReplay()

expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.FORCED)
})
})

Expand All @@ -178,12 +187,12 @@ describe('rum session manager', () => {
{
description: 'TRACKED_WITH_SESSION_REPLAY should have replay',
sessionReplaySampleRate: 100,
expectSessionReplay: true,
expectSessionReplay: SessionReplayState.SAMPLED,
},
{
description: 'TRACKED_WITHOUT_SESSION_REPLAY should have no replay',
sessionReplaySampleRate: 0,
expectSessionReplay: false,
expectSessionReplay: SessionReplayState.OFF,
},
].forEach(
({
Expand All @@ -193,13 +202,13 @@ describe('rum session manager', () => {
}: {
description: string
sessionReplaySampleRate: number
expectSessionReplay: boolean
expectSessionReplay: SessionReplayState
}) => {
it(description, () => {
const rumSessionManager = startRumSessionManagerWithDefaults({
configuration: { sessionSampleRate: 100, sessionReplaySampleRate },
})
expect(rumSessionManager.findTrackedSession()!.sessionReplayAllowed).toBe(expectSessionReplay)
expect(rumSessionManager.findTrackedSession()!.sessionReplay).toBe(expectSessionReplay)
})
}
)
Expand All @@ -224,11 +233,11 @@ describe('rum session manager', () => {
describe('rum session manager stub', () => {
it('should return a tracked session with replay allowed when the event bridge support records', () => {
initEventBridgeStub({ capabilities: [BridgeCapability.RECORDS] })
expect(startRumSessionManagerStub().findTrackedSession()!.sessionReplayAllowed).toEqual(true)
expect(startRumSessionManagerStub().findTrackedSession()!.sessionReplay).toEqual(SessionReplayState.SAMPLED)
})

it('should return a tracked session without replay allowed when the event bridge support records', () => {
initEventBridgeStub({ capabilities: [] })
expect(startRumSessionManagerStub().findTrackedSession()!.sessionReplayAllowed).toEqual(false)
expect(startRumSessionManagerStub().findTrackedSession()!.sessionReplay).toEqual(SessionReplayState.OFF)
})
})
Loading