Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚗ Collect scroll metrics #2180

Merged
merged 50 commits into from
Jun 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
5d29647
Initial poc
hamza-kadiri Apr 20, 2023
8e28f17
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri Apr 20, 2023
2607d28
Fix conflict
hamza-kadiri Apr 20, 2023
96f8b87
Nudge CI
hamza-kadiri Apr 20, 2023
deea157
Fix event listener
hamza-kadiri Apr 20, 2023
c4bf47c
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri May 10, 2023
a53e403
Collect scroll metrics
hamza-kadiri May 10, 2023
cc680c3
remove test
hamza-kadiri May 10, 2023
137a6af
Move scrollX/scrollY to browser-rum=core
hamza-kadiri May 12, 2023
667c190
address code review comments
hamza-kadiri May 12, 2023
1bb8cd6
Fix duration
hamza-kadiri May 12, 2023
854a25d
rename max_depth_timestamp to max_depth_time
hamza-kadiri May 12, 2023
0f4291e
fix failing unit test
hamza-kadiri May 12, 2023
3506fc0
generate type
hamza-kadiri May 17, 2023
35e005d
Empty commit
hamza-kadiri May 17, 2023
e33b0b4
fix failing ts job
hamza-kadiri May 17, 2023
222abf8
remove initialization & add passive listener
hamza-kadiri May 22, 2023
d6aaeab
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri May 22, 2023
4207c96
rebase rum-events-format
hamza-kadiri May 22, 2023
dc1d836
Initialize scroll metrics on page load
hamza-kadiri May 24, 2023
348a5f0
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri May 25, 2023
1c40a50
rename scrollTop to maxScrollTop
hamza-kadiri May 25, 2023
3ca5814
fix unit test
hamza-kadiri May 25, 2023
dbcbd29
Empty commit
hamza-kadiri May 25, 2023
ab946b2
Empty commit
hamza-kadiri May 25, 2023
15dcead
submodule ref
hamza-kadiri May 25, 2023
d7f1fba
add timestamp to initial scroll event
hamza-kadiri May 25, 2023
034fa46
update unit tests
hamza-kadiri Jun 5, 2023
6c972cc
Add experimental flag
hamza-kadiri Jun 5, 2023
ca25d67
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri Jun 5, 2023
e7ef101
address code review comments
hamza-kadiri Jun 6, 2023
453bd69
update rum-events-format
hamza-kadiri Jun 8, 2023
ccb7bb8
remove min when calculating scroll depth
hamza-kadiri Jun 8, 2023
8fee8f0
non optional params in scroll metrics
hamza-kadiri Jun 9, 2023
bdb636b
rum events format update
hamza-kadiri Jun 9, 2023
c4fe4d5
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri Jun 12, 2023
6152b1d
address code review
hamza-kadiri Jun 13, 2023
b6105e2
address code review comments
hamza-kadiri Jun 15, 2023
34f259a
update schema
hamza-kadiri Jun 15, 2023
add4f37
update rum events format
hamza-kadiri Jun 15, 2023
41f84b0
update rum-events-format
hamza-kadiri Jun 19, 2023
eb33c6e
update .d.ts file
hamza-kadiri Jun 19, 2023
bcecb47
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri Jun 20, 2023
bb7f9b9
Fix flaky unit test
hamza-kadiri Jun 20, 2023
48a8f09
update submodule
hamza-kadiri Jun 21, 2023
9466bea
update submodule
hamza-kadiri Jun 23, 2023
fa6dc88
renaming nits
hamza-kadiri Jun 23, 2023
7d714fc
sync event format
hamza-kadiri Jun 23, 2023
5677a0c
Merge remote-tracking branch 'origin/main' into hamza/scrolldepth-poc
hamza-kadiri Jun 23, 2023
131d2f7
change naming
hamza-kadiri Jun 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/tools/experimentalFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum ExperimentalFeature {
PAGE_STATES = 'page_states',
COLLECT_FLUSH_REASON = 'collect_flush_reason',
NO_RESOURCE_DURATION_FROZEN_STATE = 'no_resource_duration_frozen_state',
SCROLLMAP = 'scrollmap',
}

const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { addEventListener, DOM_EVENT, isIE } from '@datadog/browser-core'
import { getScrollX, getScrollY } from './viewports'
import { getScrollX, getScrollY } from './scroll'

function isMobileSafari12() {
return /iPhone OS 12.* like Mac OS.* Version\/12.* Mobile.*Safari/.test(navigator.userAgent)
}

describe('layout viewport', () => {
describe('scroll', () => {
let shouldWaitForWindowScrollEvent: boolean

const addVerticalScrollBar = () => {
Expand Down
25 changes: 25 additions & 0 deletions packages/rum-core/src/browser/scroll.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export function getScrollX() {
let scrollX
const visual = window.visualViewport
if (visual) {
scrollX = visual.pageLeft - visual.offsetLeft
} else if (window.scrollX !== undefined) {
scrollX = window.scrollX
} else {
scrollX = window.pageXOffset || 0
}
return Math.round(scrollX)
}

export function getScrollY() {
let scrollY
const visual = window.visualViewport
if (visual) {
scrollY = visual.pageTop - visual.offsetTop
} else if (window.scrollY !== undefined) {
scrollY = window.scrollY
} else {
scrollY = window.pageYOffset || 0
}
return Math.round(scrollY)
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { Context, RelativeTime, Duration } from '@datadog/browser-core'
import { addDuration, relativeNow } from '@datadog/browser-core'
import type { Context, RelativeTime, TimeStamp, Duration } from '@datadog/browser-core'
import {
ExperimentalFeature,
addExperimentalFeatures,
resetExperimentalFeatures,
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'
import type { TestSetupBuilder } from '../../../../test'
import { setup } from '../../../../test'
Expand All @@ -11,6 +20,8 @@ import { PAGE_ACTIVITY_END_DELAY, PAGE_ACTIVITY_VALIDATION_DELAY } from '../../w
import { THROTTLE_VIEW_UPDATE_PERIOD } from './trackViews'
import type { ViewTest } from './setupViewTest.specHelper'
import { setupViewTest } from './setupViewTest.specHelper'
import type { ScrollMetrics } from './trackViewMetrics'
import { THROTTLE_SCROLL_DURATION, trackScrollMetrics } from './trackViewMetrics'

const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = (PAGE_ACTIVITY_VALIDATION_DELAY * 0.8) as Duration

Expand Down Expand Up @@ -519,4 +530,161 @@ describe('rum track view metrics', () => {
expect(getViewUpdate(2).cumulativeLayoutShift).toBe(0.5)
})
})

describe('scroll metrics', () => {
describe('on scroll', () => {
let scrollMetrics: ScrollMetrics | undefined
let stopTrackScrollMetrics: () => void
let clock: Clock
const getMetrics = jasmine.createSpy('getMetrics')

const newScroll = (scrollParams: { scrollHeight: number; scrollDepth: number; scrollTop: number }) => {
getMetrics.and.returnValue(scrollParams)

window.dispatchEvent(createNewEvent(DOM_EVENT.SCROLL))

clock.tick(THROTTLE_SCROLL_DURATION)
}
describe('with flag enabled', () => {
beforeEach(() => {
clock = mockClock()
addExperimentalFeatures([ExperimentalFeature.SCROLLMAP])
stopTrackScrollMetrics = trackScrollMetrics(
{ 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,
})
})

it('should update scroll metrics when scroll depth has increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 500, scrollTop: 100 })

newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })

expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 2000 as Duration,
})
})

it('should NOT update scroll metrics when scroll depth has not increased', () => {
newScroll({ scrollHeight: 1000, scrollDepth: 600, scrollTop: 200 })

newScroll({ scrollHeight: 1000, scrollDepth: 450, scrollTop: 50 })

expect(scrollMetrics).toEqual({
maxDepthScrollHeight: 1000,
maxDepth: 600,
maxDepthScrollTop: 200,
maxDepthTime: 1000 as Duration,
})
})
})

describe('with flag disabled', () => {
beforeEach(() => {
clock = mockClock()
stopTrackScrollMetrics = trackScrollMetrics(
{ relative: 1 as RelativeTime, timeStamp: 2 as TimeStamp },
(s) => (scrollMetrics = s),
getMetrics
).stop
})

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 })

expect(scrollMetrics).toEqual(undefined)
})
})
})

describe('on load', () => {
beforeEach(() => {
setupBuilder.withFakeClock()
})

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)

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),
})
})
})

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)
})
})
})
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import type { Duration, RelativeTime, Observable, ClocksState } from '@datadog/browser-core'
import { noop, round, ONE_SECOND, elapsed } from '@datadog/browser-core'
import type { ClocksState, Duration, Observable, RelativeTime } from '@datadog/browser-core'
import {
ExperimentalFeature,
isExperimentalFeatureEnabled,
DOM_EVENT,
ONE_SECOND,
addEventListener,
elapsed,
noop,
relativeNow,
round,
throttle,
} from '@datadog/browser-core'
import type { RumLayoutShiftTiming } from '../../../browser/performanceCollection'
import { supportPerformanceTimingEvent } from '../../../browser/performanceCollection'
import { ViewLoadingType } from '../../../rawRumEvent.types'
Expand All @@ -8,6 +19,19 @@ import type { LifeCycle } from '../../lifeCycle'
import { LifeCycleEventType } from '../../lifeCycle'
import { waitPageActivityEnd } from '../../waitPageActivityEnd'

import { getScrollY } from '../../../browser/scroll'
import { getViewportDimension } from '../../../browser/viewportObservable'

export interface ScrollMetrics {
maxDepth: number
maxDepthScrollHeight: number
maxDepthScrollTop: number
maxDepthTime: Duration
}

/** Arbitrary scroll throttle duration */
export const THROTTLE_SCROLL_DURATION = ONE_SECOND
hamza-kadiri marked this conversation as resolved.
Show resolved Hide resolved

export interface ViewMetrics {
loadingTime?: Duration
cumulativeLayoutShift?: number
Expand All @@ -23,6 +47,8 @@ export function trackViewMetrics(
) {
const viewMetrics: ViewMetrics = {}

let scrollMetrics: ScrollMetrics | undefined

const { stop: stopLoadingTimeTracking, setLoadEvent } = trackLoadingTime(
lifeCycle,
domMutationObservable,
Expand All @@ -31,10 +57,31 @@ export function trackViewMetrics(
viewStart,
(newLoadingTime) => {
viewMetrics.loadingTime = newLoadingTime

// 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.
Comment on lines +61 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ question: a fair amount of views don't have loading time, could it be bothering to not have scroll information for those views?
why not doing it at init?‏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not really a problem to not have scroll information for all the views, we can consider that this information is saved only when the user has spent enough time on the view (i.e. it has at least finished loading). Loading time is intersting because the page has usually reached a state where metrics like the scroll height are stable. It's true that it's not 100% reliable though, we sometimes send values before the page is fully loaded (See how small the scroll height can be here)

if (isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) {
amortemousque marked this conversation as resolved.
Show resolved Hide resolved
const { scrollHeight, scrollDepth, scrollTop } = computeScrollValues()

scrollMetrics = {
maxDepth: scrollDepth,
maxDepthScrollHeight: scrollHeight,
maxDepthTime: newLoadingTime,
maxDepthScrollTop: scrollTop,
}
}
scheduleViewUpdate()
}
)

const { stop: stopScrollMetricsTracking } = trackScrollMetrics(
viewStart,
(newScrollMetrics) => {
scrollMetrics = newScrollMetrics
},
computeScrollValues
)

let stopCLSTracking: () => void
if (isLayoutShiftSupported()) {
viewMetrics.cumulativeLayoutShift = 0
Expand All @@ -45,13 +92,70 @@ export function trackViewMetrics(
} else {
stopCLSTracking = noop
}

return {
stop: () => {
stopLoadingTimeTracking()
stopCLSTracking()
stopScrollMetricsTracking()
},
setLoadEvent,
viewMetrics,
getScrollMetrics: () => scrollMetrics,
}
}

export function trackScrollMetrics(
viewStart: ClocksState,
callback: (scrollMetrics: ScrollMetrics) => void,
getScrollValues = computeScrollValues
) {
if (!isExperimentalFeatureEnabled(ExperimentalFeature.SCROLLMAP)) {
return { stop: noop }
}
let maxDepth = 0
const handleScrollEvent = throttle(
() => {
const { scrollHeight, scrollDepth, scrollTop } = getScrollValues()

if (scrollDepth > maxDepth) {
const now = relativeNow()
const maxDepthTime = elapsed(viewStart.relative, now)
maxDepth = scrollDepth
callback({
maxDepth,
maxDepthScrollHeight: scrollHeight,
maxDepthTime,
maxDepthScrollTop: scrollTop,
})
}
},
THROTTLE_SCROLL_DURATION,
{ leading: false, trailing: true }
)

const { stop } = addEventListener(window, DOM_EVENT.SCROLL, handleScrollEvent.throttled, { passive: true })

return {
stop: () => {
handleScrollEvent.cancel()
stop()
},
}
}

function computeScrollValues() {
const scrollTop = getScrollY()

const { height } = getViewportDimension()

const scrollHeight = Math.round((document.scrollingElement || document.documentElement).scrollHeight)
const scrollDepth = Math.round(height + scrollTop)

return {
scrollHeight,
scrollDepth,
scrollTop,
}
}

Expand Down
Loading