From d5e20f7993c6641914643558c1dd498a2ff6121a Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 21 Apr 2020 14:56:51 +0200 Subject: [PATCH 01/42] :sparkles: add page load UA --- packages/rum/src/trackEventCounts.ts | 8 +++-- packages/rum/src/userActionCollection.ts | 38 ++++++++++++++++++------ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/rum/src/trackEventCounts.ts b/packages/rum/src/trackEventCounts.ts index dc0400ba32..847b603cf7 100644 --- a/packages/rum/src/trackEventCounts.ts +++ b/packages/rum/src/trackEventCounts.ts @@ -8,8 +8,12 @@ export interface EventCounts { resourceCount: number } -export function trackEventCounts(lifeCycle: LifeCycle, callback: (eventCounts: EventCounts) => void = noop) { - const eventCounts = { +export function trackEventCounts( + lifeCycle: LifeCycle, + inputEventCounts?: EventCounts, + callback: (eventCounts: EventCounts) => void = noop +) { + const eventCounts = inputEventCounts || { errorCount: 0, longTaskCount: 0, resourceCount: 0, diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 818d193352..c56020b202 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -1,7 +1,8 @@ import { Context, DOM_EVENT, generateUUID, monitor, Observable } from '@datadog/browser-core' import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' -import { trackEventCounts } from './trackEventCounts' +import { EventCounts, trackEventCounts } from './trackEventCounts' +import { View } from './viewTracker' // Automatic user action collection lifecycle overview: // @@ -66,6 +67,7 @@ export interface AutoUserAction { export type UserAction = CustomUserAction | AutoUserAction export function startUserActionCollection(lifeCycle: LifeCycle) { + const subscriptions: Subscription[] = [] function processClick(event: Event) { if (!(event.target instanceof Element)) { return @@ -76,27 +78,43 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) + function processLoadView(loadedView: View) { + if (!loadedView) { + return + } + + const name = `Load page ${loadedView.location.pathname}` + newUserAction(lifeCycle, UserActionType.LOAD_VIEW, name, loadedView.startTime) + } + + subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processLoadView)) + return { stop() { removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) + subscriptions.forEach((s) => s.unsubscribe()) }, } } -let currentUserAction: { id: string; startTime: number } | undefined +let currentUserAction: { id: string; startTime: number; type: UserActionType; name: string } | undefined +let currentEventCounts: EventCounts | undefined -function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { - if (currentUserAction) { - // Discard any new user action if another one is already occuring. +function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string, inputStartTime?: number) { + if (currentUserAction && (currentUserAction.type === UserActionType.LOAD_VIEW || type === UserActionType.CLICK)) { + // Discard any new click user action if another one is already occuring. + // Discard any new user action if a load_view is already occuring. return } - const id = generateUUID() - const startTime = performance.now() - currentUserAction = { id, startTime } + const id = currentUserAction?.id || generateUUID() + const startTime = currentUserAction?.startTime || inputStartTime || performance.now() + currentUserAction = { id, startTime, type, name } const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) - const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) + const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle, currentEventCounts) + + currentEventCounts = eventCounts waitUserActionCompletion(pageActivitiesObservable, (endTime) => { stopPageActivitiesTracking() @@ -213,6 +231,7 @@ function waitUserActionCompletion( clearTimeout(idleTimeoutId) clearTimeout(maxDurationTimeoutId) currentUserAction = undefined + currentEventCounts = undefined completionCallback(endTime) } } @@ -222,6 +241,7 @@ export const $$tests = { trackPageActivities, resetUserAction() { currentUserAction = undefined + currentEventCounts = undefined }, waitUserActionCompletion, } From 3f0b7fea5e1a637b4c215fa0adfddb21ca58d5a0 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 21 Apr 2020 15:01:54 +0200 Subject: [PATCH 02/42] rm inputEventCounts --- packages/rum/src/trackEventCounts.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/rum/src/trackEventCounts.ts b/packages/rum/src/trackEventCounts.ts index 847b603cf7..dc0400ba32 100644 --- a/packages/rum/src/trackEventCounts.ts +++ b/packages/rum/src/trackEventCounts.ts @@ -8,12 +8,8 @@ export interface EventCounts { resourceCount: number } -export function trackEventCounts( - lifeCycle: LifeCycle, - inputEventCounts?: EventCounts, - callback: (eventCounts: EventCounts) => void = noop -) { - const eventCounts = inputEventCounts || { +export function trackEventCounts(lifeCycle: LifeCycle, callback: (eventCounts: EventCounts) => void = noop) { + const eventCounts = { errorCount: 0, longTaskCount: 0, resourceCount: 0, From a24903d945f73a84381c46bfb135f8dd5299360e Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 21 Apr 2020 15:23:21 +0200 Subject: [PATCH 03/42] :sparkles: fix duplicated UserActionType.LOAD_VIEW sent on update --- packages/rum/src/userActionCollection.ts | 59 +++++++++++++++--------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index c56020b202..4219145e31 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -2,7 +2,7 @@ import { Context, DOM_EVENT, generateUUID, monitor, Observable } from '@datadog/ import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' import { EventCounts, trackEventCounts } from './trackEventCounts' -import { View } from './viewTracker' +import { View } from './viewCollection' // Automatic user action collection lifecycle overview: // @@ -68,6 +68,11 @@ export type UserAction = CustomUserAction | AutoUserAction export function startUserActionCollection(lifeCycle: LifeCycle) { const subscriptions: Subscription[] = [] + let currentViewId: string | undefined + + addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) + subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processLoadView)) + function processClick(event: Event) { if (!(event.target instanceof Element)) { return @@ -76,19 +81,16 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } - addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) - function processLoadView(loadedView: View) { - if (!loadedView) { + if (!loadedView || loadedView.id === currentViewId) { return } + currentViewId = loadedView.id const name = `Load page ${loadedView.location.pathname}` newUserAction(lifeCycle, UserActionType.LOAD_VIEW, name, loadedView.startTime) } - subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processLoadView)) - return { stop() { removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) @@ -97,8 +99,14 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { } } -let currentUserAction: { id: string; startTime: number; type: UserActionType; name: string } | undefined -let currentEventCounts: EventCounts | undefined +interface PartialUserAction { + id: string + startTime: number + type: UserActionType + name: string +} + +let currentUserAction: PartialUserAction | undefined function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string, inputStartTime?: number) { if (currentUserAction && (currentUserAction.type === UserActionType.LOAD_VIEW || type === UserActionType.CLICK)) { @@ -107,30 +115,37 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string, return } - const id = currentUserAction?.id || generateUUID() - const startTime = currentUserAction?.startTime || inputStartTime || performance.now() + if (currentUserAction && currentUserAction.type === UserActionType.CLICK && type === UserActionType.LOAD_VIEW) { + currentUserAction.type = UserActionType.LOAD_VIEW + currentUserAction.name = name + if (inputStartTime) { + currentUserAction.startTime = Math.min(currentUserAction.startTime, inputStartTime) + } + return + } + + const id = generateUUID() + const startTime = inputStartTime || performance.now() currentUserAction = { id, startTime, type, name } const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) - const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle, currentEventCounts) - - currentEventCounts = eventCounts + const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - waitUserActionCompletion(pageActivitiesObservable, (endTime) => { + waitUserActionCompletion(pageActivitiesObservable, (endTime, partialUserAction) => { stopPageActivitiesTracking() stopEventCountsTracking() - if (endTime !== undefined) { + if (endTime !== undefined && partialUserAction !== undefined) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, - name, - startTime, - type, - duration: endTime - startTime, + duration: endTime - partialUserAction.startTime, measures: { errorCount: eventCounts.errorCount, longTaskCount: eventCounts.longTaskCount, resourceCount: eventCounts.resourceCount, }, + name: partialUserAction.name, + startTime: partialUserAction.startTime, + type: partialUserAction.type, }) } }) @@ -205,7 +220,7 @@ function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable, - completionCallback: (endTime: number | undefined) => void + completionCallback: (endTime: number | undefined, partialUserAction: PartialUserAction | undefined) => void ) { let idleTimeoutId: ReturnType let hasCompleted = false @@ -230,9 +245,8 @@ function waitUserActionCompletion( clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) clearTimeout(maxDurationTimeoutId) + completionCallback(endTime, currentUserAction) currentUserAction = undefined - currentEventCounts = undefined - completionCallback(endTime) } } @@ -241,7 +255,6 @@ export const $$tests = { trackPageActivities, resetUserAction() { currentUserAction = undefined - currentEventCounts = undefined }, waitUserActionCompletion, } From f8c40a38269871fdf5df5a5b0bf2e5fb69e4d86c Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 21 Apr 2020 15:37:38 +0200 Subject: [PATCH 04/42] :sparkles: clean currentUserAction = undefined process --- packages/rum/src/userActionCollection.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 4219145e31..12c94b62c0 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -131,23 +131,24 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string, const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - waitUserActionCompletion(pageActivitiesObservable, (endTime, partialUserAction) => { + waitUserActionCompletion(pageActivitiesObservable, (endTime) => { stopPageActivitiesTracking() stopEventCountsTracking() - if (endTime !== undefined && partialUserAction !== undefined) { + if (endTime !== undefined && currentUserAction !== undefined) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, - duration: endTime - partialUserAction.startTime, + duration: endTime - currentUserAction.startTime, measures: { errorCount: eventCounts.errorCount, longTaskCount: eventCounts.longTaskCount, resourceCount: eventCounts.resourceCount, }, - name: partialUserAction.name, - startTime: partialUserAction.startTime, - type: partialUserAction.type, + name: currentUserAction.name, + startTime: currentUserAction.startTime, + type: currentUserAction.type, }) } + currentUserAction = undefined }) } @@ -220,7 +221,7 @@ function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable, - completionCallback: (endTime: number | undefined, partialUserAction: PartialUserAction | undefined) => void + completionCallback: (endTime: number | undefined) => void ) { let idleTimeoutId: ReturnType let hasCompleted = false @@ -245,8 +246,7 @@ function waitUserActionCompletion( clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) clearTimeout(maxDurationTimeoutId) - completionCallback(endTime, currentUserAction) - currentUserAction = undefined + completionCallback(endTime) } } From f99052ef33b2ce9a8324d98f47fd2a69af6e6594 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 29 Apr 2020 16:44:55 +0200 Subject: [PATCH 05/42] add view loading state in the UA lifecycle --- packages/rum/src/lifeCycle.ts | 5 ++- packages/rum/src/userActionCollection.ts | 55 ++++++++++++++++-------- packages/rum/src/viewCollection.ts | 5 +++ 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/packages/rum/src/lifeCycle.ts b/packages/rum/src/lifeCycle.ts index 23caac565b..9c47c49ba4 100644 --- a/packages/rum/src/lifeCycle.ts +++ b/packages/rum/src/lifeCycle.ts @@ -1,12 +1,13 @@ import { ErrorMessage, RequestCompleteEvent, RequestStartEvent } from '@datadog/browser-core' import { UserAction } from './userActionCollection' -import { View } from './viewCollection' +import { View, ViewLoad } from './viewCollection' export enum LifeCycleEventType { ERROR_COLLECTED, PERFORMANCE_ENTRY_COLLECTED, USER_ACTION_COLLECTED, VIEW_COLLECTED, + VIEW_LOAD_COMPLETED, REQUEST_STARTED, REQUEST_COMPLETED, SESSION_RENEWED, @@ -28,6 +29,7 @@ export class LifeCycle { notify(eventType: LifeCycleEventType.REQUEST_COMPLETED, data: RequestCompleteEvent): void notify(eventType: LifeCycleEventType.USER_ACTION_COLLECTED, data: UserAction): void notify(eventType: LifeCycleEventType.VIEW_COLLECTED, data: View): void + notify(eventType: LifeCycleEventType.VIEW_LOAD_COMPLETED, data: ViewLoad): void notify( eventType: | LifeCycleEventType.SESSION_RENEWED @@ -54,6 +56,7 @@ export class LifeCycle { ): Subscription subscribe(eventType: LifeCycleEventType.USER_ACTION_COLLECTED, callback: (data: UserAction) => void): Subscription subscribe(eventType: LifeCycleEventType.VIEW_COLLECTED, callback: (data: View) => void): Subscription + subscribe(eventType: LifeCycleEventType.VIEW_LOAD_COMPLETED, callback: (data: ViewLoad) => void): Subscription subscribe( eventType: | LifeCycleEventType.SESSION_RENEWED diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 12c94b62c0..f18a1cd1be 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -71,24 +71,20 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { let currentViewId: string | undefined addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) - subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processLoadView)) - function processClick(event: Event) { if (!(event.target instanceof Element)) { return } - newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } - function processLoadView(loadedView: View) { + subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processViewLoading)) + function processViewLoading(loadedView: View) { if (!loadedView || loadedView.id === currentViewId) { return } currentViewId = loadedView.id - - const name = `Load page ${loadedView.location.pathname}` - newUserAction(lifeCycle, UserActionType.LOAD_VIEW, name, loadedView.startTime) + newViewLoading(lifeCycle, loadedView.location.pathname, loadedView.startTime) } return { @@ -105,27 +101,48 @@ interface PartialUserAction { type: UserActionType name: string } - let currentUserAction: PartialUserAction | undefined -function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string, inputStartTime?: number) { - if (currentUserAction && (currentUserAction.type === UserActionType.LOAD_VIEW || type === UserActionType.CLICK)) { - // Discard any new click user action if another one is already occuring. - // Discard any new user action if a load_view is already occuring. - return +interface ViewLoadingState { + pathname: string + startTime: number + stopPageActivitiesTracking: () => void +} +let currentViewLoadingState: ViewLoadingState | undefined + +function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: number) { + if (currentViewLoadingState) { + currentViewLoadingState.stopPageActivitiesTracking() } - if (currentUserAction && currentUserAction.type === UserActionType.CLICK && type === UserActionType.LOAD_VIEW) { - currentUserAction.type = UserActionType.LOAD_VIEW - currentUserAction.name = name - if (inputStartTime) { - currentUserAction.startTime = Math.min(currentUserAction.startTime, inputStartTime) + const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) + currentViewLoadingState = { + pathname, + startTime, + stopPageActivitiesTracking, + } + + waitUserActionCompletion(pageActivitiesObservable, (endTime) => { + stopPageActivitiesTracking() + if (endTime !== undefined && currentViewLoadingState !== undefined) { + lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { + duration: endTime - currentViewLoadingState.startTime, + startTime: currentViewLoadingState.startTime, + }) } + currentViewLoadingState = undefined + }) +} + +function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { + if (currentViewLoadingState || currentUserAction) { + // Discard any new click user action if another one is already occuring. + // Discard any new user action if page is in a loading state. return } const id = generateUUID() - const startTime = inputStartTime || performance.now() + const startTime = performance.now() currentUserAction = { id, startTime, type, name } const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 2d829eccfd..6308d4e351 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -14,6 +14,11 @@ export interface View { duration: number } +export interface ViewLoad { + startTime: number + duration: number +} + export interface ViewMeasures { firstContentfulPaint?: number domInteractive?: number From 6eeb910ca81e541285f6a139cf15cc77dcb6c6e7 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 29 Apr 2020 17:18:38 +0200 Subject: [PATCH 06/42] cancel current UA in case of load view --- packages/rum/src/userActionCollection.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index f18a1cd1be..67c1907852 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -111,6 +111,9 @@ interface ViewLoadingState { let currentViewLoadingState: ViewLoadingState | undefined function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: number) { + // Cancel current user action + currentUserAction = undefined + if (currentViewLoadingState) { currentViewLoadingState.stopPageActivitiesTracking() } From 4ade8fdbaa6488cd6368ec4b795359846ea7156d Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 30 Apr 2020 15:31:04 +0200 Subject: [PATCH 07/42] :sparkles: collect loadDuration --- packages/rum/src/userActionCollection.ts | 6 ++++-- packages/rum/src/viewCollection.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 67c1907852..a10c673435 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -127,9 +127,11 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe waitUserActionCompletion(pageActivitiesObservable, (endTime) => { stopPageActivitiesTracking() - if (endTime !== undefined && currentViewLoadingState !== undefined) { + if (currentViewLoadingState !== undefined) { + // Validation timeout completion does not return an end time + const loadingEndTime = endTime || performance.now() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { - duration: endTime - currentViewLoadingState.startTime, + duration: loadingEndTime - currentViewLoadingState.startTime, startTime: currentViewLoadingState.startTime, }) } diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 6308d4e351..839c8ed62a 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -12,6 +12,7 @@ export interface View { documentVersion: number startTime: number duration: number + loadDuration?: number } export interface ViewLoad { @@ -82,6 +83,7 @@ function newView( userActionCount: 0, } let documentVersion = 0 + let loadDuration: number viewContext = { id, location, sessionId: session.getId() } @@ -100,6 +102,12 @@ function newView( const { stop: stopTimingsTracking } = trackTimings(lifeCycle, updateMeasures) const { stop: stopEventCountsTracking } = trackEventCounts(lifeCycle, updateMeasures) + function updateLoadDuration(loadDurationValue: number) { + loadDuration = loadDurationValue + scheduleViewUpdate() + } + const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, updateLoadDuration) + // Initial view update updateView() @@ -108,6 +116,7 @@ function newView( lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { documentVersion, id, + loadDuration, location, measures, duration: performance.now() - startOrigin, @@ -180,3 +189,13 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void ) return { stop: stopPerformanceTracking } } + +function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { + const { unsubscribe: stopViewLoadTracking } = lifeCycle.subscribe( + LifeCycleEventType.VIEW_LOAD_COMPLETED, + (viewLoad: ViewLoad) => { + callback(viewLoad.duration) + } + ) + return { stop: stopViewLoadTracking } +} From 8c2847282f9b425d3e8c0be9ece9615364273c7a Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 30 Apr 2020 16:17:34 +0200 Subject: [PATCH 08/42] :sparkles: add view load type --- packages/rum/src/viewCollection.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 839c8ed62a..ec63feca5e 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -13,6 +13,7 @@ export interface View { startTime: number duration: number loadDuration?: number + loadType: ViewLoadType } export interface ViewLoad { @@ -32,26 +33,33 @@ export interface ViewMeasures { userActionCount: number } +export enum ViewLoadType { + HARD = 'hard', + SOFT = 'soft', +} + export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export function startViewCollection(location: Location, lifeCycle: LifeCycle, session: RumSession) { let currentLocation = { ...location } const startOrigin = 0 - let currentView = newView(lifeCycle, currentLocation, session, startOrigin) + let currentViewLoadType: ViewLoadType = ViewLoadType.HARD + let currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType, startOrigin) // Renew view on history changes trackHistory(() => { + currentViewLoadType = ViewLoadType.SOFT if (areDifferentViews(currentLocation, location)) { currentLocation = { ...location } currentView.end() - currentView = newView(lifeCycle, currentLocation, session) + currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType) } }) // Renew view on session renewal lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { currentView.end() - currentView = newView(lifeCycle, currentLocation, session) + currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType) }) // End the current view on page unload @@ -72,6 +80,7 @@ function newView( lifeCycle: LifeCycle, location: Location, session: RumSession, + loadType: ViewLoadType, startOrigin: number = performance.now() ) { // Setup initial values @@ -117,6 +126,7 @@ function newView( documentVersion, id, loadDuration, + loadType, location, measures, duration: performance.now() - startOrigin, From 3dff2fd62cb05dec251a9dc4ec9556b61cb5e053 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 4 May 2020 11:15:23 +0200 Subject: [PATCH 09/42] :white_check_mark: add soft, hard load tests --- packages/rum/src/viewCollection.ts | 3 ++- packages/rum/test/viewCollection.spec.ts | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index ec63feca5e..5b1d71636a 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -72,6 +72,7 @@ interface ViewContext { id: string location: Location sessionId: string | undefined + loadType: ViewLoadType } export let viewContext: ViewContext @@ -94,7 +95,7 @@ function newView( let documentVersion = 0 let loadDuration: number - viewContext = { id, location, sessionId: session.getId() } + viewContext = { id, location, loadType, sessionId: session.getId() } // Update the view every time the measures are changing const { throttled: scheduleViewUpdate, stop: stopScheduleViewUpdate } = throttle( diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 096f9e8aed..ca8e2509c7 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -4,7 +4,13 @@ import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PerformanceLongTaskTiming, PerformancePaintTiming } from '../src/rum' import { RumSession } from '../src/rumSession' import { UserAction, UserActionType } from '../src/userActionCollection' -import { startViewCollection, THROTTLE_VIEW_UPDATE_PERIOD, View, viewContext } from '../src/viewCollection' +import { + startViewCollection, + THROTTLE_VIEW_UPDATE_PERIOD, + View, + viewContext, + ViewLoadType, +} from '../src/viewCollection' function setup(lifeCycle: LifeCycle = new LifeCycle()) { spyOn(history, 'pushState').and.callFake((_: any, __: string, pathname: string) => { @@ -32,11 +38,16 @@ describe('rum track url change', () => { initialLocation = viewContext.location }) - it('should update view id on path change', () => { + it('should be a hard load before any path change', () => { + expect(viewContext.loadType).toEqual(ViewLoadType.HARD) + }) + + it('should update view id and be a soft load on path change', () => { history.pushState({}, '', '/bar') expect(viewContext.id).not.toEqual(initialView) expect(viewContext.location).not.toEqual(initialLocation) + expect(viewContext.loadType).toEqual(ViewLoadType.SOFT) }) it('should not update view id on search change', () => { From 52d880b01955410095bd1bde6440f2eff86e5f95 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 4 May 2020 12:17:46 +0200 Subject: [PATCH 10/42] :white_check_mark: add load duration View update subscription --- packages/rum/test/viewCollection.spec.ts | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index ca8e2509c7..22219a50f7 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -110,6 +110,32 @@ describe('rum track renew session', () => { }) }) +describe('rum track load duration', () => { + let lifeCycle: LifeCycle + let getViewEvent: (index: number) => View + beforeEach(() => { + ;({ lifeCycle, getViewEvent } = spyOnViews()) + }) + + it('should not have loadDuration before being notified', () => { + expect(getViewEvent(0).loadDuration).toBeUndefined() + }) + + it('should have loadDuration once notified', () => { + jasmine.clock().install() + const loadingEndTime = performance.now() + lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { + duration: loadingEndTime, + startTime: 0, + }) + jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewEvent(1).loadDuration).toEqual(loadingEndTime) + + jasmine.clock().uninstall() + }) +}) + describe('rum view measures', () => { const FAKE_LONG_TASK = { entryType: 'longtask', From 2278761a3eb1bd21c45a35ad78e2711615b05d37 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 4 May 2020 14:01:24 +0200 Subject: [PATCH 11/42] rename load types --- packages/rum/src/viewCollection.ts | 8 ++++---- packages/rum/test/viewCollection.spec.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 5b1d71636a..fb8d7ce96b 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -34,8 +34,8 @@ export interface ViewMeasures { } export enum ViewLoadType { - HARD = 'hard', - SOFT = 'soft', + INITIAL_PAGE_LOAD = 'initial page load', + ROUTE_CHANGE = 'route change', } export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 @@ -43,12 +43,12 @@ export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export function startViewCollection(location: Location, lifeCycle: LifeCycle, session: RumSession) { let currentLocation = { ...location } const startOrigin = 0 - let currentViewLoadType: ViewLoadType = ViewLoadType.HARD + let currentViewLoadType: ViewLoadType = ViewLoadType.INITIAL_PAGE_LOAD let currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType, startOrigin) // Renew view on history changes trackHistory(() => { - currentViewLoadType = ViewLoadType.SOFT + currentViewLoadType = ViewLoadType.ROUTE_CHANGE if (areDifferentViews(currentLocation, location)) { currentLocation = { ...location } currentView.end() diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 22219a50f7..686897daf8 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -39,7 +39,7 @@ describe('rum track url change', () => { }) it('should be a hard load before any path change', () => { - expect(viewContext.loadType).toEqual(ViewLoadType.HARD) + expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_PAGE_LOAD) }) it('should update view id and be a soft load on path change', () => { @@ -47,7 +47,7 @@ describe('rum track url change', () => { expect(viewContext.id).not.toEqual(initialView) expect(viewContext.location).not.toEqual(initialLocation) - expect(viewContext.loadType).toEqual(ViewLoadType.SOFT) + expect(viewContext.loadType).toEqual(ViewLoadType.ROUTE_CHANGE) }) it('should not update view id on search change', () => { From 53d468d3c433cfc6517365ebc3ed2eeddfd7c21b Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 4 May 2020 15:13:30 +0200 Subject: [PATCH 12/42] :white_check_mark: add UA vs view load tests --- .../rum/test/userActionCollection.spec.ts | 63 ++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index e5605fbef2..433c1a4012 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,4 +1,12 @@ -import { DOM_EVENT, ErrorMessage, Observable, RequestCompleteEvent } from '@datadog/browser-core' +import { + DOM_EVENT, + ErrorMessage, + getHash, + getPathName, + getSearch, + Observable, + RequestCompleteEvent, +} from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { $$tests, @@ -13,11 +21,14 @@ import { UserActionType, } from '../src/userActionCollection' const { waitUserActionCompletion, trackPageActivities, resetUserAction, newUserAction } = $$tests +import { View, ViewLoadType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_USER_ACTION_VALIDATION_DELAY = USER_ACTION_VALIDATION_DELAY * 0.8 // Used to wait some time before the (potential) end of a user action const BEFORE_USER_ACTION_END_DELAY = USER_ACTION_END_DELAY * 0.8 +// Used to wait some time after the (potential) end of a user action +const AFTER_USER_ACTION_END_DELAY = USER_ACTION_END_DELAY * 1.2 // Used to wait some time but it doesn't matter how much. const SOME_ARBITRARY_DELAY = 50 // A long delay used to wait after any user action is finished. @@ -81,7 +92,7 @@ describe('startUserActionCollection', () => { userActionCollectionSubscription.stop() }) - it('starts a user action when clicking on an element', () => { + function mockValidatedClickUserAction() { button.addEventListener(DOM_EVENT.CLICK, () => { clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) // Since we don't collect dom mutations for this test, manually dispatch one @@ -92,6 +103,54 @@ describe('startUserActionCollection', () => { button.click() clock.expire() + } + + it('cancels user action on view loading', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + id: 'foo', + location: fakeLocation as Location, + } + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + + mockValidatedClickUserAction() + + expect(events).toEqual([]) + }) + + it('starts a user action when clicking on an element after a view loading', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + id: 'foo', + location: fakeLocation as Location, + } + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + + // View loads are completed like a UA would have been completed when there is no activity for a given time + clock.tick(AFTER_USER_ACTION_END_DELAY) + clock.expire() + + mockValidatedClickUserAction() + expect(events).toEqual([ + { + duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + id: jasmine.any(String), + measures: { + errorCount: 0, + longTaskCount: 0, + resourceCount: 0, + }, + name: 'Click me', + startTime: jasmine.any(Number), + type: UserActionType.CLICK, + }, + ]) + }) + + it('starts a user action when clicking on an element', () => { + mockValidatedClickUserAction() expect(events).toEqual([ { duration: BEFORE_USER_ACTION_VALIDATION_DELAY, From 64c39ef63cb1da07561d4037ae936a2cc843cb0c Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 10:56:52 +0200 Subject: [PATCH 13/42] :ok_hand: v1 --- packages/rum/src/userActionCollection.ts | 12 +++++------- packages/rum/src/viewCollection.ts | 1 - packages/rum/test/userActionCollection.spec.ts | 1 - packages/rum/test/viewCollection.spec.ts | 1 - 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index a10c673435..4ed195aeb0 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -98,8 +98,6 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { interface PartialUserAction { id: string startTime: number - type: UserActionType - name: string } let currentUserAction: PartialUserAction | undefined @@ -132,7 +130,6 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe const loadingEndTime = endTime || performance.now() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { duration: loadingEndTime - currentViewLoadingState.startTime, - startTime: currentViewLoadingState.startTime, }) } currentViewLoadingState = undefined @@ -148,7 +145,8 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) const id = generateUUID() const startTime = performance.now() - currentUserAction = { id, startTime, type, name } + + currentUserAction = { id, startTime } const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) @@ -156,18 +154,18 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) waitUserActionCompletion(pageActivitiesObservable, (endTime) => { stopPageActivitiesTracking() stopEventCountsTracking() - if (endTime !== undefined && currentUserAction !== undefined) { + if (endTime !== undefined && currentUserAction !== undefined && currentUserAction.id === id) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, + name, + type, duration: endTime - currentUserAction.startTime, measures: { errorCount: eventCounts.errorCount, longTaskCount: eventCounts.longTaskCount, resourceCount: eventCounts.resourceCount, }, - name: currentUserAction.name, startTime: currentUserAction.startTime, - type: currentUserAction.type, }) } currentUserAction = undefined diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index fb8d7ce96b..d0478f4d06 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -17,7 +17,6 @@ export interface View { } export interface ViewLoad { - startTime: number duration: number } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 433c1a4012..948e20da33 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -130,7 +130,6 @@ describe('startUserActionCollection', () => { // View loads are completed like a UA would have been completed when there is no activity for a given time clock.tick(AFTER_USER_ACTION_END_DELAY) - clock.expire() mockValidatedClickUserAction() expect(events).toEqual([ diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 686897daf8..2c3c912f79 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -126,7 +126,6 @@ describe('rum track load duration', () => { const loadingEndTime = performance.now() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { duration: loadingEndTime, - startTime: 0, }) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) From 86a9b74baaa936a2e85753ae8afe9ba41689f5bf Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 11:55:22 +0200 Subject: [PATCH 14/42] :truck: create the new trackPageActivities module and it generic waitIdlePageActivity function --- packages/rum/src/trackPageActivities.ts | 111 +++++++++++++++ packages/rum/src/userActionCollection.ts | 127 ++---------------- .../rum/test/userActionCollection.spec.ts | 33 +++-- 3 files changed, 143 insertions(+), 128 deletions(-) create mode 100644 packages/rum/src/trackPageActivities.ts diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts new file mode 100644 index 0000000000..f340e503ab --- /dev/null +++ b/packages/rum/src/trackPageActivities.ts @@ -0,0 +1,111 @@ +import { monitor, Observable } from '@datadog/browser-core' +import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' + +// Delay to wait for a page activity to validate the user action +export const USER_ACTION_VALIDATION_DELAY = 100 +// Delay to wait after a page activity to end the user action +export const USER_ACTION_END_DELAY = 100 +// Maximum duration of a user action +export const USER_ACTION_MAX_DURATION = 10_000 + +export interface PageActivityEvent { + isBusy: boolean +} + +export function waitIdlePageActivity( + lifeCycle: LifeCycle, + completionCallback: (endTime: number | undefined) => void +): () => void { + const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) + + waitPageActivitiesCompletion(pageActivitiesObservable, stopPageActivitiesTracking, (endTime) => { + completionCallback(endTime) + }) + + return stopPageActivitiesTracking +} + +export function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable; stop(): void } { + const observable = new Observable() + const subscriptions: Subscription[] = [] + let firstRequestId: undefined | number + let pendingRequestsCount = 0 + + subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.DOM_MUTATED, () => notifyPageActivity())) + + subscriptions.push( + lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => { + if (entry.entryType !== 'resource') { + return + } + + notifyPageActivity() + }) + ) + + subscriptions.push( + lifeCycle.subscribe(LifeCycleEventType.REQUEST_STARTED, (startEvent) => { + if (firstRequestId === undefined) { + firstRequestId = startEvent.requestId + } + + pendingRequestsCount += 1 + notifyPageActivity() + }) + ) + + subscriptions.push( + lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, (request) => { + // If the request started before the tracking start, ignore it + if (firstRequestId === undefined || request.requestId < firstRequestId) { + return + } + pendingRequestsCount -= 1 + notifyPageActivity() + }) + ) + + function notifyPageActivity() { + observable.notify({ isBusy: pendingRequestsCount > 0 }) + } + + return { + observable, + stop() { + subscriptions.forEach((s) => s.unsubscribe()) + }, + } +} + +export function waitPageActivitiesCompletion( + pageActivitiesObservable: Observable, + stopPageActivitiesTracking: () => void, + completionCallback: (endTime: number | undefined) => void +) { + let idleTimeoutId: ReturnType + let hasCompleted = false + + const validationTimeoutId = setTimeout(monitor(() => complete(undefined)), USER_ACTION_VALIDATION_DELAY) + const maxDurationTimeoutId = setTimeout(monitor(() => complete(performance.now())), USER_ACTION_MAX_DURATION) + + pageActivitiesObservable.subscribe(({ isBusy }) => { + clearTimeout(validationTimeoutId) + clearTimeout(idleTimeoutId) + const lastChangeTime = performance.now() + if (!isBusy) { + idleTimeoutId = setTimeout(monitor(() => complete(lastChangeTime)), USER_ACTION_END_DELAY) + } + }) + + function complete(endTime: number | undefined) { + if (hasCompleted) { + return + } + hasCompleted = true + clearTimeout(validationTimeoutId) + clearTimeout(idleTimeoutId) + clearTimeout(maxDurationTimeoutId) + stopPageActivitiesTracking() + completionCallback(endTime) + } +} diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 4ed195aeb0..b0b79483c5 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -1,7 +1,8 @@ import { Context, DOM_EVENT, generateUUID, monitor, Observable } from '@datadog/browser-core' import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' -import { EventCounts, trackEventCounts } from './trackEventCounts' +import { trackEventCounts } from './trackEventCounts' +import { waitIdlePageActivity, trackPageActivities, waitPageActivitiesCompletion } from './trackPageActivities' import { View } from './viewCollection' // Automatic user action collection lifecycle overview: @@ -30,16 +31,8 @@ import { View } from './viewCollection' // Note: because MAX_DURATION > VALIDATION_DELAY, we are sure that if the user action is still alive // after MAX_DURATION, it has been validated. -// Delay to wait for a page activity to validate the user action -export const USER_ACTION_VALIDATION_DELAY = 100 -// Delay to wait after a page activity to end the user action -export const USER_ACTION_END_DELAY = 100 -// Maximum duration of a user action -export const USER_ACTION_MAX_DURATION = 10_000 - export enum UserActionType { CLICK = 'click', - LOAD_VIEW = 'load_view', CUSTOM = 'custom', } @@ -55,8 +48,8 @@ interface CustomUserAction { context?: Context } -export interface AutoUserAction { - type: UserActionType.LOAD_VIEW | UserActionType.CLICK +export interface ClickUserAction { + type: UserActionType.CLICK id: string name: string startTime: number @@ -64,7 +57,7 @@ export interface AutoUserAction { measures: UserActionMeasures } -export type UserAction = CustomUserAction | AutoUserAction +export type UserAction = CustomUserAction | ClickUserAction export function startUserActionCollection(lifeCycle: LifeCycle) { const subscriptions: Subscription[] = [] @@ -116,15 +109,7 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe currentViewLoadingState.stopPageActivitiesTracking() } - const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) - currentViewLoadingState = { - pathname, - startTime, - stopPageActivitiesTracking, - } - - waitUserActionCompletion(pageActivitiesObservable, (endTime) => { - stopPageActivitiesTracking() + const stopPageActivitiesTracking = waitIdlePageActivity(lifeCycle, (endTime) => { if (currentViewLoadingState !== undefined) { // Validation timeout completion does not return an end time const loadingEndTime = endTime || performance.now() @@ -134,6 +119,12 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe } currentViewLoadingState = undefined }) + + currentViewLoadingState = { + pathname, + startTime, + stopPageActivitiesTracking, + } } function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { @@ -145,14 +136,11 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) const id = generateUUID() const startTime = performance.now() - currentUserAction = { id, startTime } - const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - waitUserActionCompletion(pageActivitiesObservable, (endTime) => { - stopPageActivitiesTracking() + waitIdlePageActivity(lifeCycle, (endTime) => { stopEventCountsTracking() if (endTime !== undefined && currentUserAction !== undefined && currentUserAction.id === id) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { @@ -183,98 +171,9 @@ export function getUserActionReference(time?: number): UserActionReference | und return { id: currentUserAction.id } } -export interface PageActivityEvent { - isBusy: boolean -} - -function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable; stop(): void } { - const observable = new Observable() - const subscriptions: Subscription[] = [] - let firstRequestId: undefined | number - let pendingRequestsCount = 0 - - subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.DOM_MUTATED, () => notifyPageActivity())) - - subscriptions.push( - lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => { - if (entry.entryType !== 'resource') { - return - } - - notifyPageActivity() - }) - ) - - subscriptions.push( - lifeCycle.subscribe(LifeCycleEventType.REQUEST_STARTED, (startEvent) => { - if (firstRequestId === undefined) { - firstRequestId = startEvent.requestId - } - - pendingRequestsCount += 1 - notifyPageActivity() - }) - ) - - subscriptions.push( - lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, (request) => { - // If the request started before the tracking start, ignore it - if (firstRequestId === undefined || request.requestId < firstRequestId) { - return - } - pendingRequestsCount -= 1 - notifyPageActivity() - }) - ) - - function notifyPageActivity() { - observable.notify({ isBusy: pendingRequestsCount > 0 }) - } - - return { - observable, - stop() { - subscriptions.forEach((s) => s.unsubscribe()) - }, - } -} - -function waitUserActionCompletion( - pageActivitiesObservable: Observable, - completionCallback: (endTime: number | undefined) => void -) { - let idleTimeoutId: ReturnType - let hasCompleted = false - - const validationTimeoutId = setTimeout(monitor(() => complete(undefined)), USER_ACTION_VALIDATION_DELAY) - const maxDurationTimeoutId = setTimeout(monitor(() => complete(performance.now())), USER_ACTION_MAX_DURATION) - - pageActivitiesObservable.subscribe(({ isBusy }) => { - clearTimeout(validationTimeoutId) - clearTimeout(idleTimeoutId) - const lastChangeTime = performance.now() - if (!isBusy) { - idleTimeoutId = setTimeout(monitor(() => complete(lastChangeTime)), USER_ACTION_END_DELAY) - } - }) - - function complete(endTime: number | undefined) { - if (hasCompleted) { - return - } - hasCompleted = true - clearTimeout(validationTimeoutId) - clearTimeout(idleTimeoutId) - clearTimeout(maxDurationTimeoutId) - completionCallback(endTime) - } -} - export const $$tests = { newUserAction, - trackPageActivities, resetUserAction() { currentUserAction = undefined }, - waitUserActionCompletion, } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 948e20da33..fca9fd72cf 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -4,23 +4,28 @@ import { getHash, getPathName, getSearch, + noop, Observable, RequestCompleteEvent, } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { - $$tests, - AutoUserAction, - getUserActionReference, PageActivityEvent, - startUserActionCollection, + trackPageActivities, USER_ACTION_END_DELAY, USER_ACTION_MAX_DURATION, USER_ACTION_VALIDATION_DELAY, + waitPageActivitiesCompletion, +} from '../src/trackPageActivities' +import { + $$tests, + ClickUserAction, + getUserActionReference, + startUserActionCollection, UserAction, UserActionType, } from '../src/userActionCollection' -const { waitUserActionCompletion, trackPageActivities, resetUserAction, newUserAction } = $$tests +const { resetUserAction, newUserAction } = $$tests import { View, ViewLoadType } from '../src/viewCollection' // Used to wait some time after the creation of a user action @@ -203,7 +208,7 @@ describe('getUserActionReference', () => { expect(getUserActionReference()).toBeUndefined() - const userAction = events[0] as AutoUserAction + const userAction = events[0] as ClickUserAction expect(userAction.id).toBe(userActionReference.id) }) @@ -260,7 +265,7 @@ describe('newUserAction', () => { lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, error as ErrorMessage) expect(events.length).toBe(1) - const userAction = events[0] as AutoUserAction + const userAction = events[0] as ClickUserAction expect(userAction.measures).toEqual({ errorCount: 2, longTaskCount: 0, @@ -361,11 +366,11 @@ describe('trackPagePageActivities', () => { }) }) -describe('waitUserActionCompletion', () => { +describe('waitPageActivitiesCompletion', () => { const clock = mockClock() it('should not collect an event that is not followed by page activity', (done) => { - waitUserActionCompletion(new Observable(), (endTime) => { + waitPageActivitiesCompletion(new Observable(), noop, (endTime) => { expect(endTime).toBeUndefined() done() }) @@ -377,7 +382,7 @@ describe('waitUserActionCompletion', () => { const activityObservable = new Observable() const startTime = performance.now() - waitUserActionCompletion(activityObservable, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { expect(endTime).toEqual(startTime + BEFORE_USER_ACTION_VALIDATION_DELAY) done() }) @@ -396,7 +401,7 @@ describe('waitUserActionCompletion', () => { // Extend the user action but stops before USER_ACTION_MAX_DURATION const extendCount = Math.floor(USER_ACTION_MAX_DURATION / BEFORE_USER_ACTION_END_DELAY - 1) - waitUserActionCompletion(activityObservable, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_USER_ACTION_END_DELAY) done() }) @@ -417,7 +422,7 @@ describe('waitUserActionCompletion', () => { // Extend the user action until it's more than USER_ACTION_MAX_DURATION const extendCount = Math.ceil(USER_ACTION_MAX_DURATION / BEFORE_USER_ACTION_END_DELAY + 1) - waitUserActionCompletion(activityObservable, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { expect(endTime).toBe(startTime + USER_ACTION_MAX_DURATION) stop = true done() @@ -436,7 +441,7 @@ describe('waitUserActionCompletion', () => { it('is extended while the page is busy', (done) => { const activityObservable = new Observable() const startTime = performance.now() - waitUserActionCompletion(activityObservable, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { expect(endTime).toBe(startTime + BEFORE_USER_ACTION_VALIDATION_DELAY + USER_ACTION_END_DELAY * 2) done() }) @@ -453,7 +458,7 @@ describe('waitUserActionCompletion', () => { it('expires is the page is busy for too long', (done) => { const activityObservable = new Observable() const startTime = performance.now() - waitUserActionCompletion(activityObservable, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { expect(endTime).toBe(startTime + USER_ACTION_MAX_DURATION) done() }) From 39146fa7dd0f3d18aeb5179c5970b17a73afdad9 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 14:00:48 +0200 Subject: [PATCH 15/42] :ok_hand: v2 --- packages/rum/src/trackPageActivities.ts | 4 +-- packages/rum/src/userActionCollection.ts | 8 ++--- packages/rum/src/viewCollection.ts | 1 + .../rum/test/userActionCollection.spec.ts | 36 ++++++++++++------- packages/rum/test/viewCollection.spec.ts | 11 +++--- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts index f340e503ab..14fe72da95 100644 --- a/packages/rum/src/trackPageActivities.ts +++ b/packages/rum/src/trackPageActivities.ts @@ -15,14 +15,14 @@ export interface PageActivityEvent { export function waitIdlePageActivity( lifeCycle: LifeCycle, completionCallback: (endTime: number | undefined) => void -): () => void { +): { stop(): void } { const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) waitPageActivitiesCompletion(pageActivitiesObservable, stopPageActivitiesTracking, (endTime) => { completionCallback(endTime) }) - return stopPageActivitiesTracking + return { stop: stopPageActivitiesTracking } } export function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable; stop(): void } { diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index b0b79483c5..1dbfd9e8ca 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -97,7 +97,7 @@ let currentUserAction: PartialUserAction | undefined interface ViewLoadingState { pathname: string startTime: number - stopPageActivitiesTracking: () => void + stopWaitIdlePageActivity: () => void } let currentViewLoadingState: ViewLoadingState | undefined @@ -106,10 +106,10 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe currentUserAction = undefined if (currentViewLoadingState) { - currentViewLoadingState.stopPageActivitiesTracking() + currentViewLoadingState.stopWaitIdlePageActivity() } - const stopPageActivitiesTracking = waitIdlePageActivity(lifeCycle, (endTime) => { + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { if (currentViewLoadingState !== undefined) { // Validation timeout completion does not return an end time const loadingEndTime = endTime || performance.now() @@ -123,7 +123,7 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe currentViewLoadingState = { pathname, startTime, - stopPageActivitiesTracking, + stopWaitIdlePageActivity, } } diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index d0478f4d06..6278472d05 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -138,6 +138,7 @@ function newView( end() { stopTimingsTracking() stopEventCountsTracking() + stopLoadDurationTracking() // prevent pending view updates execution stopScheduleViewUpdate() // Make a final view update diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index fca9fd72cf..39beb2c17b 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,13 +1,4 @@ -import { - DOM_EVENT, - ErrorMessage, - getHash, - getPathName, - getSearch, - noop, - Observable, - RequestCompleteEvent, -} from '@datadog/browser-core' +import { DOM_EVENT, ErrorMessage, noop, Observable, RequestCompleteEvent } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PageActivityEvent, @@ -26,7 +17,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { View, ViewLoadType } from '../src/viewCollection' +import { View } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_USER_ACTION_VALIDATION_DELAY = USER_ACTION_VALIDATION_DELAY * 0.8 @@ -110,7 +101,7 @@ describe('startUserActionCollection', () => { clock.expire() } - it('cancels user action on view loading', () => { + it('cancels new user action on view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { documentVersion: 0, @@ -124,6 +115,27 @@ describe('startUserActionCollection', () => { expect(events).toEqual([]) }) + it('cancels ongoing user action on view loading', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + id: 'foo', + location: fakeLocation as Location, + } + button.addEventListener(DOM_EVENT.CLICK, () => { + clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + // Since we don't collect dom mutations for this test, manually dispatch one + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + }) + + clock.tick(SOME_ARBITRARY_DELAY) + button.click() + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + clock.expire() + + expect(events).toEqual([]) + }) + it('starts a user action when clicking on an element after a view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 2c3c912f79..2faeb11893 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -38,11 +38,11 @@ describe('rum track url change', () => { initialLocation = viewContext.location }) - it('should be a hard load before any path change', () => { + it('should collect initial view type as "initial page load"', () => { expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_PAGE_LOAD) }) - it('should update view id and be a soft load on path change', () => { + it('should update view id and type on path change', () => { history.pushState({}, '', '/bar') expect(viewContext.id).not.toEqual(initialView) @@ -117,13 +117,10 @@ describe('rum track load duration', () => { ;({ lifeCycle, getViewEvent } = spyOnViews()) }) - it('should not have loadDuration before being notified', () => { - expect(getViewEvent(0).loadDuration).toBeUndefined() - }) - - it('should have loadDuration once notified', () => { + it('should set a loadDuration once the load is complete', () => { jasmine.clock().install() const loadingEndTime = performance.now() + expect(getViewEvent(0).loadDuration).toBeUndefined() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { duration: loadingEndTime, }) From 6b8d47b602f55a5a6c237da4e48a18f6a88763ca Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 14:58:52 +0200 Subject: [PATCH 16/42] :pencil: about UA lifecycle --- packages/rum/src/userActionCollection.ts | 48 ++++++++++++++----- .../rum/test/userActionCollection.spec.ts | 6 +-- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 1dbfd9e8ca..0e616cd660 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -1,26 +1,50 @@ -import { Context, DOM_EVENT, generateUUID, monitor, Observable } from '@datadog/browser-core' +import { Context, DOM_EVENT, generateUUID } from '@datadog/browser-core' import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' import { trackEventCounts } from './trackEventCounts' -import { waitIdlePageActivity, trackPageActivities, waitPageActivitiesCompletion } from './trackPageActivities' +import { waitIdlePageActivity } from './trackPageActivities' import { View } from './viewCollection' // Automatic user action collection lifecycle overview: -// -// (Start) -// .--------------'----------------------. -// v v +// (Start new view) ____,, +// .------------------'------------------. || +// v v || +// [Wait for a page activity ] [Wait for a maximum duration] || +// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] || +// / \ | || +// v v | || +// [No page activity] [Page activity] | || +// | |,-----------------------. | || +// | v | | || +// | [Wait for a page activity] | | ||-- NO USER ACTION COLLECTION +// | [ timeout: END_DELAY ] | | || +// | / \ | | || +// | v v | | || +// | [No page activity] [Page activity] | | || +// | | | | | || +// | | '------------' | || +// | | | || +// '------------. ,----------------------------------' || +// v || +// (View load complete) ____|| +// | +// | +// '-----------. +// v +// (Start new user action) +// .-------------------'--------------------. +// v v // [Wait for a page activity ] [Wait for a maximum duration] // [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] // / \ | -// v v | +// v v | // [No page activity] [Page activity] | // | |,----------------------. | // v v | | // (Discard) [Wait for a page activity] | | // [ timeout: END_DELAY ] | | // / \ | | -// v v | | +// v v | | // [No page activity] [Page activity] | | // | | | | // | '------------' | @@ -48,7 +72,7 @@ interface CustomUserAction { context?: Context } -export interface ClickUserAction { +export interface AutoUserAction { type: UserActionType.CLICK id: string name: string @@ -57,7 +81,7 @@ export interface ClickUserAction { measures: UserActionMeasures } -export type UserAction = CustomUserAction | ClickUserAction +export type UserAction = CustomUserAction | AutoUserAction export function startUserActionCollection(lifeCycle: LifeCycle) { const subscriptions: Subscription[] = [] @@ -88,11 +112,11 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { } } -interface PartialUserAction { +interface PendingAutoUserAction { id: string startTime: number } -let currentUserAction: PartialUserAction | undefined +let currentUserAction: PendingAutoUserAction | undefined interface ViewLoadingState { pathname: string diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 39beb2c17b..495649375a 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -10,7 +10,7 @@ import { } from '../src/trackPageActivities' import { $$tests, - ClickUserAction, + AutoUserAction, getUserActionReference, startUserActionCollection, UserAction, @@ -220,7 +220,7 @@ describe('getUserActionReference', () => { expect(getUserActionReference()).toBeUndefined() - const userAction = events[0] as ClickUserAction + const userAction = events[0] as AutoUserAction expect(userAction.id).toBe(userActionReference.id) }) @@ -277,7 +277,7 @@ describe('newUserAction', () => { lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, error as ErrorMessage) expect(events.length).toBe(1) - const userAction = events[0] as ClickUserAction + const userAction = events[0] as AutoUserAction expect(userAction.measures).toEqual({ errorCount: 2, longTaskCount: 0, From 8a6994d6b86863cd51010f82c086b78bb9ca7dca Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 15:37:03 +0200 Subject: [PATCH 17/42] :white_check_mark: add view id checks --- packages/rum/src/userActionCollection.ts | 17 +++++----- packages/rum/src/viewCollection.ts | 9 +++-- .../rum/test/userActionCollection.spec.ts | 33 +++++++++++++++++++ packages/rum/test/viewCollection.spec.ts | 1 + 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 0e616cd660..5537b0213d 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -24,12 +24,12 @@ import { View } from './viewCollection' // | | | | | || // | | '------------' | || // | | | || -// '------------. ,----------------------------------' || -// v || -// (View load complete) ____|| -// | -// | -// '-----------. +// '-------------'----------. ,----------------------' || +// v || +// (View load complete) ____|| +// | +// | +// | // v // (Start new user action) // .-------------------'--------------------. @@ -101,7 +101,7 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { return } currentViewId = loadedView.id - newViewLoading(lifeCycle, loadedView.location.pathname, loadedView.startTime) + newViewLoading(lifeCycle, currentViewId, loadedView.location.pathname, loadedView.startTime) } return { @@ -125,7 +125,7 @@ interface ViewLoadingState { } let currentViewLoadingState: ViewLoadingState | undefined -function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: number) { +function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, startTime: number) { // Cancel current user action currentUserAction = undefined @@ -138,6 +138,7 @@ function newViewLoading(lifeCycle: LifeCycle, pathname: string, startTime: numbe // Validation timeout completion does not return an end time const loadingEndTime = endTime || performance.now() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { + id, duration: loadingEndTime - currentViewLoadingState.startTime, }) } diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 6278472d05..afdadfafb4 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -17,6 +17,7 @@ export interface View { } export interface ViewLoad { + id: string duration: number } @@ -115,7 +116,7 @@ function newView( loadDuration = loadDurationValue scheduleViewUpdate() } - const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, updateLoadDuration) + const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, id, updateLoadDuration) // Initial view update updateView() @@ -201,11 +202,13 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { +function trackLoadDuration(lifeCycle: LifeCycle, id: string, callback: (loadDurationValue: number) => void) { const { unsubscribe: stopViewLoadTracking } = lifeCycle.subscribe( LifeCycleEventType.VIEW_LOAD_COMPLETED, (viewLoad: ViewLoad) => { - callback(viewLoad.duration) + if (viewLoad.id === id) { + callback(viewLoad.duration) + } } ) return { stop: stopViewLoadTracking } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 495649375a..c751aafa14 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -165,6 +165,39 @@ describe('startUserActionCollection', () => { ]) }) + it('starts a user action when clicking on an element when the View is updated but has already been loaded', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + id: 'foo', + location: fakeLocation as Location, + } + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + + // View loads are completed like a UA would have been completed when there is no activity for a given time + clock.tick(AFTER_USER_ACTION_END_DELAY) + + // View is updated to documentVersion = 1 + mockView.documentVersion = 1 + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + mockValidatedClickUserAction() + + expect(events).toEqual([ + { + duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + id: jasmine.any(String), + measures: { + errorCount: 0, + longTaskCount: 0, + resourceCount: 0, + }, + name: 'Click me', + startTime: jasmine.any(Number), + type: UserActionType.CLICK, + }, + ]) + }) + it('starts a user action when clicking on an element', () => { mockValidatedClickUserAction() expect(events).toEqual([ diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 2faeb11893..87a2d0eb1c 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -123,6 +123,7 @@ describe('rum track load duration', () => { expect(getViewEvent(0).loadDuration).toBeUndefined() lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { duration: loadingEndTime, + id: viewContext.id, }) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) From bfa22eb53c4e73ebaab2479b83c7206d7fc1f439 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 5 May 2020 15:42:46 +0200 Subject: [PATCH 18/42] :white_check_mark: add test when missing id --- .../rum/test/userActionCollection.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index c751aafa14..af02844ba1 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -165,6 +165,32 @@ describe('startUserActionCollection', () => { ]) }) + it('starts a user action when clicking on an element when the View does not have an id', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + location: fakeLocation as Location, + } + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + + mockValidatedClickUserAction() + + expect(events).toEqual([ + { + duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + id: jasmine.any(String), + measures: { + errorCount: 0, + longTaskCount: 0, + resourceCount: 0, + }, + name: 'Click me', + startTime: jasmine.any(Number), + type: UserActionType.CLICK, + }, + ]) + }) + it('starts a user action when clicking on an element when the View is updated but has already been loaded', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { From c1162c47c50fa4eafc4c59b21c8251794fd2c4d7 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 6 May 2020 09:46:05 +0200 Subject: [PATCH 19/42] :sparkles: Collect new UA when the page is loading --- packages/rum/src/userActionCollection.ts | 45 +++++++++---------- .../rum/test/userActionCollection.spec.ts | 41 +++++++++++------ 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 5537b0213d..48e3ace44f 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -6,27 +6,27 @@ import { waitIdlePageActivity } from './trackPageActivities' import { View } from './viewCollection' // Automatic user action collection lifecycle overview: -// (Start new view) ____,, -// .------------------'------------------. || -// v v || -// [Wait for a page activity ] [Wait for a maximum duration] || -// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] || -// / \ | || -// v v | || -// [No page activity] [Page activity] | || -// | |,-----------------------. | || -// | v | | || -// | [Wait for a page activity] | | ||-- NO USER ACTION COLLECTION -// | [ timeout: END_DELAY ] | | || -// | / \ | | || -// | v v | | || -// | [No page activity] [Page activity] | | || -// | | | | | || -// | | '------------' | || -// | | | || -// '-------------'----------. ,----------------------' || -// v || -// (View load complete) ____|| +// (Start new view) +// .------------------'------------------. +// v v +// [Wait for a page activity ] [Wait for a maximum duration] +// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] +// / \ | +// v v | +// [No page activity] [Page activity] | +// | |,-----------------------. | +// | v | | +// | [Wait for a page activity] | | +// | [ timeout: END_DELAY ] | | +// | / \ | | +// | v v | | +// | [No page activity] [Page activity] | | +// | | | | | +// | | '------------' | +// | | | +// '-------------'----------. ,----------------------' +// v +// (View load complete) // | // | // | @@ -153,9 +153,8 @@ function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, star } function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { - if (currentViewLoadingState || currentUserAction) { + if (currentUserAction) { // Discard any new click user action if another one is already occuring. - // Discard any new user action if page is in a loading state. return } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index af02844ba1..0063c63fef 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -101,20 +101,6 @@ describe('startUserActionCollection', () => { clock.expire() } - it('cancels new user action on view loading', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - id: 'foo', - location: fakeLocation as Location, - } - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - - mockValidatedClickUserAction() - - expect(events).toEqual([]) - }) - it('cancels ongoing user action on view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { @@ -136,6 +122,33 @@ describe('startUserActionCollection', () => { expect(events).toEqual([]) }) + it('starts a user action when clicking on an element during a view loading', () => { + const fakeLocation: Partial = { pathname: '/foo' } + const mockView: Partial = { + documentVersion: 0, + id: 'foo', + location: fakeLocation as Location, + } + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + + mockValidatedClickUserAction() + + expect(events).toEqual([ + { + duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + id: jasmine.any(String), + measures: { + errorCount: 0, + longTaskCount: 0, + resourceCount: 0, + }, + name: 'Click me', + startTime: jasmine.any(Number), + type: UserActionType.CLICK, + }, + ]) + }) + it('starts a user action when clicking on an element after a view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { From f49c3775ea624df52588815d5d30f9e8163e6dcb Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 6 May 2020 16:12:10 +0200 Subject: [PATCH 20/42] rename load types --- packages/rum/src/viewCollection.ts | 4 ++-- packages/rum/test/viewCollection.spec.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index afdadfafb4..d759f4e0aa 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -34,7 +34,7 @@ export interface ViewMeasures { } export enum ViewLoadType { - INITIAL_PAGE_LOAD = 'initial page load', + INITIAL_LOAD = 'initial load', ROUTE_CHANGE = 'route change', } @@ -43,7 +43,7 @@ export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export function startViewCollection(location: Location, lifeCycle: LifeCycle, session: RumSession) { let currentLocation = { ...location } const startOrigin = 0 - let currentViewLoadType: ViewLoadType = ViewLoadType.INITIAL_PAGE_LOAD + let currentViewLoadType: ViewLoadType = ViewLoadType.INITIAL_LOAD let currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType, startOrigin) // Renew view on history changes diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 87a2d0eb1c..a6b6fcee46 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -39,7 +39,7 @@ describe('rum track url change', () => { }) it('should collect initial view type as "initial page load"', () => { - expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_PAGE_LOAD) + expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_LOAD) }) it('should update view id and type on path change', () => { From 35e4ed5de590eb1455babe1285d1ff54c3eff7f9 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 7 May 2020 18:14:31 +0200 Subject: [PATCH 21/42] :ok_hand: add trackPageActivities.spec.ts test file --- packages/rum/src/trackPageActivities.ts | 33 ++-- packages/rum/test/trackPageActivities.spec.ts | 108 +++++++++++++ .../rum/test/userActionCollection.spec.ts | 151 +----------------- 3 files changed, 133 insertions(+), 159 deletions(-) create mode 100644 packages/rum/test/trackPageActivities.spec.ts diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts index 14fe72da95..a6a76e7836 100644 --- a/packages/rum/src/trackPageActivities.ts +++ b/packages/rum/src/trackPageActivities.ts @@ -18,11 +18,20 @@ export function waitIdlePageActivity( ): { stop(): void } { const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) - waitPageActivitiesCompletion(pageActivitiesObservable, stopPageActivitiesTracking, (endTime) => { - completionCallback(endTime) - }) + const { stop: stopWaitPageActivitiesCompletion } = waitPageActivitiesCompletion( + pageActivitiesObservable, + stopPageActivitiesTracking, + (endTime) => { + completionCallback(endTime) + } + ) + + function stop() { + stopWaitPageActivitiesCompletion() + stopPageActivitiesTracking() + } - return { stop: stopPageActivitiesTracking } + return { stop } } export function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable; stop(): void } { @@ -81,7 +90,7 @@ export function waitPageActivitiesCompletion( pageActivitiesObservable: Observable, stopPageActivitiesTracking: () => void, completionCallback: (endTime: number | undefined) => void -) { +): { stop(): void } { let idleTimeoutId: ReturnType let hasCompleted = false @@ -97,15 +106,21 @@ export function waitPageActivitiesCompletion( } }) - function complete(endTime: number | undefined) { - if (hasCompleted) { - return - } + function stop() { hasCompleted = true clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) clearTimeout(maxDurationTimeoutId) stopPageActivitiesTracking() + } + + function complete(endTime: number | undefined) { + if (hasCompleted) { + return + } + stop() completionCallback(endTime) } + + return { stop } } diff --git a/packages/rum/test/trackPageActivities.spec.ts b/packages/rum/test/trackPageActivities.spec.ts new file mode 100644 index 0000000000..07c0ee45c1 --- /dev/null +++ b/packages/rum/test/trackPageActivities.spec.ts @@ -0,0 +1,108 @@ +import { RequestCompleteEvent } from '@datadog/browser-core' +import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' +import { PageActivityEvent, trackPageActivities } from '../src/trackPageActivities' + +function eventsCollector() { + const events: T[] = [] + beforeEach(() => { + events.length = 0 + }) + return { + events, + pushEvent(event: T) { + events.push(event) + }, + } +} + +describe('trackPagePageActivities', () => { + const { events, pushEvent } = eventsCollector() + it('emits an activity event on dom mutation', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + expect(events).toEqual([{ isBusy: false }]) + }) + + it('emits an activity event on resource collected', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + const performanceEntry = { + entryType: 'resource', + } + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) + expect(events).toEqual([{ isBusy: false }]) + }) + + it('does not emit an activity event when a navigation occurs', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + const performanceEntry = { + entryType: 'navigation', + } + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) + expect(events).toEqual([]) + }) + + it('stops emiting activities after calling stop()', () => { + const lifeCycle = new LifeCycle() + const { stop, observable } = trackPageActivities(lifeCycle) + observable.subscribe(pushEvent) + + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + expect(events).toEqual([{ isBusy: false }]) + + stop() + + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + + expect(events).toEqual([{ isBusy: false }]) + }) + + describe('requests', () => { + function makeFakeRequestCompleteEvent(requestId: number): RequestCompleteEvent { + return { requestId } as any + } + it('emits an activity event when a request starts', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { + requestId: 10, + }) + expect(events).toEqual([{ isBusy: true }]) + }) + + it('emits an activity event when a request completes', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { + requestId: 10, + }) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) + expect(events).toEqual([{ isBusy: true }, { isBusy: false }]) + }) + + it('ignores requests that has started before', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) + expect(events).toEqual([]) + }) + + it('keeps emiting busy events while all requests are not completed', () => { + const lifeCycle = new LifeCycle() + trackPageActivities(lifeCycle).observable.subscribe(pushEvent) + lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { + requestId: 10, + }) + lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { + requestId: 11, + }) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(9)) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(11)) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) + expect(events).toEqual([{ isBusy: true }, { isBusy: true }, { isBusy: true }, { isBusy: false }]) + }) + }) +}) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 0063c63fef..9e147ed106 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,8 +1,7 @@ -import { DOM_EVENT, ErrorMessage, noop, Observable, RequestCompleteEvent } from '@datadog/browser-core' +import { DOM_EVENT, ErrorMessage, noop, Observable } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PageActivityEvent, - trackPageActivities, USER_ACTION_END_DELAY, USER_ACTION_MAX_DURATION, USER_ACTION_VALIDATION_DELAY, @@ -122,62 +121,6 @@ describe('startUserActionCollection', () => { expect(events).toEqual([]) }) - it('starts a user action when clicking on an element during a view loading', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - id: 'foo', - location: fakeLocation as Location, - } - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - - mockValidatedClickUserAction() - - expect(events).toEqual([ - { - duration: BEFORE_USER_ACTION_VALIDATION_DELAY, - id: jasmine.any(String), - measures: { - errorCount: 0, - longTaskCount: 0, - resourceCount: 0, - }, - name: 'Click me', - startTime: jasmine.any(Number), - type: UserActionType.CLICK, - }, - ]) - }) - - it('starts a user action when clicking on an element after a view loading', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - id: 'foo', - location: fakeLocation as Location, - } - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - - // View loads are completed like a UA would have been completed when there is no activity for a given time - clock.tick(AFTER_USER_ACTION_END_DELAY) - - mockValidatedClickUserAction() - expect(events).toEqual([ - { - duration: BEFORE_USER_ACTION_VALIDATION_DELAY, - id: jasmine.any(String), - measures: { - errorCount: 0, - longTaskCount: 0, - resourceCount: 0, - }, - name: 'Click me', - startTime: jasmine.any(Number), - type: UserActionType.CLICK, - }, - ]) - }) - it('starts a user action when clicking on an element when the View does not have an id', () => { const fakeLocation: Partial = { pathname: '/foo' } const mockView: Partial = { @@ -358,98 +301,6 @@ describe('newUserAction', () => { }) }) -describe('trackPagePageActivities', () => { - const { events, pushEvent } = eventsCollector() - it('emits an activity event on dom mutation', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) - expect(events).toEqual([{ isBusy: false }]) - }) - - it('emits an activity event on resource collected', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - const performanceEntry = { - entryType: 'resource', - } - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) - expect(events).toEqual([{ isBusy: false }]) - }) - - it('does not emit an activity event when a navigation occurs', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - const performanceEntry = { - entryType: 'navigation', - } - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) - expect(events).toEqual([]) - }) - - it('stops emiting activities after calling stop()', () => { - const lifeCycle = new LifeCycle() - const { stop, observable } = trackPageActivities(lifeCycle) - observable.subscribe(pushEvent) - - lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) - expect(events).toEqual([{ isBusy: false }]) - - stop() - - lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) - lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) - - expect(events).toEqual([{ isBusy: false }]) - }) - - describe('requests', () => { - function makeFakeRequestCompleteEvent(requestId: number): RequestCompleteEvent { - return { requestId } as any - } - it('emits an activity event when a request starts', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { - requestId: 10, - }) - expect(events).toEqual([{ isBusy: true }]) - }) - - it('emits an activity event when a request completes', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { - requestId: 10, - }) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) - expect(events).toEqual([{ isBusy: true }, { isBusy: false }]) - }) - - it('ignores requests that has started before', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) - expect(events).toEqual([]) - }) - - it('keeps emiting busy events while all requests are not completed', () => { - const lifeCycle = new LifeCycle() - trackPageActivities(lifeCycle).observable.subscribe(pushEvent) - lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { - requestId: 10, - }) - lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, { - requestId: 11, - }) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(9)) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(11)) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, makeFakeRequestCompleteEvent(10)) - expect(events).toEqual([{ isBusy: true }, { isBusy: true }, { isBusy: true }, { isBusy: false }]) - }) - }) -}) - describe('waitPageActivitiesCompletion', () => { const clock = mockClock() From 00d5f00094ec4368bc4b7f17675cad2cf708a127 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 11:19:18 +0200 Subject: [PATCH 22/42] :ok_hand: add stop processes to newUserAction and newViewLoading --- packages/rum/src/trackPageActivities.ts | 18 ++--- packages/rum/src/userActionCollection.ts | 25 +++++-- .../rum/test/userActionCollection.spec.ts | 66 +++++++++---------- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts index a6a76e7836..3bc0b45d08 100644 --- a/packages/rum/src/trackPageActivities.ts +++ b/packages/rum/src/trackPageActivities.ts @@ -1,12 +1,12 @@ import { monitor, Observable } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' -// Delay to wait for a page activity to validate the user action -export const USER_ACTION_VALIDATION_DELAY = 100 -// Delay to wait after a page activity to end the user action -export const USER_ACTION_END_DELAY = 100 -// Maximum duration of a user action -export const USER_ACTION_MAX_DURATION = 10_000 +// Delay to wait for a page activity to validate the tracking process +export const PAGE_ACTIVITY_VALIDATION_DELAY = 100 +// Delay to wait after a page activity to end the tracking process +export const PAGE_ACTIVITY_END_DELAY = 100 +// Maximum duration of the tracking process +export const PAGE_ACTIVITY_MAX_DURATION = 10_000 export interface PageActivityEvent { isBusy: boolean @@ -94,15 +94,15 @@ export function waitPageActivitiesCompletion( let idleTimeoutId: ReturnType let hasCompleted = false - const validationTimeoutId = setTimeout(monitor(() => complete(undefined)), USER_ACTION_VALIDATION_DELAY) - const maxDurationTimeoutId = setTimeout(monitor(() => complete(performance.now())), USER_ACTION_MAX_DURATION) + const validationTimeoutId = setTimeout(monitor(() => complete(undefined)), PAGE_ACTIVITY_VALIDATION_DELAY) + const maxDurationTimeoutId = setTimeout(monitor(() => complete(performance.now())), PAGE_ACTIVITY_MAX_DURATION) pageActivitiesObservable.subscribe(({ isBusy }) => { clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) const lastChangeTime = performance.now() if (!isBusy) { - idleTimeoutId = setTimeout(monitor(() => complete(lastChangeTime)), USER_ACTION_END_DELAY) + idleTimeoutId = setTimeout(monitor(() => complete(lastChangeTime)), PAGE_ACTIVITY_END_DELAY) } }) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 48e3ace44f..9236c3e851 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -1,4 +1,4 @@ -import { Context, DOM_EVENT, generateUUID } from '@datadog/browser-core' +import { Context, DOM_EVENT, generateUUID, noop } from '@datadog/browser-core' import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' import { trackEventCounts } from './trackEventCounts' @@ -86,13 +86,15 @@ export type UserAction = CustomUserAction | AutoUserAction export function startUserActionCollection(lifeCycle: LifeCycle) { const subscriptions: Subscription[] = [] let currentViewId: string | undefined + let currentUserActionProcess = { stop: noop } + let currentViewLoadingProcess = { stop: noop } addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { if (!(event.target instanceof Element)) { return } - newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) + currentUserActionProcess = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processViewLoading)) @@ -101,11 +103,18 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { return } currentViewId = loadedView.id - newViewLoading(lifeCycle, currentViewId, loadedView.location.pathname, loadedView.startTime) + currentViewLoadingProcess = newViewLoading( + lifeCycle, + currentViewId, + loadedView.location.pathname, + loadedView.startTime + ) } return { stop() { + currentUserActionProcess.stop() + currentViewLoadingProcess.stop() removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) subscriptions.forEach((s) => s.unsubscribe()) }, @@ -125,7 +134,7 @@ interface ViewLoadingState { } let currentViewLoadingState: ViewLoadingState | undefined -function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, startTime: number) { +function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, startTime: number): { stop(): void } { // Cancel current user action currentUserAction = undefined @@ -150,12 +159,14 @@ function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, star startTime, stopWaitIdlePageActivity, } + + return { stop: stopWaitIdlePageActivity } } -function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { +function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { if (currentUserAction) { // Discard any new click user action if another one is already occuring. - return + return { stop: noop } } const id = generateUUID() @@ -182,6 +193,8 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) } currentUserAction = undefined }) + + return { stop: stopEventCountsTracking } } export interface UserActionReference { diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 9e147ed106..1ce8892b21 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -2,9 +2,9 @@ import { DOM_EVENT, ErrorMessage, noop, Observable } from '@datadog/browser-core import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PageActivityEvent, - USER_ACTION_END_DELAY, - USER_ACTION_MAX_DURATION, - USER_ACTION_VALIDATION_DELAY, + PAGE_ACTIVITY_END_DELAY, + PAGE_ACTIVITY_MAX_DURATION, + PAGE_ACTIVITY_VALIDATION_DELAY, waitPageActivitiesCompletion, } from '../src/trackPageActivities' import { @@ -19,15 +19,15 @@ const { resetUserAction, newUserAction } = $$tests import { View } from '../src/viewCollection' // Used to wait some time after the creation of a user action -const BEFORE_USER_ACTION_VALIDATION_DELAY = USER_ACTION_VALIDATION_DELAY * 0.8 +const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 // Used to wait some time before the (potential) end of a user action -const BEFORE_USER_ACTION_END_DELAY = USER_ACTION_END_DELAY * 0.8 +const BEFORE_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 0.8 // Used to wait some time after the (potential) end of a user action -const AFTER_USER_ACTION_END_DELAY = USER_ACTION_END_DELAY * 1.2 +const AFTER_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 1.2 // Used to wait some time but it doesn't matter how much. const SOME_ARBITRARY_DELAY = 50 // A long delay used to wait after any user action is finished. -const EXPIRE_DELAY = USER_ACTION_MAX_DURATION * 10 +const EXPIRE_DELAY = PAGE_ACTIVITY_MAX_DURATION * 10 function mockClock() { beforeEach(() => { @@ -89,7 +89,7 @@ describe('startUserActionCollection', () => { function mockValidatedClickUserAction() { button.addEventListener(DOM_EVENT.CLICK, () => { - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) // Since we don't collect dom mutations for this test, manually dispatch one lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) }) @@ -108,7 +108,7 @@ describe('startUserActionCollection', () => { location: fakeLocation as Location, } button.addEventListener(DOM_EVENT.CLICK, () => { - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) // Since we don't collect dom mutations for this test, manually dispatch one lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) }) @@ -133,7 +133,7 @@ describe('startUserActionCollection', () => { expect(events).toEqual([ { - duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + duration: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY, id: jasmine.any(String), measures: { errorCount: 0, @@ -157,7 +157,7 @@ describe('startUserActionCollection', () => { lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) // View loads are completed like a UA would have been completed when there is no activity for a given time - clock.tick(AFTER_USER_ACTION_END_DELAY) + clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) // View is updated to documentVersion = 1 mockView.documentVersion = 1 @@ -166,7 +166,7 @@ describe('startUserActionCollection', () => { expect(events).toEqual([ { - duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + duration: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY, id: jasmine.any(String), measures: { errorCount: 0, @@ -184,7 +184,7 @@ describe('startUserActionCollection', () => { mockValidatedClickUserAction() expect(events).toEqual([ { - duration: BEFORE_USER_ACTION_VALIDATION_DELAY, + duration: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY, id: jasmine.any(String), measures: { errorCount: 0, @@ -226,7 +226,7 @@ describe('getUserActionReference', () => { expect(userActionReference).toBeDefined() - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) expect(getUserActionReference()).toBeDefined() @@ -245,9 +245,9 @@ describe('getUserActionReference', () => { clock.tick(SOME_ARBITRARY_DELAY) newUserAction(new LifeCycle(), UserActionType.CLICK, 'test') - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY * 0.5) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY * 0.5) const timeAfterStartingUserAction = Date.now() - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY * 0.5) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY * 0.5) expect(getUserActionReference()).toBeDefined() expect(getUserActionReference(timeAfterStartingUserAction)).toBeDefined() @@ -268,7 +268,7 @@ describe('newUserAction', () => { newUserAction(lifeCycle, UserActionType.CLICK, 'test-1') newUserAction(lifeCycle, UserActionType.CLICK, 'test-2') - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) clock.expire() @@ -284,7 +284,7 @@ describe('newUserAction', () => { newUserAction(lifeCycle, UserActionType.CLICK, 'test-1') lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, error as ErrorMessage) - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, error as ErrorMessage) @@ -318,11 +318,11 @@ describe('waitPageActivitiesCompletion', () => { const startTime = performance.now() waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { - expect(endTime).toEqual(startTime + BEFORE_USER_ACTION_VALIDATION_DELAY) + expect(endTime).toEqual(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) done() }) - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: false }) clock.expire() @@ -333,16 +333,16 @@ describe('waitPageActivitiesCompletion', () => { const activityObservable = new Observable() const startTime = performance.now() - // Extend the user action but stops before USER_ACTION_MAX_DURATION - const extendCount = Math.floor(USER_ACTION_MAX_DURATION / BEFORE_USER_ACTION_END_DELAY - 1) + // Extend the user action but stops before PAGE_ACTIVITY_MAX_DURATION + const extendCount = Math.floor(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY - 1) waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { - expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_USER_ACTION_END_DELAY) + expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_PAGE_ACTIVITY_END_DELAY) done() }) for (let i = 0; i < extendCount; i += 1) { - clock.tick(BEFORE_USER_ACTION_END_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) activityObservable.notify({ isBusy: false }) } @@ -354,17 +354,17 @@ describe('waitPageActivitiesCompletion', () => { let stop = false const startTime = performance.now() - // Extend the user action until it's more than USER_ACTION_MAX_DURATION - const extendCount = Math.ceil(USER_ACTION_MAX_DURATION / BEFORE_USER_ACTION_END_DELAY + 1) + // Extend the user action until it's more than PAGE_ACTIVITY_MAX_DURATION + const extendCount = Math.ceil(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY + 1) waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { - expect(endTime).toBe(startTime + USER_ACTION_MAX_DURATION) + expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) stop = true done() }) for (let i = 0; i < extendCount && !stop; i += 1) { - clock.tick(BEFORE_USER_ACTION_END_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) activityObservable.notify({ isBusy: false }) } @@ -377,14 +377,14 @@ describe('waitPageActivitiesCompletion', () => { const activityObservable = new Observable() const startTime = performance.now() waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { - expect(endTime).toBe(startTime + BEFORE_USER_ACTION_VALIDATION_DELAY + USER_ACTION_END_DELAY * 2) + expect(endTime).toBe(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY + PAGE_ACTIVITY_END_DELAY * 2) done() }) - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: true }) - clock.tick(USER_ACTION_END_DELAY * 2) + clock.tick(PAGE_ACTIVITY_END_DELAY * 2) activityObservable.notify({ isBusy: false }) clock.expire() @@ -394,11 +394,11 @@ describe('waitPageActivitiesCompletion', () => { const activityObservable = new Observable() const startTime = performance.now() waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { - expect(endTime).toBe(startTime + USER_ACTION_MAX_DURATION) + expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) done() }) - clock.tick(BEFORE_USER_ACTION_VALIDATION_DELAY) + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: true }) clock.expire() From 12e185cd543c2eaa457ea4c0651d8021d963f2d4 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 15:01:22 +0200 Subject: [PATCH 23/42] :ok_hand: split load waitPageactivity and UA waitpageactivity --- packages/rum/src/lifeCycle.ts | 3 - packages/rum/src/userActionCollection.ts | 91 ++----------------- packages/rum/src/viewCollection.ts | 64 +++++++++++-- .../rum/test/userActionCollection.spec.ts | 66 +------------- packages/rum/test/viewCollection.spec.ts | 21 +++-- 5 files changed, 78 insertions(+), 167 deletions(-) diff --git a/packages/rum/src/lifeCycle.ts b/packages/rum/src/lifeCycle.ts index 9c47c49ba4..584c3a9cb3 100644 --- a/packages/rum/src/lifeCycle.ts +++ b/packages/rum/src/lifeCycle.ts @@ -7,7 +7,6 @@ export enum LifeCycleEventType { PERFORMANCE_ENTRY_COLLECTED, USER_ACTION_COLLECTED, VIEW_COLLECTED, - VIEW_LOAD_COMPLETED, REQUEST_STARTED, REQUEST_COMPLETED, SESSION_RENEWED, @@ -29,7 +28,6 @@ export class LifeCycle { notify(eventType: LifeCycleEventType.REQUEST_COMPLETED, data: RequestCompleteEvent): void notify(eventType: LifeCycleEventType.USER_ACTION_COLLECTED, data: UserAction): void notify(eventType: LifeCycleEventType.VIEW_COLLECTED, data: View): void - notify(eventType: LifeCycleEventType.VIEW_LOAD_COMPLETED, data: ViewLoad): void notify( eventType: | LifeCycleEventType.SESSION_RENEWED @@ -56,7 +54,6 @@ export class LifeCycle { ): Subscription subscribe(eventType: LifeCycleEventType.USER_ACTION_COLLECTED, callback: (data: UserAction) => void): Subscription subscribe(eventType: LifeCycleEventType.VIEW_COLLECTED, callback: (data: View) => void): Subscription - subscribe(eventType: LifeCycleEventType.VIEW_LOAD_COMPLETED, callback: (data: ViewLoad) => void): Subscription subscribe( eventType: | LifeCycleEventType.SESSION_RENEWED diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 9236c3e851..73aeda0737 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -3,34 +3,8 @@ import { getElementContent } from './getElementContent' import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' -import { View } from './viewCollection' // Automatic user action collection lifecycle overview: -// (Start new view) -// .------------------'------------------. -// v v -// [Wait for a page activity ] [Wait for a maximum duration] -// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] -// / \ | -// v v | -// [No page activity] [Page activity] | -// | |,-----------------------. | -// | v | | -// | [Wait for a page activity] | | -// | [ timeout: END_DELAY ] | | -// | / \ | | -// | v v | | -// | [No page activity] [Page activity] | | -// | | | | | -// | | '------------' | -// | | | -// '-------------'----------. ,----------------------' -// v -// (View load complete) -// | -// | -// | -// v // (Start new user action) // .-------------------'--------------------. // v v @@ -83,11 +57,10 @@ export interface AutoUserAction { export type UserAction = CustomUserAction | AutoUserAction +export let currentUserActionProcess = { stop: noop } + export function startUserActionCollection(lifeCycle: LifeCycle) { const subscriptions: Subscription[] = [] - let currentViewId: string | undefined - let currentUserActionProcess = { stop: noop } - let currentViewLoadingProcess = { stop: noop } addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { @@ -97,24 +70,9 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { currentUserActionProcess = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } - subscriptions.push(lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, processViewLoading)) - function processViewLoading(loadedView: View) { - if (!loadedView || loadedView.id === currentViewId) { - return - } - currentViewId = loadedView.id - currentViewLoadingProcess = newViewLoading( - lifeCycle, - currentViewId, - loadedView.location.pathname, - loadedView.startTime - ) - } - return { stop() { currentUserActionProcess.stop() - currentViewLoadingProcess.stop() removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) subscriptions.forEach((s) => s.unsubscribe()) }, @@ -127,46 +85,10 @@ interface PendingAutoUserAction { } let currentUserAction: PendingAutoUserAction | undefined -interface ViewLoadingState { - pathname: string - startTime: number - stopWaitIdlePageActivity: () => void -} -let currentViewLoadingState: ViewLoadingState | undefined - -function newViewLoading(lifeCycle: LifeCycle, id: string, pathname: string, startTime: number): { stop(): void } { - // Cancel current user action - currentUserAction = undefined - - if (currentViewLoadingState) { - currentViewLoadingState.stopWaitIdlePageActivity() - } - - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { - if (currentViewLoadingState !== undefined) { - // Validation timeout completion does not return an end time - const loadingEndTime = endTime || performance.now() - lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { - id, - duration: loadingEndTime - currentViewLoadingState.startTime, - }) - } - currentViewLoadingState = undefined - }) - - currentViewLoadingState = { - pathname, - startTime, - stopWaitIdlePageActivity, - } - - return { stop: stopWaitIdlePageActivity } -} - function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { if (currentUserAction) { // Discard any new click user action if another one is already occuring. - return { stop: noop } + return { stop: currentUserActionProcess.stop } } const id = generateUUID() @@ -194,7 +116,12 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) currentUserAction = undefined }) - return { stop: stopEventCountsTracking } + function stop() { + stopEventCountsTracking() + currentUserAction = undefined + } + + return { stop } } export interface UserActionReference { diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index d759f4e0aa..34f049da22 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -4,6 +4,8 @@ import { LifeCycle, LifeCycleEventType } from './lifeCycle' import { PerformancePaintTiming } from './rum' import { RumSession } from './rumSession' import { trackEventCounts } from './trackEventCounts' +import { waitIdlePageActivity } from './trackPageActivities' +import { currentUserActionProcess } from './userActionCollection' export interface View { id: string @@ -116,7 +118,7 @@ function newView( loadDuration = loadDurationValue scheduleViewUpdate() } - const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, id, updateLoadDuration) + const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, updateLoadDuration) // Initial view update updateView() @@ -202,14 +204,56 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -function trackLoadDuration(lifeCycle: LifeCycle, id: string, callback: (loadDurationValue: number) => void) { - const { unsubscribe: stopViewLoadTracking } = lifeCycle.subscribe( - LifeCycleEventType.VIEW_LOAD_COMPLETED, - (viewLoad: ViewLoad) => { - if (viewLoad.id === id) { - callback(viewLoad.duration) - } +// Automatic load view duration collection lifecycle overview: +// (Start new view) +// .------------------'------------------. +// v v +// [Wait for a page activity ] [Wait for a maximum duration] +// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] +// / \ | +// v v | +// [No page activity] [Page activity] | +// | |,-----------------------. | +// | v | | +// | [Wait for a page activity] | | +// | [ timeout: END_DELAY ] | | +// | / \ | | +// | v v | | +// | [No page activity] [Page activity] | | +// | | | | | +// | | '------------' | +// | | | +// '-------------'----------. ,----------------------' +// v +// (View load complete) + +interface ViewLoadingState { + stop: () => void +} +let currentViewLoadingState: ViewLoadingState | undefined + +export function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { + // Stop the current user action if there is one + currentUserActionProcess.stop() + + if (currentViewLoadingState) { + currentViewLoadingState.stop() + } + + const startTime: number = performance.now() + + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { + if (currentViewLoadingState !== undefined) { + // If no activity before the validation timeout completion does not return an end time + const loadingEndTime = endTime || performance.now() + callback(loadingEndTime - startTime) + currentViewLoadingState = undefined } - ) - return { stop: stopViewLoadTracking } + }) + + currentViewLoadingState = { + stop: stopWaitIdlePageActivity, + } + + return { stop: stopWaitIdlePageActivity } } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 1ce8892b21..e3bba9283b 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,10 +1,10 @@ import { DOM_EVENT, ErrorMessage, noop, Observable } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { - PageActivityEvent, PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, PAGE_ACTIVITY_VALIDATION_DELAY, + PageActivityEvent, waitPageActivitiesCompletion, } from '../src/trackPageActivities' import { @@ -16,7 +16,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { View } from '../src/viewCollection' +import { trackLoadDuration, View } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -115,71 +115,13 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) + function updateLoadDuration(loadDurationValue: number) {} + trackLoadDuration(lifeCycle, updateLoadDuration) clock.expire() expect(events).toEqual([]) }) - it('starts a user action when clicking on an element when the View does not have an id', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - location: fakeLocation as Location, - } - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - - mockValidatedClickUserAction() - - expect(events).toEqual([ - { - duration: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY, - id: jasmine.any(String), - measures: { - errorCount: 0, - longTaskCount: 0, - resourceCount: 0, - }, - name: 'Click me', - startTime: jasmine.any(Number), - type: UserActionType.CLICK, - }, - ]) - }) - - it('starts a user action when clicking on an element when the View is updated but has already been loaded', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - id: 'foo', - location: fakeLocation as Location, - } - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - - // View loads are completed like a UA would have been completed when there is no activity for a given time - clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) - - // View is updated to documentVersion = 1 - mockView.documentVersion = 1 - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, mockView as View) - mockValidatedClickUserAction() - - expect(events).toEqual([ - { - duration: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY, - id: jasmine.any(String), - measures: { - errorCount: 0, - longTaskCount: 0, - resourceCount: 0, - }, - name: 'Click me', - startTime: jasmine.any(Number), - type: UserActionType.CLICK, - }, - ]) - }) - it('starts a user action when clicking on an element', () => { mockValidatedClickUserAction() expect(events).toEqual([ diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index a6b6fcee46..b5065700ad 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -12,6 +12,10 @@ import { ViewLoadType, } from '../src/viewCollection' +import { PAGE_ACTIVITY_MAX_DURATION } from '../src/trackPageActivities' + +const AFTER_PAGE_ACTIVITY_MAX_DURATION = PAGE_ACTIVITY_MAX_DURATION * 1.1 + function setup(lifeCycle: LifeCycle = new LifeCycle()) { spyOn(history, 'pushState').and.callFake((_: any, __: string, pathname: string) => { const url = `http://localhost${pathname}` @@ -114,22 +118,19 @@ describe('rum track load duration', () => { let lifeCycle: LifeCycle let getViewEvent: (index: number) => View beforeEach(() => { + jasmine.clock().install() ;({ lifeCycle, getViewEvent } = spyOnViews()) }) + afterEach(() => { + jasmine.clock().uninstall() + }) + it('should set a loadDuration once the load is complete', () => { - jasmine.clock().install() - const loadingEndTime = performance.now() - expect(getViewEvent(0).loadDuration).toBeUndefined() - lifeCycle.notify(LifeCycleEventType.VIEW_LOAD_COMPLETED, { - duration: loadingEndTime, - id: viewContext.id, - }) + jasmine.clock().tick(AFTER_PAGE_ACTIVITY_MAX_DURATION) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadDuration).toEqual(loadingEndTime) - - jasmine.clock().uninstall() + expect(getViewEvent(1).loadDuration).toBeDefined() }) }) From da8801b6fbc125379eeeeb541310994fbd9442fe Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 15:12:23 +0200 Subject: [PATCH 24/42] :ok_hand: move lifecycle doc --- packages/rum/src/trackPageActivities.ts | 24 ++++++++++++++++++++++ packages/rum/src/userActionCollection.ts | 25 ----------------------- packages/rum/src/viewCollection.ts | 26 ++---------------------- 3 files changed, 26 insertions(+), 49 deletions(-) diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts index 3bc0b45d08..4987a8169c 100644 --- a/packages/rum/src/trackPageActivities.ts +++ b/packages/rum/src/trackPageActivities.ts @@ -34,6 +34,30 @@ export function waitIdlePageActivity( return { stop } } +// Automatic user action collection lifecycle overview: +// (Start new trackPageActivities) +// .-------------------'--------------------. +// v v +// [Wait for a page activity ] [Wait for a maximum duration] +// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] +// / \ | +// v v | +// [No page activity] [Page activity] | +// | |,----------------------. | +// v v | | +// (Discard) [Wait for a page activity] | | +// [ timeout: END_DELAY ] | | +// / \ | | +// v v | | +// [No page activity] [Page activity] | | +// | | | | +// | '------------' | +// '-----------. ,--------------------' +// v +// (End) +// +// Note: because MAX_DURATION > VALIDATION_DELAY, we are sure that if the process is still alive +// after MAX_DURATION, it has been validated. export function trackPageActivities(lifeCycle: LifeCycle): { observable: Observable; stop(): void } { const observable = new Observable() const subscriptions: Subscription[] = [] diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 73aeda0737..70f3c324e6 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -4,31 +4,6 @@ import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' -// Automatic user action collection lifecycle overview: -// (Start new user action) -// .-------------------'--------------------. -// v v -// [Wait for a page activity ] [Wait for a maximum duration] -// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] -// / \ | -// v v | -// [No page activity] [Page activity] | -// | |,----------------------. | -// v v | | -// (Discard) [Wait for a page activity] | | -// [ timeout: END_DELAY ] | | -// / \ | | -// v v | | -// [No page activity] [Page activity] | | -// | | | | -// | '------------' | -// '-----------. ,--------------------' -// v -// (End) -// -// Note: because MAX_DURATION > VALIDATION_DELAY, we are sure that if the user action is still alive -// after MAX_DURATION, it has been validated. - export enum UserActionType { CLICK = 'click', CUSTOM = 'custom', diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 34f049da22..d79e419722 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -204,29 +204,6 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -// Automatic load view duration collection lifecycle overview: -// (Start new view) -// .------------------'------------------. -// v v -// [Wait for a page activity ] [Wait for a maximum duration] -// [timeout: VALIDATION_DELAY] [ timeout: MAX_DURATION ] -// / \ | -// v v | -// [No page activity] [Page activity] | -// | |,-----------------------. | -// | v | | -// | [Wait for a page activity] | | -// | [ timeout: END_DELAY ] | | -// | / \ | | -// | v v | | -// | [No page activity] [Page activity] | | -// | | | | | -// | | '------------' | -// | | | -// '-------------'----------. ,----------------------' -// v -// (View load complete) - interface ViewLoadingState { stop: () => void } @@ -244,7 +221,8 @@ export function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationV const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { if (currentViewLoadingState !== undefined) { - // If no activity before the validation timeout completion does not return an end time + // If there is no activity during waitIdlePageActivity validation timeout, + // it will not return an end time but the view is loaded as no activity has occured const loadingEndTime = endTime || performance.now() callback(loadingEndTime - startTime) currentViewLoadingState = undefined From 02f1c74d2de5366fe739aac5ab9b65e950cb10a0 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 15:17:28 +0200 Subject: [PATCH 25/42] :ok_hand: rm unused exports --- packages/rum/src/lifeCycle.ts | 2 +- packages/rum/src/viewCollection.ts | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/rum/src/lifeCycle.ts b/packages/rum/src/lifeCycle.ts index 584c3a9cb3..23caac565b 100644 --- a/packages/rum/src/lifeCycle.ts +++ b/packages/rum/src/lifeCycle.ts @@ -1,6 +1,6 @@ import { ErrorMessage, RequestCompleteEvent, RequestStartEvent } from '@datadog/browser-core' import { UserAction } from './userActionCollection' -import { View, ViewLoad } from './viewCollection' +import { View } from './viewCollection' export enum LifeCycleEventType { ERROR_COLLECTED, diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index d79e419722..7fe6f1bc11 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -18,11 +18,6 @@ export interface View { loadType: ViewLoadType } -export interface ViewLoad { - id: string - duration: number -} - export interface ViewMeasures { firstContentfulPaint?: number domInteractive?: number From d8779f0de830e72d5bebdad00614e98badafd40f Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 16:30:20 +0200 Subject: [PATCH 26/42] :ok_hand: refactor stopCurrentUserAction process --- packages/rum/src/userActionCollection.ts | 30 ++++++++++++++--------- packages/rum/src/viewCollection.ts | 31 ++++++------------------ 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 70f3c324e6..b70b02917f 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -32,24 +32,26 @@ export interface AutoUserAction { export type UserAction = CustomUserAction | AutoUserAction -export let currentUserActionProcess = { stop: noop } +export function stopCurrentUserAction() { + if (currentUserAction) { + currentUserAction.stop() + } +} export function startUserActionCollection(lifeCycle: LifeCycle) { - const subscriptions: Subscription[] = [] - + let stopNewUserAction: { stop(): void } addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { if (!(event.target instanceof Element)) { return } - currentUserActionProcess = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) + stopNewUserAction = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } return { stop() { - currentUserActionProcess.stop() + stopNewUserAction.stop() removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) - subscriptions.forEach((s) => s.unsubscribe()) }, } } @@ -57,18 +59,18 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { interface PendingAutoUserAction { id: string startTime: number + stop(): void } -let currentUserAction: PendingAutoUserAction | undefined +export let currentUserAction: PendingAutoUserAction | undefined function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { if (currentUserAction) { // Discard any new click user action if another one is already occuring. - return { stop: currentUserActionProcess.stop } + return { stop: currentUserAction.stop } } const id = generateUUID() const startTime = performance.now() - currentUserAction = { id, startTime } const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) @@ -78,14 +80,14 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, name, + startTime, type, - duration: endTime - currentUserAction.startTime, + duration: endTime - startTime, measures: { errorCount: eventCounts.errorCount, longTaskCount: eventCounts.longTaskCount, resourceCount: eventCounts.resourceCount, }, - startTime: currentUserAction.startTime, }) } currentUserAction = undefined @@ -96,6 +98,12 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) currentUserAction = undefined } + currentUserAction = { + id, + startTime, + stop, + } + return { stop } } diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 7fe6f1bc11..d7000b6a73 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -5,7 +5,7 @@ import { PerformancePaintTiming } from './rum' import { RumSession } from './rumSession' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' -import { currentUserActionProcess } from './userActionCollection' +import { stopCurrentUserAction } from './userActionCollection' export interface View { id: string @@ -199,34 +199,17 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -interface ViewLoadingState { - stop: () => void -} -let currentViewLoadingState: ViewLoadingState | undefined - export function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { - // Stop the current user action if there is one - currentUserActionProcess.stop() - - if (currentViewLoadingState) { - currentViewLoadingState.stop() - } + // Stop the current user action + stopCurrentUserAction() const startTime: number = performance.now() - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { - if (currentViewLoadingState !== undefined) { - // If there is no activity during waitIdlePageActivity validation timeout, - // it will not return an end time but the view is loaded as no activity has occured - const loadingEndTime = endTime || performance.now() - callback(loadingEndTime - startTime) - currentViewLoadingState = undefined - } + // If there is no activity during waitIdlePageActivity validation timeout, + // it will not return an end time but the view is loaded as no activity has occured + const loadingEndTime = endTime || performance.now() + callback(loadingEndTime - startTime) }) - currentViewLoadingState = { - stop: stopWaitIdlePageActivity, - } - return { stop: stopWaitIdlePageActivity } } From 8aef86e0a0586cf96b8f48ea5e3850e7e2c8bcd1 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 17:05:31 +0200 Subject: [PATCH 27/42] :ok_hand: move stopCurrentUserAction() at the newView Level --- packages/rum/src/userActionCollection.ts | 6 ++++-- packages/rum/src/viewCollection.ts | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index b70b02917f..d9954e3ef7 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -1,6 +1,6 @@ import { Context, DOM_EVENT, generateUUID, noop } from '@datadog/browser-core' import { getElementContent } from './getElementContent' -import { LifeCycle, LifeCycleEventType, Subscription } from './lifeCycle' +import { LifeCycle, LifeCycleEventType } from './lifeCycle' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' @@ -39,7 +39,9 @@ export function stopCurrentUserAction() { } export function startUserActionCollection(lifeCycle: LifeCycle) { - let stopNewUserAction: { stop(): void } + let stopNewUserAction: { stop(): void } = { + stop: noop, + } addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { if (!(event.target instanceof Element)) { diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index d7000b6a73..b9096b40fc 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -81,6 +81,9 @@ function newView( loadType: ViewLoadType, startOrigin: number = performance.now() ) { + // Stop the current user action + stopCurrentUserAction() + // Setup initial values const id = generateUUID() let measures: ViewMeasures = { @@ -200,9 +203,6 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void } export function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { - // Stop the current user action - stopCurrentUserAction() - const startTime: number = performance.now() const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { // If there is no activity during waitIdlePageActivity validation timeout, From 34f9ee88587c296854993fab694a614d7c4a726a Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 11 May 2020 18:26:11 +0200 Subject: [PATCH 28/42] :ok_hand: add newView to the test scenario --- packages/rum/src/userActionCollection.ts | 14 +++++++------- packages/rum/src/viewCollection.ts | 4 ++-- packages/rum/test/userActionCollection.spec.ts | 14 ++++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index d9954e3ef7..52f2cc4c92 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -32,6 +32,13 @@ export interface AutoUserAction { export type UserAction = CustomUserAction | AutoUserAction +interface PendingAutoUserAction { + id: string + startTime: number + stop(): void +} +export let currentUserAction: PendingAutoUserAction | undefined + export function stopCurrentUserAction() { if (currentUserAction) { currentUserAction.stop() @@ -58,13 +65,6 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { } } -interface PendingAutoUserAction { - id: string - startTime: number - stop(): void -} -export let currentUserAction: PendingAutoUserAction | undefined - function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { if (currentUserAction) { // Discard any new click user action if another one is already occuring. diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index b9096b40fc..068f4993f9 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -74,7 +74,7 @@ interface ViewContext { export let viewContext: ViewContext -function newView( +export function newView( lifeCycle: LifeCycle, location: Location, session: RumSession, @@ -202,7 +202,7 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -export function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { +function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { const startTime: number = performance.now() const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { // If there is no activity during waitIdlePageActivity validation timeout, diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index e3bba9283b..ae78e2c295 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,5 +1,6 @@ import { DOM_EVENT, ErrorMessage, noop, Observable } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' +import { RumSession } from '../src/rumSession' import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, @@ -16,7 +17,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { trackLoadDuration, View } from '../src/viewCollection' +import { newView, ViewLoadType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -102,11 +103,12 @@ describe('startUserActionCollection', () => { it('cancels ongoing user action on view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } - const mockView: Partial = { - documentVersion: 0, - id: 'foo', - location: fakeLocation as Location, + const fakeSession = { + getId() { + return '42' + }, } + button.addEventListener(DOM_EVENT.CLICK, () => { clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) // Since we don't collect dom mutations for this test, manually dispatch one @@ -116,7 +118,7 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() function updateLoadDuration(loadDurationValue: number) {} - trackLoadDuration(lifeCycle, updateLoadDuration) + newView(lifeCycle, fakeLocation as Location, fakeSession as RumSession, ViewLoadType.INITIAL_LOAD) clock.expire() expect(events).toEqual([]) From 946104d40bad8ae4f52a63ab7b1aed490bf40f04 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 12 May 2020 10:01:28 +0200 Subject: [PATCH 29/42] :ok_hand: fix lint --- packages/rum/test/userActionCollection.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index ae78e2c295..886d879323 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -117,7 +117,6 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - function updateLoadDuration(loadDurationValue: number) {} newView(lifeCycle, fakeLocation as Location, fakeSession as RumSession, ViewLoadType.INITIAL_LOAD) clock.expire() From e53c670f88bedacb55c75023f5f664ae2d4e78b6 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Tue, 12 May 2020 11:24:29 +0200 Subject: [PATCH 30/42] :ok_hand: fix user action stop process --- packages/rum/src/userActionCollection.ts | 14 +++++++++----- packages/rum/src/viewCollection.ts | 8 +++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 52f2cc4c92..3216aa2576 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -76,9 +76,13 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - waitIdlePageActivity(lifeCycle, (endTime) => { + function closeUserAction() { stopEventCountsTracking() - if (endTime !== undefined && currentUserAction !== undefined && currentUserAction.id === id) { + currentUserAction = undefined + } + + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { + if (endTime !== undefined) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, name, @@ -92,12 +96,12 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) }, }) } - currentUserAction = undefined + closeUserAction() }) function stop() { - stopEventCountsTracking() - currentUserAction = undefined + closeUserAction() + stopWaitIdlePageActivity() } currentUserAction = { diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 068f4993f9..1e8a541ebe 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -40,23 +40,21 @@ export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export function startViewCollection(location: Location, lifeCycle: LifeCycle, session: RumSession) { let currentLocation = { ...location } const startOrigin = 0 - let currentViewLoadType: ViewLoadType = ViewLoadType.INITIAL_LOAD - let currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType, startOrigin) + let currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.INITIAL_LOAD, startOrigin) // Renew view on history changes trackHistory(() => { - currentViewLoadType = ViewLoadType.ROUTE_CHANGE if (areDifferentViews(currentLocation, location)) { currentLocation = { ...location } currentView.end() - currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType) + currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.ROUTE_CHANGE) } }) // Renew view on session renewal lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { currentView.end() - currentView = newView(lifeCycle, currentLocation, session, currentViewLoadType) + currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.ROUTE_CHANGE) }) // End the current view on page unload From 761ef8af25e362a9efaabf850c9a33413033de4a Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 13 May 2020 11:58:52 +0200 Subject: [PATCH 31/42] :ok_hand: improve tests and reformat waitPageActivitiesCompletion callback --- packages/rum/src/trackPageActivities.ts | 18 ++++---- packages/rum/src/userActionCollection.ts | 43 ++++++++----------- packages/rum/src/viewCollection.ts | 10 ++--- .../rum/test/userActionCollection.spec.ts | 20 ++++++--- packages/rum/test/viewCollection.spec.ts | 33 ++++++++++++-- 5 files changed, 73 insertions(+), 51 deletions(-) diff --git a/packages/rum/src/trackPageActivities.ts b/packages/rum/src/trackPageActivities.ts index 4987a8169c..913e9393d1 100644 --- a/packages/rum/src/trackPageActivities.ts +++ b/packages/rum/src/trackPageActivities.ts @@ -14,16 +14,14 @@ export interface PageActivityEvent { export function waitIdlePageActivity( lifeCycle: LifeCycle, - completionCallback: (endTime: number | undefined) => void + completionCallback: (hadActivity: boolean, endTime: number) => void ): { stop(): void } { const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities(lifeCycle) const { stop: stopWaitPageActivitiesCompletion } = waitPageActivitiesCompletion( pageActivitiesObservable, stopPageActivitiesTracking, - (endTime) => { - completionCallback(endTime) - } + completionCallback ) function stop() { @@ -113,20 +111,20 @@ export function trackPageActivities(lifeCycle: LifeCycle): { observable: Observa export function waitPageActivitiesCompletion( pageActivitiesObservable: Observable, stopPageActivitiesTracking: () => void, - completionCallback: (endTime: number | undefined) => void + completionCallback: (hadActivity: boolean, endTime: number) => void ): { stop(): void } { let idleTimeoutId: ReturnType let hasCompleted = false - const validationTimeoutId = setTimeout(monitor(() => complete(undefined)), PAGE_ACTIVITY_VALIDATION_DELAY) - const maxDurationTimeoutId = setTimeout(monitor(() => complete(performance.now())), PAGE_ACTIVITY_MAX_DURATION) + const validationTimeoutId = setTimeout(monitor(() => complete(false, 0)), PAGE_ACTIVITY_VALIDATION_DELAY) + const maxDurationTimeoutId = setTimeout(monitor(() => complete(true, performance.now())), PAGE_ACTIVITY_MAX_DURATION) pageActivitiesObservable.subscribe(({ isBusy }) => { clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) const lastChangeTime = performance.now() if (!isBusy) { - idleTimeoutId = setTimeout(monitor(() => complete(lastChangeTime)), PAGE_ACTIVITY_END_DELAY) + idleTimeoutId = setTimeout(monitor(() => complete(true, lastChangeTime)), PAGE_ACTIVITY_END_DELAY) } }) @@ -138,12 +136,12 @@ export function waitPageActivitiesCompletion( stopPageActivitiesTracking() } - function complete(endTime: number | undefined) { + function complete(hadActivity: boolean, endTime: number) { if (hasCompleted) { return } stop() - completionCallback(endTime) + completionCallback(hadActivity, endTime) } return { stop } diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 3216aa2576..056173911a 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -35,40 +35,34 @@ export type UserAction = CustomUserAction | AutoUserAction interface PendingAutoUserAction { id: string startTime: number - stop(): void } -export let currentUserAction: PendingAutoUserAction | undefined +export let pendingAutoUserAction: PendingAutoUserAction | undefined -export function stopCurrentUserAction() { - if (currentUserAction) { - currentUserAction.stop() - } +export let stopPendingAutoUserAction: { stop(): void } = { + stop: noop, } export function startUserActionCollection(lifeCycle: LifeCycle) { - let stopNewUserAction: { stop(): void } = { - stop: noop, - } addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { if (!(event.target instanceof Element)) { return } - stopNewUserAction = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) + stopPendingAutoUserAction = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } return { stop() { - stopNewUserAction.stop() + stopPendingAutoUserAction.stop() removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) }, } } function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { - if (currentUserAction) { - // Discard any new click user action if another one is already occuring. - return { stop: currentUserAction.stop } + if (pendingAutoUserAction) { + // Discard any new user action if another one is already occurring. + return { stop: stopPendingAutoUserAction.stop } } const id = generateUUID() @@ -76,13 +70,13 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - function closeUserAction() { + function stopUserAction() { stopEventCountsTracking() - currentUserAction = undefined + pendingAutoUserAction = undefined } - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { - if (endTime !== undefined) { + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (hadActivity, endTime) => { + if (hadActivity) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { id, name, @@ -96,18 +90,17 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) }, }) } - closeUserAction() + stopUserAction() }) function stop() { - closeUserAction() + stopUserAction() stopWaitIdlePageActivity() } - currentUserAction = { + pendingAutoUserAction = { id, startTime, - stop, } return { stop } @@ -117,16 +110,16 @@ export interface UserActionReference { id: string } export function getUserActionReference(time?: number): UserActionReference | undefined { - if (!currentUserAction || (time !== undefined && time < currentUserAction.startTime)) { + if (!pendingAutoUserAction || (time !== undefined && time < pendingAutoUserAction.startTime)) { return undefined } - return { id: currentUserAction.id } + return { id: pendingAutoUserAction.id } } export const $$tests = { newUserAction, resetUserAction() { - currentUserAction = undefined + pendingAutoUserAction = undefined }, } diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 1e8a541ebe..867524fff8 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -5,7 +5,7 @@ import { PerformancePaintTiming } from './rum' import { RumSession } from './rumSession' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' -import { stopCurrentUserAction } from './userActionCollection' +import { stopPendingAutoUserAction } from './userActionCollection' export interface View { id: string @@ -80,7 +80,7 @@ export function newView( startOrigin: number = performance.now() ) { // Stop the current user action - stopCurrentUserAction() + stopPendingAutoUserAction.stop() // Setup initial values const id = generateUUID() @@ -202,10 +202,8 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { const startTime: number = performance.now() - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (endTime) => { - // If there is no activity during waitIdlePageActivity validation timeout, - // it will not return an end time but the view is loaded as no activity has occured - const loadingEndTime = endTime || performance.now() + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (hadActivity, endTime) => { + const loadingEndTime = hadActivity ? endTime : performance.now() callback(loadingEndTime - startTime) }) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 886d879323..f1cd33020c 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -248,8 +248,9 @@ describe('waitPageActivitiesCompletion', () => { const clock = mockClock() it('should not collect an event that is not followed by page activity', (done) => { - waitPageActivitiesCompletion(new Observable(), noop, (endTime) => { - expect(endTime).toBeUndefined() + waitPageActivitiesCompletion(new Observable(), noop, (hadActivity, endTime) => { + expect(hadActivity).toBeFalse() + expect(endTime).toBeFalsy() done() }) @@ -260,7 +261,8 @@ describe('waitPageActivitiesCompletion', () => { const activityObservable = new Observable() const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() expect(endTime).toEqual(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) done() }) @@ -279,7 +281,8 @@ describe('waitPageActivitiesCompletion', () => { // Extend the user action but stops before PAGE_ACTIVITY_MAX_DURATION const extendCount = Math.floor(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY - 1) - waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_PAGE_ACTIVITY_END_DELAY) done() }) @@ -300,7 +303,8 @@ describe('waitPageActivitiesCompletion', () => { // Extend the user action until it's more than PAGE_ACTIVITY_MAX_DURATION const extendCount = Math.ceil(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY + 1) - waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) stop = true done() @@ -319,7 +323,8 @@ describe('waitPageActivitiesCompletion', () => { it('is extended while the page is busy', (done) => { const activityObservable = new Observable() const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() expect(endTime).toBe(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY + PAGE_ACTIVITY_END_DELAY * 2) done() }) @@ -336,7 +341,8 @@ describe('waitPageActivitiesCompletion', () => { it('expires is the page is busy for too long', (done) => { const activityObservable = new Observable() const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (endTime) => { + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) done() }) diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index b5065700ad..216143cdd8 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -12,9 +12,15 @@ import { ViewLoadType, } from '../src/viewCollection' -import { PAGE_ACTIVITY_MAX_DURATION } from '../src/trackPageActivities' +import { + PAGE_ACTIVITY_MAX_DURATION, + PAGE_ACTIVITY_VALIDATION_DELAY, + PAGE_ACTIVITY_END_DELAY, +} from '../src/trackPageActivities' const AFTER_PAGE_ACTIVITY_MAX_DURATION = PAGE_ACTIVITY_MAX_DURATION * 1.1 +const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 +const AFTER_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 1.1 function setup(lifeCycle: LifeCycle = new LifeCycle()) { spyOn(history, 'pushState').and.callFake((_: any, __: string, pathname: string) => { @@ -117,7 +123,20 @@ describe('rum track renew session', () => { describe('rum track load duration', () => { let lifeCycle: LifeCycle let getViewEvent: (index: number) => View + + const mockDateNowT1 = 1500000000000 + const mockDateNowT2 = 1500001000000 + beforeEach(() => { + let isFirstPerformanceNow = true + spyOn(performance, 'now').and.callFake(() => { + if (isFirstPerformanceNow) { + isFirstPerformanceNow = false + return mockDateNowT1 + } + return mockDateNowT2 + }) + jasmine.clock().install() ;({ lifeCycle, getViewEvent } = spyOnViews()) }) @@ -126,11 +145,19 @@ describe('rum track load duration', () => { jasmine.clock().uninstall() }) - it('should set a loadDuration once the load is complete', () => { + it('should set a loadDuration once the load is complete without having any activity', () => { jasmine.clock().tick(AFTER_PAGE_ACTIVITY_MAX_DURATION) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadDuration).toBeDefined() + expect(getViewEvent(1).loadDuration).toEqual(mockDateNowT2 - mockDateNowT1) + }) + + it('should set a loadDuration once the load is complete after having some activity', () => { + jasmine.clock().tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) + jasmine.clock().tick(AFTER_PAGE_ACTIVITY_END_DELAY) + jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) + expect(getViewEvent(1).loadDuration).toEqual(mockDateNowT2 - mockDateNowT1) }) }) From bfb15038f7936591af45361008e642554da9bb1f Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 13 May 2020 13:54:30 +0200 Subject: [PATCH 32/42] :ok_hand: move tests into waitPageActivitiesCompletion into trackPagePageActivity module --- packages/rum/test/trackPageActivities.spec.ts | 151 +++++++++++++++++- .../rum/test/userActionCollection.spec.ts | 117 +------------- packages/rum/test/viewCollection.spec.ts | 2 +- 3 files changed, 151 insertions(+), 119 deletions(-) diff --git a/packages/rum/test/trackPageActivities.spec.ts b/packages/rum/test/trackPageActivities.spec.ts index 07c0ee45c1..22c2a98131 100644 --- a/packages/rum/test/trackPageActivities.spec.ts +++ b/packages/rum/test/trackPageActivities.spec.ts @@ -1,6 +1,42 @@ -import { RequestCompleteEvent } from '@datadog/browser-core' +import { noop, Observable, RequestCompleteEvent } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' -import { PageActivityEvent, trackPageActivities } from '../src/trackPageActivities' +import { + PAGE_ACTIVITY_END_DELAY, + PAGE_ACTIVITY_MAX_DURATION, + PAGE_ACTIVITY_VALIDATION_DELAY, + PageActivityEvent, + trackPageActivities, + waitPageActivitiesCompletion, +} from '../src/trackPageActivities' + +// Used to wait some time after the creation of a user action +const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 +// Used to wait some time before the (potential) end of a user action +const BEFORE_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 0.8 +// A long delay used to wait after any user action is finished. +const EXPIRE_DELAY = PAGE_ACTIVITY_MAX_DURATION * 10 + +function mockClock() { + beforeEach(() => { + jasmine.clock().install() + jasmine.clock().mockDate() + spyOn(performance, 'now').and.callFake(() => Date.now()) + }) + + afterEach(() => { + jasmine.clock().uninstall() + }) + + return { + tick(ms: number) { + jasmine.clock().tick(ms) + }, + expire() { + // Make sure no user action is still pending + jasmine.clock().tick(EXPIRE_DELAY) + }, + } +} function eventsCollector() { const events: T[] = [] @@ -106,3 +142,114 @@ describe('trackPagePageActivities', () => { }) }) }) + +describe('waitPageActivitiesCompletion', () => { + const clock = mockClock() + + it('should not collect an event that is not followed by page activity', (done) => { + waitPageActivitiesCompletion(new Observable(), noop, (hadActivity, endTime) => { + expect(hadActivity).toBeFalse() + expect(endTime).toBeFalsy() + done() + }) + + clock.expire() + }) + + it('should collect an event that is followed by page activity', (done) => { + const activityObservable = new Observable() + + const startTime = performance.now() + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() + expect(endTime).toEqual(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + done() + }) + + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + activityObservable.notify({ isBusy: false }) + + clock.expire() + }) + + describe('extend with activities', () => { + it('is extended while there is page activities', (done) => { + const activityObservable = new Observable() + const startTime = performance.now() + + // Extend the user action but stops before PAGE_ACTIVITY_MAX_DURATION + const extendCount = Math.floor(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY - 1) + + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() + expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_PAGE_ACTIVITY_END_DELAY) + done() + }) + + for (let i = 0; i < extendCount; i += 1) { + clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) + activityObservable.notify({ isBusy: false }) + } + + clock.expire() + }) + + it('expires after a limit', (done) => { + const activityObservable = new Observable() + let stop = false + const startTime = performance.now() + + // Extend the user action until it's more than PAGE_ACTIVITY_MAX_DURATION + const extendCount = Math.ceil(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY + 1) + + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() + expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) + stop = true + done() + }) + + for (let i = 0; i < extendCount && !stop; i += 1) { + clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) + activityObservable.notify({ isBusy: false }) + } + + clock.expire() + }) + }) + + describe('busy activities', () => { + it('is extended while the page is busy', (done) => { + const activityObservable = new Observable() + const startTime = performance.now() + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() + expect(endTime).toBe(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY + PAGE_ACTIVITY_END_DELAY * 2) + done() + }) + + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + activityObservable.notify({ isBusy: true }) + + clock.tick(PAGE_ACTIVITY_END_DELAY * 2) + activityObservable.notify({ isBusy: false }) + + clock.expire() + }) + + it('expires is the page is busy for too long', (done) => { + const activityObservable = new Observable() + const startTime = performance.now() + waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { + expect(hadActivity).toBeTrue() + expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) + done() + }) + + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + activityObservable.notify({ isBusy: true }) + + clock.expire() + }) + }) +}) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index f1cd33020c..d10486c528 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,4 +1,4 @@ -import { DOM_EVENT, ErrorMessage, noop, Observable } from '@datadog/browser-core' +import { DOM_EVENT, ErrorMessage } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { RumSession } from '../src/rumSession' import { @@ -21,10 +21,6 @@ import { newView, ViewLoadType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 -// Used to wait some time before the (potential) end of a user action -const BEFORE_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 0.8 -// Used to wait some time after the (potential) end of a user action -const AFTER_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 1.2 // Used to wait some time but it doesn't matter how much. const SOME_ARBITRARY_DELAY = 50 // A long delay used to wait after any user action is finished. @@ -243,114 +239,3 @@ describe('newUserAction', () => { }) }) }) - -describe('waitPageActivitiesCompletion', () => { - const clock = mockClock() - - it('should not collect an event that is not followed by page activity', (done) => { - waitPageActivitiesCompletion(new Observable(), noop, (hadActivity, endTime) => { - expect(hadActivity).toBeFalse() - expect(endTime).toBeFalsy() - done() - }) - - clock.expire() - }) - - it('should collect an event that is followed by page activity', (done) => { - const activityObservable = new Observable() - - const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { - expect(hadActivity).toBeTrue() - expect(endTime).toEqual(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - done() - }) - - clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - activityObservable.notify({ isBusy: false }) - - clock.expire() - }) - - describe('extend with activities', () => { - it('is extended while there is page activities', (done) => { - const activityObservable = new Observable() - const startTime = performance.now() - - // Extend the user action but stops before PAGE_ACTIVITY_MAX_DURATION - const extendCount = Math.floor(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY - 1) - - waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { - expect(hadActivity).toBeTrue() - expect(endTime).toBe(startTime + (extendCount + 1) * BEFORE_PAGE_ACTIVITY_END_DELAY) - done() - }) - - for (let i = 0; i < extendCount; i += 1) { - clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) - activityObservable.notify({ isBusy: false }) - } - - clock.expire() - }) - - it('expires after a limit', (done) => { - const activityObservable = new Observable() - let stop = false - const startTime = performance.now() - - // Extend the user action until it's more than PAGE_ACTIVITY_MAX_DURATION - const extendCount = Math.ceil(PAGE_ACTIVITY_MAX_DURATION / BEFORE_PAGE_ACTIVITY_END_DELAY + 1) - - waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { - expect(hadActivity).toBeTrue() - expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) - stop = true - done() - }) - - for (let i = 0; i < extendCount && !stop; i += 1) { - clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) - activityObservable.notify({ isBusy: false }) - } - - clock.expire() - }) - }) - - describe('busy activities', () => { - it('is extended while the page is busy', (done) => { - const activityObservable = new Observable() - const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { - expect(hadActivity).toBeTrue() - expect(endTime).toBe(startTime + BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY + PAGE_ACTIVITY_END_DELAY * 2) - done() - }) - - clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - activityObservable.notify({ isBusy: true }) - - clock.tick(PAGE_ACTIVITY_END_DELAY * 2) - activityObservable.notify({ isBusy: false }) - - clock.expire() - }) - - it('expires is the page is busy for too long', (done) => { - const activityObservable = new Observable() - const startTime = performance.now() - waitPageActivitiesCompletion(activityObservable, noop, (hadActivity, endTime) => { - expect(hadActivity).toBeTrue() - expect(endTime).toBe(startTime + PAGE_ACTIVITY_MAX_DURATION) - done() - }) - - clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - activityObservable.notify({ isBusy: true }) - - clock.expire() - }) - }) -}) diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 216143cdd8..b11c50261f 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -13,9 +13,9 @@ import { } from '../src/viewCollection' import { + PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_MAX_DURATION, PAGE_ACTIVITY_VALIDATION_DELAY, - PAGE_ACTIVITY_END_DELAY, } from '../src/trackPageActivities' const AFTER_PAGE_ACTIVITY_MAX_DURATION = PAGE_ACTIVITY_MAX_DURATION * 1.1 From c2999e34e951991d5cdf4f9f0cb517851b81bbe4 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 13 May 2020 13:59:54 +0200 Subject: [PATCH 33/42] :ok_hand: nit --- packages/rum/test/userActionCollection.spec.ts | 2 +- packages/rum/test/viewCollection.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index d10486c528..73997d4451 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -97,7 +97,7 @@ describe('startUserActionCollection', () => { clock.expire() } - it('cancels ongoing user action on view loading', () => { + it('cancels pending user action on view loading', () => { const fakeLocation: Partial = { pathname: '/foo' } const fakeSession = { getId() { diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index b11c50261f..3950d9ef68 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -48,11 +48,11 @@ describe('rum track url change', () => { initialLocation = viewContext.location }) - it('should collect initial view type as "initial page load"', () => { + it('should collect initial view type as "initial load"', () => { expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_LOAD) }) - it('should update view id and type on path change', () => { + it('should update view id and type on route change', () => { history.pushState({}, '', '/bar') expect(viewContext.id).not.toEqual(initialView) From 8131f75f9b125aa6bf6d1b7564b09b714f24efff Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Wed, 13 May 2020 16:18:13 +0200 Subject: [PATCH 34/42] :ok_hand: improve view loading type testing --- packages/rum/src/viewCollection.ts | 7 +++-- packages/rum/test/viewCollection.spec.ts | 34 +++++++++++++++++++----- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index 867524fff8..de9e69405b 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -31,8 +31,8 @@ export interface ViewMeasures { } export enum ViewLoadType { - INITIAL_LOAD = 'initial load', - ROUTE_CHANGE = 'route change', + INITIAL_LOAD = 'initial_load', + ROUTE_CHANGE = 'route_change', } export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 @@ -67,7 +67,6 @@ interface ViewContext { id: string location: Location sessionId: string | undefined - loadType: ViewLoadType } export let viewContext: ViewContext @@ -93,7 +92,7 @@ export function newView( let documentVersion = 0 let loadDuration: number - viewContext = { id, location, loadType, sessionId: session.getId() } + viewContext = { id, location, sessionId: session.getId() } // Update the view every time the measures are changing const { throttled: scheduleViewUpdate, stop: stopScheduleViewUpdate } = throttle( diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 3950d9ef68..9387b4c05d 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -48,16 +48,11 @@ describe('rum track url change', () => { initialLocation = viewContext.location }) - it('should collect initial view type as "initial load"', () => { - expect(viewContext.loadType).toEqual(ViewLoadType.INITIAL_LOAD) - }) - - it('should update view id and type on route change', () => { + it('should update view id on path change', () => { history.pushState({}, '', '/bar') expect(viewContext.id).not.toEqual(initialView) expect(viewContext.location).not.toEqual(initialLocation) - expect(viewContext.loadType).toEqual(ViewLoadType.ROUTE_CHANGE) }) it('should not update view id on search change', () => { @@ -120,6 +115,33 @@ describe('rum track renew session', () => { }) }) +describe('rum track load duration', () => { + let lifeCycle: LifeCycle + let getViewEvent: (index: number) => View + + beforeEach(() => { + jasmine.clock().install() + ;({ lifeCycle, getViewEvent } = spyOnViews()) + }) + + afterEach(() => { + jasmine.clock().uninstall() + }) + + it('should collect intital view type as "initial_load"', () => { + expect(getViewEvent(0).loadType).toEqual(ViewLoadType.INITIAL_LOAD) + }) + + it('should collect view type as "route_change" after a route change', () => { + history.pushState({}, '', '/bar') + expect(getViewEvent(1).location.pathname).toEqual('/foo') + expect(getViewEvent(1).loadType).toEqual(ViewLoadType.INITIAL_LOAD) + + expect(getViewEvent(2).location.pathname).toEqual('/bar') + expect(getViewEvent(2).loadType).toEqual(ViewLoadType.ROUTE_CHANGE) + }) +}) + describe('rum track load duration', () => { let lifeCycle: LifeCycle let getViewEvent: (index: number) => View From 94fc77fd670b1cbc753bb31e6a258bc37001493b Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 14 May 2020 10:41:32 +0200 Subject: [PATCH 35/42] :ok_hand: remove stop ua function dependency in the ViewCollection --- packages/rum/src/userActionCollection.ts | 9 +++++++-- packages/rum/src/viewCollection.ts | 4 ---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index 056173911a..f919316ae5 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -36,9 +36,9 @@ interface PendingAutoUserAction { id: string startTime: number } -export let pendingAutoUserAction: PendingAutoUserAction | undefined +let pendingAutoUserAction: PendingAutoUserAction | undefined -export let stopPendingAutoUserAction: { stop(): void } = { +let stopPendingAutoUserAction: { stop(): void } = { stop: noop, } @@ -51,6 +51,11 @@ export function startUserActionCollection(lifeCycle: LifeCycle) { stopPendingAutoUserAction = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } + // New views trigger the cancellation of the current pending User Action + lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, () => { + stopPendingAutoUserAction.stop() + }) + return { stop() { stopPendingAutoUserAction.stop() diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index de9e69405b..f1474912f2 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -5,7 +5,6 @@ import { PerformancePaintTiming } from './rum' import { RumSession } from './rumSession' import { trackEventCounts } from './trackEventCounts' import { waitIdlePageActivity } from './trackPageActivities' -import { stopPendingAutoUserAction } from './userActionCollection' export interface View { id: string @@ -78,9 +77,6 @@ export function newView( loadType: ViewLoadType, startOrigin: number = performance.now() ) { - // Stop the current user action - stopPendingAutoUserAction.stop() - // Setup initial values const id = generateUUID() let measures: ViewMeasures = { From 6cb28077916b8d32a27088260248539d264091ca Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 14 May 2020 16:21:21 +0200 Subject: [PATCH 36/42] =?UTF-8?q?=F0=9F=8E=A8=20rename=20laodDuration=20to?= =?UTF-8?q?=20loadTime?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/viewCollection.ts | 16 ++++++++-------- packages/rum/test/viewCollection.spec.ts | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index f1474912f2..b927de82f8 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -13,7 +13,7 @@ export interface View { documentVersion: number startTime: number duration: number - loadDuration?: number + loadingTime?: number loadType: ViewLoadType } @@ -86,7 +86,7 @@ export function newView( userActionCount: 0, } let documentVersion = 0 - let loadDuration: number + let loadingTime: number viewContext = { id, location, sessionId: session.getId() } @@ -105,11 +105,11 @@ export function newView( const { stop: stopTimingsTracking } = trackTimings(lifeCycle, updateMeasures) const { stop: stopEventCountsTracking } = trackEventCounts(lifeCycle, updateMeasures) - function updateLoadDuration(loadDurationValue: number) { - loadDuration = loadDurationValue + function updateLoadingTime(loadingTimeValue: number) { + loadingTime = loadingTimeValue scheduleViewUpdate() } - const { stop: stopLoadDurationTracking } = trackLoadDuration(lifeCycle, updateLoadDuration) + const { stop: stopLoadingTimeTracking } = trackLoadingTime(lifeCycle, updateLoadingTime) // Initial view update updateView() @@ -119,8 +119,8 @@ export function newView( lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { documentVersion, id, - loadDuration, loadType, + loadingTime, location, measures, duration: performance.now() - startOrigin, @@ -132,7 +132,7 @@ export function newView( end() { stopTimingsTracking() stopEventCountsTracking() - stopLoadDurationTracking() + stopLoadingTimeTracking() // prevent pending view updates execution stopScheduleViewUpdate() // Make a final view update @@ -195,7 +195,7 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -function trackLoadDuration(lifeCycle: LifeCycle, callback: (loadDurationValue: number) => void) { +function trackLoadingTime(lifeCycle: LifeCycle, callback: (loadingTimeValue: number) => void) { const startTime: number = performance.now() const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (hadActivity, endTime) => { const loadingEndTime = hadActivity ? endTime : performance.now() diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 9387b4c05d..d4f04c9bd1 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -167,19 +167,19 @@ describe('rum track load duration', () => { jasmine.clock().uninstall() }) - it('should set a loadDuration once the load is complete without having any activity', () => { + it('should set a loadingTime once the load is complete without having any activity', () => { jasmine.clock().tick(AFTER_PAGE_ACTIVITY_MAX_DURATION) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadDuration).toEqual(mockDateNowT2 - mockDateNowT1) + expect(getViewEvent(1).loadingTime).toEqual(mockDateNowT2 - mockDateNowT1) }) - it('should set a loadDuration once the load is complete after having some activity', () => { + it('should set a loadingTime once the load is complete after having some activity', () => { jasmine.clock().tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) jasmine.clock().tick(AFTER_PAGE_ACTIVITY_END_DELAY) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadDuration).toEqual(mockDateNowT2 - mockDateNowT1) + expect(getViewEvent(1).loadingTime).toEqual(mockDateNowT2 - mockDateNowT1) }) }) From 75d0f7a1063567af51741c75196d7cc4f84b4aa6 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Thu, 14 May 2020 17:40:22 +0200 Subject: [PATCH 37/42] =?UTF-8?q?=E2=9C=A8=20add=20the=20loadTime=20and=20?= =?UTF-8?q?loadType=20in=20the=20RumEvent=20batch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/rum.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/rum/src/rum.ts b/packages/rum/src/rum.ts index 0e7de977ab..451113a0ae 100644 --- a/packages/rum/src/rum.ts +++ b/packages/rum/src/rum.ts @@ -32,7 +32,7 @@ import { import { InternalContext, RumGlobal } from './rum.entry' import { RumSession } from './rumSession' import { getUserActionReference, UserActionMeasures, UserActionReference, UserActionType } from './userActionCollection' -import { viewContext, ViewMeasures } from './viewCollection' +import { viewContext, ViewLoadType, ViewMeasures } from './viewCollection' export interface PerformancePaintTiming extends PerformanceEntry { entryType: 'paint' @@ -108,6 +108,8 @@ export interface RumViewEvent { documentVersion: number } view: { + loadingTime?: number + loadType: ViewLoadType measures: ViewMeasures } } @@ -261,6 +263,8 @@ function trackView(lifeCycle: LifeCycle, upsertRumEvent: (event: RumViewEvent, k documentVersion: view.documentVersion, }, view: { + loadType: view.loadType, + loadingTime: view.loadingTime, measures: view.measures, }, }, From 4614cfdc94677f0f0e87a4822d53b63bc4466a26 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Fri, 15 May 2020 11:55:25 +0200 Subject: [PATCH 38/42] =?UTF-8?q?=F0=9F=91=8C=20implement=20new=20reviews?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/rum.ts | 6 ++-- packages/rum/src/userActionCollection.ts | 30 +++++++++---------- packages/rum/src/viewCollection.ts | 14 ++++----- .../rum/test/userActionCollection.spec.ts | 4 +-- packages/rum/test/viewCollection.spec.ts | 8 ++--- 5 files changed, 30 insertions(+), 32 deletions(-) diff --git a/packages/rum/src/rum.ts b/packages/rum/src/rum.ts index 451113a0ae..b19c44e2fe 100644 --- a/packages/rum/src/rum.ts +++ b/packages/rum/src/rum.ts @@ -32,7 +32,7 @@ import { import { InternalContext, RumGlobal } from './rum.entry' import { RumSession } from './rumSession' import { getUserActionReference, UserActionMeasures, UserActionReference, UserActionType } from './userActionCollection' -import { viewContext, ViewLoadType, ViewMeasures } from './viewCollection' +import { viewContext, ViewLoadingType, ViewMeasures } from './viewCollection' export interface PerformancePaintTiming extends PerformanceEntry { entryType: 'paint' @@ -109,7 +109,7 @@ export interface RumViewEvent { } view: { loadingTime?: number - loadType: ViewLoadType + loadingType: ViewLoadingType measures: ViewMeasures } } @@ -263,7 +263,7 @@ function trackView(lifeCycle: LifeCycle, upsertRumEvent: (event: RumViewEvent, k documentVersion: view.documentVersion, }, view: { - loadType: view.loadType, + loadingType: view.loadingType, loadingTime: view.loadingTime, measures: view.measures, }, diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index f919316ae5..eb42955544 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -35,39 +35,40 @@ export type UserAction = CustomUserAction | AutoUserAction interface PendingAutoUserAction { id: string startTime: number + stop(): void } let pendingAutoUserAction: PendingAutoUserAction | undefined -let stopPendingAutoUserAction: { stop(): void } = { - stop: noop, -} - export function startUserActionCollection(lifeCycle: LifeCycle) { addEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) function processClick(event: Event) { if (!(event.target instanceof Element)) { return } - stopPendingAutoUserAction = newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) + newUserAction(lifeCycle, UserActionType.CLICK, getElementContent(event.target)) } // New views trigger the cancellation of the current pending User Action lifeCycle.subscribe(LifeCycleEventType.VIEW_COLLECTED, () => { - stopPendingAutoUserAction.stop() + if (pendingAutoUserAction) { + pendingAutoUserAction.stop() + } }) return { stop() { - stopPendingAutoUserAction.stop() + if (pendingAutoUserAction) { + pendingAutoUserAction.stop() + } removeEventListener(DOM_EVENT.CLICK, processClick, { capture: true }) }, } } -function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string): { stop(): void } { +function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) { if (pendingAutoUserAction) { // Discard any new user action if another one is already occurring. - return { stop: stopPendingAutoUserAction.stop } + return } const id = generateUUID() @@ -98,17 +99,14 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) stopUserAction() }) - function stop() { - stopUserAction() - stopWaitIdlePageActivity() - } - pendingAutoUserAction = { id, startTime, + stop() { + stopUserAction() + stopWaitIdlePageActivity() + }, } - - return { stop } } export interface UserActionReference { diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index b927de82f8..aa3d2332a0 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -14,7 +14,7 @@ export interface View { startTime: number duration: number loadingTime?: number - loadType: ViewLoadType + loadingType: ViewLoadingType } export interface ViewMeasures { @@ -29,7 +29,7 @@ export interface ViewMeasures { userActionCount: number } -export enum ViewLoadType { +export enum ViewLoadingType { INITIAL_LOAD = 'initial_load', ROUTE_CHANGE = 'route_change', } @@ -39,21 +39,21 @@ export const THROTTLE_VIEW_UPDATE_PERIOD = 3000 export function startViewCollection(location: Location, lifeCycle: LifeCycle, session: RumSession) { let currentLocation = { ...location } const startOrigin = 0 - let currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.INITIAL_LOAD, startOrigin) + let currentView = newView(lifeCycle, currentLocation, session, ViewLoadingType.INITIAL_LOAD, startOrigin) // Renew view on history changes trackHistory(() => { if (areDifferentViews(currentLocation, location)) { currentLocation = { ...location } currentView.end() - currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.ROUTE_CHANGE) + currentView = newView(lifeCycle, currentLocation, session, ViewLoadingType.ROUTE_CHANGE) } }) // Renew view on session renewal lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { currentView.end() - currentView = newView(lifeCycle, currentLocation, session, ViewLoadType.ROUTE_CHANGE) + currentView = newView(lifeCycle, currentLocation, session, ViewLoadingType.ROUTE_CHANGE) }) // End the current view on page unload @@ -74,7 +74,7 @@ export function newView( lifeCycle: LifeCycle, location: Location, session: RumSession, - loadType: ViewLoadType, + loadingType: ViewLoadingType, startOrigin: number = performance.now() ) { // Setup initial values @@ -119,7 +119,7 @@ export function newView( lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { documentVersion, id, - loadType, + loadingType, loadingTime, location, measures, diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 73997d4451..dca348141f 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -17,7 +17,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { newView, ViewLoadType } from '../src/viewCollection' +import { newView, ViewLoadingType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -113,7 +113,7 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - newView(lifeCycle, fakeLocation as Location, fakeSession as RumSession, ViewLoadType.INITIAL_LOAD) + newView(lifeCycle, fakeLocation as Location, fakeSession as RumSession, ViewLoadingType.INITIAL_LOAD) clock.expire() expect(events).toEqual([]) diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index d4f04c9bd1..0c4ebdb741 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -9,7 +9,7 @@ import { THROTTLE_VIEW_UPDATE_PERIOD, View, viewContext, - ViewLoadType, + ViewLoadingType, } from '../src/viewCollection' import { @@ -129,16 +129,16 @@ describe('rum track load duration', () => { }) it('should collect intital view type as "initial_load"', () => { - expect(getViewEvent(0).loadType).toEqual(ViewLoadType.INITIAL_LOAD) + expect(getViewEvent(0).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) }) it('should collect view type as "route_change" after a route change', () => { history.pushState({}, '', '/bar') expect(getViewEvent(1).location.pathname).toEqual('/foo') - expect(getViewEvent(1).loadType).toEqual(ViewLoadType.INITIAL_LOAD) + expect(getViewEvent(1).loadingType).toEqual(ViewLoadingType.INITIAL_LOAD) expect(getViewEvent(2).location.pathname).toEqual('/bar') - expect(getViewEvent(2).loadType).toEqual(ViewLoadType.ROUTE_CHANGE) + expect(getViewEvent(2).loadingType).toEqual(ViewLoadingType.ROUTE_CHANGE) }) }) From 1eaf083c1f833f523ff0936d07a16c5eaccd3025 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Fri, 15 May 2020 16:43:02 +0200 Subject: [PATCH 39/42] =?UTF-8?q?=E2=9C=85=20clarify=20loading=20time=20te?= =?UTF-8?q?sts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/rum.ts | 2 +- packages/rum/src/viewCollection.ts | 19 ++++++++------ .../rum/test/userActionCollection.spec.ts | 4 +-- packages/rum/test/viewCollection.spec.ts | 25 ++++++------------- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/packages/rum/src/rum.ts b/packages/rum/src/rum.ts index b19c44e2fe..0a308b6f22 100644 --- a/packages/rum/src/rum.ts +++ b/packages/rum/src/rum.ts @@ -263,8 +263,8 @@ function trackView(lifeCycle: LifeCycle, upsertRumEvent: (event: RumViewEvent, k documentVersion: view.documentVersion, }, view: { - loadingType: view.loadingType, loadingTime: view.loadingTime, + loadingType: view.loadingType, measures: view.measures, }, }, diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index aa3d2332a0..15a53206b1 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -13,7 +13,7 @@ export interface View { documentVersion: number startTime: number duration: number - loadingTime?: number + loadingTime?: number | undefined loadingType: ViewLoadingType } @@ -70,7 +70,7 @@ interface ViewContext { export let viewContext: ViewContext -export function newView( +function newView( lifeCycle: LifeCycle, location: Location, session: RumSession, @@ -86,7 +86,7 @@ export function newView( userActionCount: 0, } let documentVersion = 0 - let loadingTime: number + let loadingTime: number | undefined viewContext = { id, location, sessionId: session.getId() } @@ -105,7 +105,7 @@ export function newView( const { stop: stopTimingsTracking } = trackTimings(lifeCycle, updateMeasures) const { stop: stopEventCountsTracking } = trackEventCounts(lifeCycle, updateMeasures) - function updateLoadingTime(loadingTimeValue: number) { + function updateLoadingTime(loadingTimeValue: number | undefined) { loadingTime = loadingTimeValue scheduleViewUpdate() } @@ -119,8 +119,8 @@ export function newView( lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { documentVersion, id, - loadingType, loadingTime, + loadingType, location, measures, duration: performance.now() - startOrigin, @@ -195,11 +195,14 @@ function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void return { stop: stopPerformanceTracking } } -function trackLoadingTime(lifeCycle: LifeCycle, callback: (loadingTimeValue: number) => void) { +function trackLoadingTime(lifeCycle: LifeCycle, callback: (loadingTimeValue: number | undefined) => void) { const startTime: number = performance.now() const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (hadActivity, endTime) => { - const loadingEndTime = hadActivity ? endTime : performance.now() - callback(loadingEndTime - startTime) + if (hadActivity) { + callback(endTime - startTime) + } else { + callback(undefined) + } }) return { stop: stopWaitIdlePageActivity } diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index dca348141f..9e1d2f2311 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -17,7 +17,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { newView, ViewLoadingType } from '../src/viewCollection' +import { startViewCollection, ViewLoadingType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -113,7 +113,7 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - newView(lifeCycle, fakeLocation as Location, fakeSession as RumSession, ViewLoadingType.INITIAL_LOAD) + startViewCollection(fakeLocation as Location, lifeCycle, fakeSession as RumSession) clock.expire() expect(events).toEqual([]) diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index 0c4ebdb741..16dd73735a 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -142,24 +142,14 @@ describe('rum track load duration', () => { }) }) -describe('rum track load duration', () => { +describe('rum track loading time', () => { let lifeCycle: LifeCycle let getViewEvent: (index: number) => View - const mockDateNowT1 = 1500000000000 - const mockDateNowT2 = 1500001000000 - beforeEach(() => { - let isFirstPerformanceNow = true - spyOn(performance, 'now').and.callFake(() => { - if (isFirstPerformanceNow) { - isFirstPerformanceNow = false - return mockDateNowT1 - } - return mockDateNowT2 - }) - jasmine.clock().install() + jasmine.clock().mockDate() + spyOn(performance, 'now').and.callFake(() => Date.now()) ;({ lifeCycle, getViewEvent } = spyOnViews()) }) @@ -167,19 +157,20 @@ describe('rum track load duration', () => { jasmine.clock().uninstall() }) - it('should set a loadingTime once the load is complete without having any activity', () => { + it('should have an undefined loading time if the load is complete without having any activity', () => { jasmine.clock().tick(AFTER_PAGE_ACTIVITY_MAX_DURATION) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadingTime).toEqual(mockDateNowT2 - mockDateNowT1) + expect(getViewEvent(1).loadingTime).toBeUndefined() }) - it('should set a loadingTime once the load is complete after having some activity', () => { + it('should have a loading time equal to the activity time if the load is complete with a unique activity', () => { jasmine.clock().tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) lifeCycle.notify(LifeCycleEventType.DOM_MUTATED) jasmine.clock().tick(AFTER_PAGE_ACTIVITY_END_DELAY) jasmine.clock().tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewEvent(1).loadingTime).toEqual(mockDateNowT2 - mockDateNowT1) + + expect(getViewEvent(1).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) }) }) From 7035c08832a7ebec603a16af1457b871030c1d18 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 18 May 2020 10:03:15 +0200 Subject: [PATCH 40/42] =?UTF-8?q?=E2=9C=85=20remove=20view=20collection=20?= =?UTF-8?q?dependence=20from=20user=20action=20testing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum/test/userActionCollection.spec.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index 9e1d2f2311..f9eb621c7b 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -1,13 +1,6 @@ import { DOM_EVENT, ErrorMessage } from '@datadog/browser-core' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' -import { RumSession } from '../src/rumSession' -import { - PAGE_ACTIVITY_END_DELAY, - PAGE_ACTIVITY_MAX_DURATION, - PAGE_ACTIVITY_VALIDATION_DELAY, - PageActivityEvent, - waitPageActivitiesCompletion, -} from '../src/trackPageActivities' +import { PAGE_ACTIVITY_MAX_DURATION, PAGE_ACTIVITY_VALIDATION_DELAY } from '../src/trackPageActivities' import { $$tests, AutoUserAction, @@ -17,7 +10,7 @@ import { UserActionType, } from '../src/userActionCollection' const { resetUserAction, newUserAction } = $$tests -import { startViewCollection, ViewLoadingType } from '../src/viewCollection' +import { View, ViewLoadingType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -113,7 +106,20 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - startViewCollection(fakeLocation as Location, lifeCycle, fakeSession as RumSession) + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { + documentVersion: 0, + duration: 0, + id: 'foo', + loadingType: ViewLoadingType.INITIAL_LOAD, + location: fakeLocation as Location, + measures: { + errorCount: 0, + longTaskCount: 0, + resourceCount: 0, + userActionCount: 0, + }, + startTime: 0, + }) clock.expire() expect(events).toEqual([]) From aa2180d43fb0181986af1dccb617467e9d9671b7 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 18 May 2020 10:53:23 +0200 Subject: [PATCH 41/42] =?UTF-8?q?=E2=9C=85=20clarify=20the=20LifeCycleEven?= =?UTF-8?q?tType.VIEW=5FCOLLECTED=20subscribe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum/test/userActionCollection.spec.ts | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/packages/rum/test/userActionCollection.spec.ts b/packages/rum/test/userActionCollection.spec.ts index f9eb621c7b..01fbb32728 100644 --- a/packages/rum/test/userActionCollection.spec.ts +++ b/packages/rum/test/userActionCollection.spec.ts @@ -9,8 +9,9 @@ import { UserAction, UserActionType, } from '../src/userActionCollection' +import { View } from '../src/viewCollection' + const { resetUserAction, newUserAction } = $$tests -import { View, ViewLoadingType } from '../src/viewCollection' // Used to wait some time after the creation of a user action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -91,13 +92,6 @@ describe('startUserActionCollection', () => { } it('cancels pending user action on view loading', () => { - const fakeLocation: Partial = { pathname: '/foo' } - const fakeSession = { - getId() { - return '42' - }, - } - button.addEventListener(DOM_EVENT.CLICK, () => { clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) // Since we don't collect dom mutations for this test, manually dispatch one @@ -106,20 +100,8 @@ describe('startUserActionCollection', () => { clock.tick(SOME_ARBITRARY_DELAY) button.click() - lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, { - documentVersion: 0, - duration: 0, - id: 'foo', - loadingType: ViewLoadingType.INITIAL_LOAD, - location: fakeLocation as Location, - measures: { - errorCount: 0, - longTaskCount: 0, - resourceCount: 0, - userActionCount: 0, - }, - startTime: 0, - }) + const fakeView = {} + lifeCycle.notify(LifeCycleEventType.VIEW_COLLECTED, fakeView as View) clock.expire() expect(events).toEqual([]) From 502a7d2d943f13ee839fea524e28fdc4c3ab0ef2 Mon Sep 17 00:00:00 2001 From: "maxime.quentin" Date: Mon, 18 May 2020 14:09:54 +0200 Subject: [PATCH 42/42] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20stop=20current=20?= =?UTF-8?q?user=20action?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/userActionCollection.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/rum/src/userActionCollection.ts b/packages/rum/src/userActionCollection.ts index eb42955544..c732097c2b 100644 --- a/packages/rum/src/userActionCollection.ts +++ b/packages/rum/src/userActionCollection.ts @@ -76,11 +76,6 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) const { eventCounts, stop: stopEventCountsTracking } = trackEventCounts(lifeCycle) - function stopUserAction() { - stopEventCountsTracking() - pendingAutoUserAction = undefined - } - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, (hadActivity, endTime) => { if (hadActivity) { lifeCycle.notify(LifeCycleEventType.USER_ACTION_COLLECTED, { @@ -96,15 +91,18 @@ function newUserAction(lifeCycle: LifeCycle, type: UserActionType, name: string) }, }) } - stopUserAction() + + stopEventCountsTracking() + pendingAutoUserAction = undefined }) pendingAutoUserAction = { id, startTime, stop() { - stopUserAction() + stopEventCountsTracking() stopWaitIdlePageActivity() + pendingAutoUserAction = undefined }, } }