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 13 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
17 changes: 9 additions & 8 deletions developer-extension/src/panel/components/tabs/replayTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ 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." />
}

return <Player />
return <Player isReplayForced={infos.cookie.forcedReplay === '1'} />
}

function Player() {
function Player({ isReplayForced }: { isReplayForced: boolean }) {
const frameRef = useRef<HTMLIFrameElement | null>(null)
const [playerStatus, setPlayerStatus] = useState<SessionReplayPlayerStatus>('loading')

Expand All @@ -43,30 +43,31 @@ function Player() {
return (
<TabBase>
<iframe ref={frameRef} className={classes.iframe} data-status={playerStatus} />
{playerStatus === 'waiting-for-full-snapshot' && <WaitingForFullSnapshot />}
{playerStatus === 'waiting-for-full-snapshot' && <WaitingForFullSnapshot isReplayForced={isReplayForced} />}
</TabBase>
)
}

function WaitingForFullSnapshot() {
function WaitingForFullSnapshot({ isReplayForced }: { isReplayForced: boolean }) {
return (
<Alert
level="warning"
message="Waiting for a full snapshot to be generated..."
button={
<Button onClick={generateFullSnapshot} color="orange">
<Button onClick={() => generateFullSnapshot(isReplayForced)} color="orange">
Generate Full Snapshot
</Button>
}
/>
)
}

function generateFullSnapshot() {
function generateFullSnapshot(isReplayForced: boolean) {
// Restart to make sure we have a fresh Full Snapshot
const startParam = isReplayForced ? '{ force: true }' : ''
evalInWindow(`
DD_RUM.stopSessionReplayRecording()
DD_RUM.startSessionReplayRecording()
DD_RUM.startSessionReplayRecording(${startParam})
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
`).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
10 changes: 10 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,16 @@ describe('startSessionManager', () => {
})
})

describe('forced replay', () => {
it('should update current entity when replay recording is forced', () => {
const sessionManager = startSessionManagerWithDefaults()
sessionManager.setForcedReplay()

expectSessionIdToBeDefined(sessionManager)
expect(sessionManager.findSession()!.isReplayForced).toBe(true)
})
})

function startSessionManagerWithDefaults({
configuration,
productKey = FIRST_PRODUCT_KEY,
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/domain/session/sessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ export interface SessionManager<TrackingType extends string> {
renewObservable: Observable<void>
expireObservable: Observable<void>
expire: () => void
setForcedReplay: () => 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 @@ -53,6 +55,12 @@ export function startSessionManager<TrackingType extends string>(
expireObservable.notify()
sessionContextHistory.closeActive(relativeNow())
})
sessionStore.forceReplayObservable.subscribe(() => {
Copy link
Member

Choose a reason for hiding this comment

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

💭 thought: ‏RUM logic is leaking into core. Maybe we can think of a way to abstract it, via computeSessionState?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but that wouldn't be enough because the sessionContextHistory entry won't be updated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✔️

const sessionEntity = sessionContextHistory.find()
if (sessionEntity) {
sessionEntity.isReplayForced = true
}
})

// We expand/renew session unconditionally as tracking consent is always granted when the session
// manager is started.
Expand All @@ -79,6 +87,7 @@ export function startSessionManager<TrackingType extends string>(
return {
id: sessionStore.getSession().id!,
trackingType: sessionStore.getSession()[productKey] as TrackingType,
isReplayForced: !!sessionStore.getSession().forcedReplay,
}
}

Expand All @@ -87,6 +96,7 @@ export function startSessionManager<TrackingType extends string>(
renewObservable,
expireObservable,
expire: sessionStore.expire,
setForcedReplay: sessionStore.setForcedReplay,
}
}

Expand Down
56 changes: 56 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,60 @@ describe('session store', () => {
})
})
})

describe('session update and synchronisation', () => {
let updateSpy: () => void
let otherUpdateSpy: () => void
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.forceReplayObservable.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.setForcedReplay()

expect(updateSpy).toHaveBeenCalled()

// Need to wait until watch is triggered
clock.tick(STORAGE_POLL_DELAY)
expect(otherUpdateSpy).toHaveBeenCalled()
})
})
})
19 changes: 19 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>
forceReplayObservable: Observable<void>
nulrich marked this conversation as resolved.
Show resolved Hide resolved
expire: () => void
stop: () => void
setForcedReplay: () => 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 forceReplayObservable = new Observable<void>()

const sessionStoreStrategy =
sessionStoreStrategyType.type === 'Cookie'
Expand Down Expand Up @@ -128,6 +132,9 @@ export function startSessionStore<TrackingType extends string>(
if (isSessionInCacheOutdated(sessionState)) {
expireSessionInCache()
} else {
if (sessionState.forcedReplay && !sessionCache.forcedReplay) {
forceReplayObservable.notify()
}
sessionCache = sessionState
}
}
Expand Down Expand Up @@ -182,12 +189,23 @@ export function startSessionStore<TrackingType extends string>(
renewObservable.notify()
}

function setForcedReplay() {
processSessionStoreOperations(
{
process: (sessionState) => assign({}, sessionState, { forcedReplay: '1' }),
after: synchronizeSession,
},
sessionStoreStrategy
)
}

return {
expandOrRenewSession: throttledExpandOrRenewSession,
expandSession,
getSession: () => sessionCache,
renewObservable,
expireObservable,
forceReplayObservable,
restartSession: startSession,
expire: () => {
cancelExpandOrRenewSession()
Expand All @@ -197,5 +215,6 @@ export function startSessionStore<TrackingType extends string>(
stop: () => {
clearInterval(watchSessionTimeoutId)
},
setForcedReplay,
}
}
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 @@ -482,10 +485,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
2 changes: 1 addition & 1 deletion packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ 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.sampledForReplay
}

if (!isEmptyObject(commonContext.user)) {
Expand Down
9 changes: 8 additions & 1 deletion packages/rum-core/src/domain/rumSessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ export interface RumSessionManager {
findTrackedSession: (startTime?: RelativeTime) => RumSession | undefined
expire: () => void
expireObservable: Observable<void>
setForcedReplay: () => void
}

export type RumSession = {
id: string
sampledForReplay: boolean
sessionReplayAllowed: boolean
}

Expand Down Expand Up @@ -58,11 +60,14 @@ export function startRumSessionManager(
}
return {
id: session.id,
sessionReplayAllowed: session.trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY,
sampledForReplay: session.trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY,
sessionReplayAllowed:
session.trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || session.isReplayForced,
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
}
},
expire: sessionManager.expire,
expireObservable: sessionManager.expireObservable,
setForcedReplay: sessionManager.setForcedReplay,
}
}

Expand All @@ -72,12 +77,14 @@ export function startRumSessionManager(
export function startRumSessionManagerStub(): RumSessionManager {
const session: RumSession = {
id: '00000000-aaaa-0000-aaaa-000000000000',
sampledForReplay: bridgeSupports(BridgeCapability.RECORDS),
sessionReplayAllowed: bridgeSupports(BridgeCapability.RECORDS),
}
return {
findTrackedSession: () => session,
expire: noop,
expireObservable: new Observable(),
setForcedReplay: noop,
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/rum-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { RumPublicApi, makeRumPublicApi, RecorderApi } from './boot/rumPublicApi'
export { RumPublicApi, makeRumPublicApi, RecorderApi, StartRecordingOptions } from './boot/rumPublicApi'
export { StartRum } from './boot/startRum'
export {
RumEvent,
Expand Down
2 changes: 2 additions & 0 deletions packages/rum-core/test/mockRumSessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export function createRumSessionManagerMock(): RumSessionManagerMock {
}
return {
id,
sampledForReplay: sessionStatus === SessionStatus.TRACKED_WITH_SESSION_REPLAY,
sessionReplayAllowed: sessionStatus === SessionStatus.TRACKED_WITH_SESSION_REPLAY,
}
},
Expand All @@ -53,5 +54,6 @@ export function createRumSessionManagerMock(): RumSessionManagerMock {
sessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY
return this
},
setForcedReplay() {},
}
}
7 changes: 7 additions & 0 deletions packages/rum/src/boot/recorderApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ describe('makeRecorderApi', () => {
expect(startRecordingSpy).not.toHaveBeenCalled()
})

it('should start recording if session is tracked without session replay when forced', () => {
setupBuilder.withSessionManager(createRumSessionManagerMock().setTrackedWithoutSessionReplay()).build()
rumInit()
recorderApi.start({ force: true })
expect(startRecordingSpy).toHaveBeenCalledTimes(1)
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
})

it('uses the previously created worker if available', () => {
setupBuilder.build()
rumInit({ worker: mockWorker })
Expand Down
Loading