From 436d2669f782beb2c88e5b8e6e0c8de46b4efbb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 30 Aug 2021 17:21:36 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9A=97=F0=9F=9A=A9=20[RUMF-989]=20add=20a=20?= =?UTF-8?q?feature=20flag=20to=20disable=20max=20activity=20duration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/trackActions.ts | 31 ++++--- .../view/trackViewMetrics.ts | 24 ++++-- .../rumEventsCollection/view/trackViews.ts | 8 +- .../view/viewCollection.ts | 9 +- .../src/domain/trackPageActivities.spec.ts | 86 ++++++++++++++----- .../src/domain/trackPageActivities.ts | 19 ++-- packages/rum-core/test/specHelper.ts | 1 + 7 files changed, 131 insertions(+), 47 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackActions.ts index 6465b28077..8735799cee 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackActions.ts @@ -50,9 +50,9 @@ export interface AutoActionCreatedEvent { export function trackActions( lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable, - { actionNameAttribute }: Configuration + configuration: Configuration ) { - const action = startActionManagement(lifeCycle, domMutationObservable) + const action = startActionManagement(lifeCycle, domMutationObservable, configuration) // New views trigger the discard of the current pending Action lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, () => { @@ -66,7 +66,7 @@ export function trackActions( if (!(event.target instanceof Element)) { return } - const name = getActionNameFromElement(event.target, actionNameAttribute) + const name = getActionNameFromElement(event.target, configuration.actionNameAttribute) if (!name) { return } @@ -84,7 +84,11 @@ export function trackActions( } } -function startActionManagement(lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable) { +function startActionManagement( + lifeCycle: LifeCycle, + domMutationObservable: DOMMutationObservable, + configuration: Configuration +) { let currentAction: PendingAutoAction | undefined let currentIdlePageActivitySubscription: { stop: () => void } @@ -97,14 +101,19 @@ function startActionManagement(lifeCycle: LifeCycle, domMutationObservable: DOMM const pendingAutoAction = new PendingAutoAction(lifeCycle, type, name, event) currentAction = pendingAutoAction - currentIdlePageActivitySubscription = waitIdlePageActivity(lifeCycle, domMutationObservable, (params) => { - if (params.hadActivity) { - pendingAutoAction.complete(params.endTime) - } else { - pendingAutoAction.discard() + currentIdlePageActivitySubscription = waitIdlePageActivity( + lifeCycle, + domMutationObservable, + configuration, + (params) => { + if (params.hadActivity) { + pendingAutoAction.complete(params.endTime) + } else { + pendingAutoAction.discard() + } + currentAction = undefined } - currentAction = undefined - }) + ) }, discardCurrent: () => { if (currentAction) { diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index ecfc80747b..51446c23e9 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -1,4 +1,4 @@ -import { Duration, noop, elapsed, round, timeStampNow } from '@datadog/browser-core' +import { Duration, noop, elapsed, round, timeStampNow, Configuration } from '@datadog/browser-core' import { supportPerformanceTimingEvent } from '../../../browser/performanceCollection' import { ViewLoadingType } from '../../../rawRumEvent.types' import { LifeCycle, LifeCycleEventType } from '../../lifeCycle' @@ -16,7 +16,8 @@ export function trackViewMetrics( lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable, scheduleViewUpdate: () => void, - loadingType: ViewLoadingType + loadingType: ViewLoadingType, + configuration: Configuration ) { const viewMetrics: ViewMetrics = { eventCounts: { @@ -39,6 +40,7 @@ export function trackViewMetrics( const { stop: stopActivityLoadingTimeTracking } = trackActivityLoadingTime( lifeCycle, domMutationObservable, + configuration, setActivityLoadingTime ) @@ -97,16 +99,22 @@ function trackLoadingTime(loadType: ViewLoadingType, callback: (loadingTime: Dur function trackActivityLoadingTime( lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable, + configuration: Configuration, callback: (loadingTimeValue: Duration | undefined) => void ) { const startTime = timeStampNow() - const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity(lifeCycle, domMutationObservable, (params) => { - if (params.hadActivity) { - callback(elapsed(startTime, params.endTime)) - } else { - callback(undefined) + const { stop: stopWaitIdlePageActivity } = waitIdlePageActivity( + lifeCycle, + domMutationObservable, + configuration, + (params) => { + if (params.hadActivity) { + callback(elapsed(startTime, params.endTime)) + } else { + callback(undefined) + } } - }) + ) return { stop: stopWaitIdlePageActivity } } diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts index b7b8b05044..40a54dff3a 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts @@ -11,6 +11,7 @@ import { timeStampNow, TimeStamp, display, + Configuration, } from '@datadog/browser-core' import { DOMMutationObservable } from '../../../browser/domMutationObservable' import { ViewLoadingType, ViewCustomTimings } from '../../../rawRumEvent.types' @@ -58,6 +59,7 @@ export function trackViews( lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable, areViewsTrackedAutomatically: boolean, + configuration: Configuration, initialViewName?: string ) { const { stop: stopInitialViewTracking, initialView } = trackInitialView(initialViewName) @@ -75,6 +77,7 @@ export function trackViews( location, ViewLoadingType.INITIAL_LOAD, document.referrer, + configuration, clocksOrigin(), name ) @@ -92,6 +95,7 @@ export function trackViews( location, ViewLoadingType.ROUTE_CHANGE, currentView.url, + configuration, startClocks, name ) @@ -172,6 +176,7 @@ function newView( initialLocation: Location, loadingType: ViewLoadingType, referrer: string, + configuration: Configuration, startClocks: ClocksState = clocksNow(), name?: string ) { @@ -198,7 +203,8 @@ function newView( lifeCycle, domMutationObservable, scheduleViewUpdate, - loadingType + loadingType, + configuration ) // Initial view update diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts index f9cf2f1be8..ae8806d745 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts @@ -29,7 +29,14 @@ export function startViewCollection( ) ) - return trackViews(location, lifeCycle, domMutationObservable, !configuration.trackViewsManually, initialViewName) + return trackViews( + location, + lifeCycle, + domMutationObservable, + !configuration.trackViewsManually, + configuration, + initialViewName + ) } function processViewUpdate( diff --git a/packages/rum-core/src/domain/trackPageActivities.spec.ts b/packages/rum-core/src/domain/trackPageActivities.spec.ts index 0bda199b1c..31810168cb 100644 --- a/packages/rum-core/src/domain/trackPageActivities.spec.ts +++ b/packages/rum-core/src/domain/trackPageActivities.spec.ts @@ -1,4 +1,4 @@ -import { noop, Observable, TimeStamp, timeStampNow } from '@datadog/browser-core' +import { Configuration, noop, Observable, TimeStamp, timeStampNow } from '@datadog/browser-core' import { Clock, mockClock } from '../../../core/test/specHelper' import { RumPerformanceNavigationTiming, RumPerformanceResourceTiming } from '../browser/performanceCollection' import { LifeCycle, LifeCycleEventType } from './lifeCycle' @@ -20,19 +20,6 @@ const BEFORE_PAGE_ACTIVITY_END_DELAY = PAGE_ACTIVITY_END_DELAY * 0.8 // A long delay used to wait after any action is finished. const EXPIRE_DELAY = PAGE_ACTIVITY_MAX_DURATION * 10 -function eventsCollector() { - const events: T[] = [] - beforeEach(() => { - events.length = 0 - }) - return { - events, - pushEvent: (event: T) => { - events.push(event) - }, - } -} - describe('trackPagePageActivities', () => { const { events, pushEvent } = eventsCollector() @@ -154,7 +141,7 @@ describe('waitPageActivitiesCompletion', () => { }) it('should not collect an event that is not followed by page activity', () => { - waitPageActivitiesCompletion(new Observable(), noop, completionCallbackSpy) + waitPageActivitiesCompletion(new Observable(), noop, createConfiguration(), completionCallbackSpy) clock.tick(EXPIRE_DELAY) @@ -167,7 +154,7 @@ describe('waitPageActivitiesCompletion', () => { const activityObservable = new Observable() const startTime = timeStampNow() - waitPageActivitiesCompletion(activityObservable, noop, completionCallbackSpy) + waitPageActivitiesCompletion(activityObservable, noop, createConfiguration(), completionCallbackSpy) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: false }) @@ -189,7 +176,7 @@ describe('waitPageActivitiesCompletion', () => { // Extend the 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, completionCallbackSpy) + waitPageActivitiesCompletion(activityObservable, noop, createConfiguration(), completionCallbackSpy) for (let i = 0; i < extendCount; i += 1) { clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) @@ -216,7 +203,7 @@ describe('waitPageActivitiesCompletion', () => { completionCallbackSpy.and.callFake(() => { stop = true }) - waitPageActivitiesCompletion(activityObservable, noop, completionCallbackSpy) + waitPageActivitiesCompletion(activityObservable, noop, createConfiguration(), completionCallbackSpy) for (let i = 0; i < extendCount && !stop; i += 1) { clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) @@ -231,13 +218,34 @@ describe('waitPageActivitiesCompletion', () => { endTime: (startTime + PAGE_ACTIVITY_MAX_DURATION) as TimeStamp, }) }) + + it('with eternal-page-activities, does not expire if the page is busy for too long', () => { + const activityObservable = new Observable() + + // Extend the 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, + createConfiguration('eternal-page-activities'), + completionCallbackSpy + ) + + for (let i = 0; i < extendCount; i += 1) { + clock.tick(BEFORE_PAGE_ACTIVITY_END_DELAY) + activityObservable.notify({ isBusy: false }) + } + + expect(completionCallbackSpy).not.toHaveBeenCalled() + }) }) describe('busy activities', () => { it('is extended while the page is busy', () => { const activityObservable = new Observable() const startTime = timeStampNow() - waitPageActivitiesCompletion(activityObservable, noop, completionCallbackSpy) + waitPageActivitiesCompletion(activityObservable, noop, createConfiguration(), completionCallbackSpy) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: true }) @@ -257,7 +265,7 @@ describe('waitPageActivitiesCompletion', () => { it('expires is the page is busy for too long', () => { const activityObservable = new Observable() const startTime = timeStampNow() - waitPageActivitiesCompletion(activityObservable, noop, completionCallbackSpy) + waitPageActivitiesCompletion(activityObservable, noop, createConfiguration(), completionCallbackSpy) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) activityObservable.notify({ isBusy: true }) @@ -270,5 +278,43 @@ describe('waitPageActivitiesCompletion', () => { endTime: (startTime + PAGE_ACTIVITY_MAX_DURATION) as TimeStamp, }) }) + + it('with eternal-page-activities, does not expire if the page is busy for too long', () => { + const activityObservable = new Observable() + waitPageActivitiesCompletion( + activityObservable, + noop, + createConfiguration('eternal-page-activities'), + completionCallbackSpy + ) + + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + activityObservable.notify({ isBusy: true }) + + clock.tick(EXPIRE_DELAY) + + expect(completionCallbackSpy).not.toHaveBeenCalled() + }) }) }) + +function eventsCollector() { + const events: T[] = [] + beforeEach(() => { + events.length = 0 + }) + return { + events, + pushEvent: (event: T) => { + events.push(event) + }, + } +} + +function createConfiguration(enabledFeatureFlag?: string) { + return { + isEnabled(flag: string) { + return enabledFeatureFlag === flag + }, + } as Configuration +} diff --git a/packages/rum-core/src/domain/trackPageActivities.ts b/packages/rum-core/src/domain/trackPageActivities.ts index 22b0daad2c..833d21b7d2 100644 --- a/packages/rum-core/src/domain/trackPageActivities.ts +++ b/packages/rum-core/src/domain/trackPageActivities.ts @@ -1,4 +1,4 @@ -import { monitor, Observable, Subscription, TimeStamp, timeStampNow } from '@datadog/browser-core' +import { Configuration, monitor, Observable, Subscription, TimeStamp, timeStampNow } from '@datadog/browser-core' import { DOMMutationObservable } from '../browser/domMutationObservable' import { LifeCycle, LifeCycleEventType } from './lifeCycle' @@ -18,6 +18,7 @@ export type CompletionCallbackParameters = { hadActivity: true; endTime: TimeSta export function waitIdlePageActivity( lifeCycle: LifeCycle, domMutationObservable: DOMMutationObservable, + configuration: Configuration, completionCallback: (params: CompletionCallbackParameters) => void ) { const { observable: pageActivitiesObservable, stop: stopPageActivitiesTracking } = trackPageActivities( @@ -28,6 +29,7 @@ export function waitIdlePageActivity( const { stop: stopWaitPageActivitiesCompletion } = waitPageActivitiesCompletion( pageActivitiesObservable, stopPageActivitiesTracking, + configuration, completionCallback ) @@ -121,6 +123,7 @@ export function trackPageActivities( export function waitPageActivitiesCompletion( pageActivitiesObservable: Observable, stopPageActivitiesTracking: () => void, + configuration: Configuration, completionCallback: (params: CompletionCallbackParameters) => void ): { stop: () => void } { let idleTimeoutId: number @@ -130,10 +133,12 @@ export function waitPageActivitiesCompletion( monitor(() => complete({ hadActivity: false })), PAGE_ACTIVITY_VALIDATION_DELAY ) - const maxDurationTimeoutId = setTimeout( - monitor(() => complete({ hadActivity: true, endTime: timeStampNow() })), - PAGE_ACTIVITY_MAX_DURATION - ) + const maxDurationTimeoutId = + !configuration.isEnabled('eternal-page-activities') && + setTimeout( + monitor(() => complete({ hadActivity: true, endTime: timeStampNow() })), + PAGE_ACTIVITY_MAX_DURATION + ) pageActivitiesObservable.subscribe(({ isBusy }) => { clearTimeout(validationTimeoutId) @@ -151,7 +156,9 @@ export function waitPageActivitiesCompletion( hasCompleted = true clearTimeout(validationTimeoutId) clearTimeout(idleTimeoutId) - clearTimeout(maxDurationTimeoutId) + if (maxDurationTimeoutId) { + clearTimeout(maxDurationTimeoutId) + } stopPageActivitiesTracking() } diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index 97fef5cb75..28fdf3e8b0 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -217,6 +217,7 @@ export function setupViewTest( lifeCycle, domMutationObservable, !configuration.trackViewsManually, + configuration, initialViewName ) return {