From 3425bbfcea5dcb09501d29350b184bcfa3e78f55 Mon Sep 17 00:00:00 2001 From: Hamza Kadiri Date: Wed, 16 Aug 2023 13:13:12 +0200 Subject: [PATCH] remove scrollmap experimental flag (#2374) --- .../view/trackViewMetrics.spec.ts | 188 ++++++------------ .../view/trackViewMetrics.ts | 19 +- 2 files changed, 70 insertions(+), 137 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 7ed6c3fc08..3284c24ea7 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -1,12 +1,5 @@ import type { Context, RelativeTime, TimeStamp, Duration } from '@datadog/browser-core' -import { - ExperimentalFeature, - addExperimentalFeatures, - resetExperimentalFeatures, - DOM_EVENT, - addDuration, - relativeNow, -} from '@datadog/browser-core' +import { DOM_EVENT, addDuration, relativeNow } from '@datadog/browser-core' import type { Clock } from '@datadog/browser-core/test' import { createNewEvent, mockClock } from '@datadog/browser-core/test' import type { RumEvent } from '../../../rumEvent.types' @@ -548,85 +541,58 @@ describe('rum track view metrics', () => { clock.tick(THROTTLE_SCROLL_DURATION) } - describe('with flag enabled', () => { - beforeEach(() => { - configuration = {} as RumConfiguration - clock = mockClock() - addExperimentalFeatures([ExperimentalFeature.SCROLLMAP]) - stopTrackScrollMetrics = trackScrollMetrics( - configuration, - { relative: 0 as RelativeTime, timeStamp: 0 as TimeStamp }, - (s) => (scrollMetrics = s), - getMetrics - ).stop - }) - - afterEach(() => { - stopTrackScrollMetrics() - resetExperimentalFeatures() - scrollMetrics = undefined - clock.cleanup() - }) - it('should update scroll metrics when scrolling the first time', () => { - newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 }) - - expect(scrollMetrics).toEqual({ - maxDepthScrollHeight: 1000, - maxDepth: 500, - maxDepthScrollTop: 100, - maxDepthTime: 1000 as Duration, - }) - }) + beforeEach(() => { + configuration = {} as RumConfiguration + clock = mockClock() + stopTrackScrollMetrics = trackScrollMetrics( + configuration, + { relative: 0 as RelativeTime, timeStamp: 0 as TimeStamp }, + (s) => (scrollMetrics = s), + getMetrics + ).stop + }) - it('should update scroll metrics when scroll depth has increased', () => { - newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 }) + afterEach(() => { + stopTrackScrollMetrics() + scrollMetrics = undefined + clock.cleanup() + }) - newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 }) + it('should update scroll metrics when scrolling the first time', () => { + newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 }) - expect(scrollMetrics).toEqual({ - maxDepthScrollHeight: 1000, - maxDepth: 600, - maxDepthScrollTop: 200, - maxDepthTime: 2000 as Duration, - }) + expect(scrollMetrics).toEqual({ + maxDepthScrollHeight: 1000, + maxDepth: 500, + maxDepthScrollTop: 100, + maxDepthTime: 1000 as Duration, }) + }) - it('should NOT update scroll metrics when scroll depth has not increased', () => { - newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 }) + it('should update scroll metrics when scroll depth has increased', () => { + newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 }) - newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 }) + newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 }) - expect(scrollMetrics).toEqual({ - maxDepthScrollHeight: 1000, - maxDepth: 600, - maxDepthScrollTop: 200, - maxDepthTime: 1000 as Duration, - }) + expect(scrollMetrics).toEqual({ + maxDepthScrollHeight: 1000, + maxDepth: 600, + maxDepthScrollTop: 200, + maxDepthTime: 2000 as Duration, }) }) - describe('with flag disabled', () => { - beforeEach(() => { - clock = mockClock() - stopTrackScrollMetrics = trackScrollMetrics( - configuration, - { relative: 1 as RelativeTime, timeStamp: 2 as TimeStamp }, - (s) => (scrollMetrics = s), - getMetrics - ).stop - }) + it('should NOT update scroll metrics when scroll depth has not increased', () => { + newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 }) - afterEach(() => { - stopTrackScrollMetrics() - resetExperimentalFeatures() - scrollMetrics = undefined - clock.cleanup() - }) - it('should NOT update scroll metrics when scrolling the first time', () => { - newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 }) + newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 }) - expect(scrollMetrics).toEqual(undefined) + expect(scrollMetrics).toEqual({ + maxDepthScrollHeight: 1000, + maxDepth: 600, + maxDepthScrollTop: 200, + maxDepthTime: 1000 as Duration, }) }) }) @@ -635,60 +601,34 @@ describe('rum track view metrics', () => { beforeEach(() => { setupBuilder.withFakeClock() }) + it('should have an undefined loading time and empty scroll metrics if there is no activity on a route change', () => { + const { clock } = setupBuilder.build() + const { getViewUpdate, getViewUpdateCount, startView } = viewTest - describe('with flag enabled', () => { - beforeEach(() => { - addExperimentalFeatures([ExperimentalFeature.SCROLLMAP]) - }) - - afterEach(() => { - resetExperimentalFeatures() - }) - it('should have an undefined loading time and empty scroll metrics if there is no activity on a route change', () => { - const { clock } = setupBuilder.build() - const { getViewUpdate, getViewUpdateCount, startView } = viewTest - - startView() - clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + startView() + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - expect(getViewUpdateCount()).toEqual(3) - expect(getViewUpdate(2).loadingTime).toBeUndefined() - expect(getViewUpdate(2).scrollMetrics).toEqual(undefined) - }) - - it('should have a loading time equal to the activity time and scroll metrics if there is a unique activity on a route change', () => { - const { domMutationObservable, clock } = setupBuilder.build() - const { getViewUpdate, startView } = viewTest - - startView() - clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - domMutationObservable.notify() - clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) - clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - - expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - expect(getViewUpdate(3).scrollMetrics).toEqual({ - maxDepthScrollHeight: jasmine.any(Number), - maxDepth: jasmine.any(Number), - maxDepthTime: jasmine.any(Number), - maxDepthScrollTop: jasmine.any(Number), - }) - }) + expect(getViewUpdateCount()).toEqual(3) + expect(getViewUpdate(2).loadingTime).toBeUndefined() + expect(getViewUpdate(2).scrollMetrics).toEqual(undefined) }) - describe('with flag disabled', () => { - it('should have a loading time equal to the activity time but no scroll metrics if there is a unique activity on a route change', () => { - const { domMutationObservable, clock } = setupBuilder.build() - const { getViewUpdate, startView } = viewTest - - startView() - clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - domMutationObservable.notify() - clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) - clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) - - expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) - expect(getViewUpdate(3).scrollMetrics).toEqual(undefined) + it('should have a loading time equal to the activity time and scroll metrics if there is a unique activity on a route change', () => { + const { domMutationObservable, clock } = setupBuilder.build() + const { getViewUpdate, startView } = viewTest + + startView() + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + domMutationObservable.notify() + clock.tick(AFTER_PAGE_ACTIVITY_END_DELAY) + clock.tick(THROTTLE_VIEW_UPDATE_PERIOD) + + expect(getViewUpdate(3).loadingTime).toEqual(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + expect(getViewUpdate(3).scrollMetrics).toEqual({ + maxDepthScrollHeight: jasmine.any(Number), + maxDepth: jasmine.any(Number), + maxDepthTime: jasmine.any(Number), + maxDepthScrollTop: jasmine.any(Number), }) }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts index b71febf126..e8f55b5f0a 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.ts @@ -1,7 +1,5 @@ import type { ClocksState, Duration, Observable, RelativeTime } from '@datadog/browser-core' import { - ExperimentalFeature, - isExperimentalFeatureEnabled, DOM_EVENT, ONE_SECOND, addEventListener, @@ -63,15 +61,13 @@ export function trackViewMetrics( // We compute scroll metrics at loading time to ensure we have scroll data when loading the view initially // This is to ensure that we have the depth data even if the user didn't scroll or if the view is not scrollable. - if (isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) { - const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues() + const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues() - scrollMetrics = { - maxDepth: scrollDepth, - maxDepthScrollHeight: scrollHeight, - maxDepthTime: newLoadingTime, - maxDepthScrollTop: scrollTop, - } + scrollMetrics = { + maxDepth: scrollDepth, + maxDepthScrollHeight: scrollHeight, + maxDepthTime: newLoadingTime, + maxDepthScrollTop: scrollTop, } scheduleViewUpdate() } @@ -124,9 +120,6 @@ export function trackScrollMetrics( callback: (scrollMetrics: ScrollMetrics) => void, getScrollValues = computeScrollValues ) { - if (!isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) { - return { stop: noop } - } let maxDepth = 0 const handleScrollEvent = throttle( () => {