Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

👷‍♀️ [RUM 5088] Reduce INP Null Target #2950

Merged
merged 44 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
2f089aa
Add telemetry for null INP target
cy-moi Jul 25, 2024
c031764
Format
cy-moi Jul 26, 2024
8fe1537
Add feature flag and reduce the amount of telemetry
cy-moi Jul 26, 2024
c072151
Format
cy-moi Jul 26, 2024
2ee800a
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Jul 29, 2024
518f9c6
Rename ff
cy-moi Jul 29, 2024
d6e9575
Rename
cy-moi Jul 30, 2024
784a92f
Simplify and collect isElementNode
cy-moi Jul 30, 2024
32dfdcf
Format
cy-moi Jul 30, 2024
18a0a3d
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Aug 20, 2024
29f89ab
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Aug 20, 2024
c98fd4b
Add selector timestamp implementation and telemetry
cy-moi Aug 20, 2024
48282ce
test in processPointerDown
cy-moi Aug 22, 2024
09b0bea
Test with both pointerup and pointerdown timestamp
cy-moi Aug 23, 2024
1c99ed5
remove unused function
cy-moi Aug 23, 2024
7d8b514
remove testing
cy-moi Aug 26, 2024
e9e1823
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Aug 29, 2024
afa3ed9
Format
cy-moi Aug 29, 2024
add9d1a
Add map clearing in view ended
cy-moi Aug 30, 2024
92daf77
format
cy-moi Aug 30, 2024
028ccd8
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Sep 9, 2024
33ecfda
Clear the map in perf timing unsubscribe
cy-moi Sep 9, 2024
d6a41d0
Add pointerUp selector collect and clear outdated entries
cy-moi Sep 11, 2024
62c4f63
Change one hour to one day
cy-moi Sep 11, 2024
74268f8
Fix bug in comparing time
cy-moi Sep 13, 2024
0b6e4a0
Remove unused import
cy-moi Sep 13, 2024
4ae666f
Clear the map at 10 seconds
cy-moi Sep 16, 2024
6afb5d5
Remove telemetry after experiments
cy-moi Sep 19, 2024
399fb3a
Fix for comments and remove telemetry feature flag
cy-moi Sep 20, 2024
a64b276
Fix type for mouse event
cy-moi Sep 20, 2024
e8356a8
Add trackClickAction test coverage
cy-moi Sep 23, 2024
9ba6691
Remove ff NULL_INP_TELEMETRY
cy-moi Sep 23, 2024
b2ad82b
Extract logic to a separate file and fix tests
cy-moi Oct 2, 2024
309100d
Separate cache object to functions
cy-moi Oct 7, 2024
e5b8894
Simplify and avoid verbose
cy-moi Oct 7, 2024
780f181
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Oct 7, 2024
bee48e3
Remove commented codes
cy-moi Oct 7, 2024
9d24694
Rename functions to be more precise
cy-moi Oct 7, 2024
986bfe0
Fix tests with relative now timestamp
cy-moi Oct 7, 2024
f029cd9
avoid using emulateClick with delay
cy-moi Oct 7, 2024
e4a2494
Avoid using arrow functions
cy-moi Oct 8, 2024
8d67742
Merge branch 'main' into congyao/RUM-5590-add-telemetry-for-INP-null
cy-moi Oct 8, 2024
768e1a4
Test with ci unit test with emulateClick
cy-moi Oct 9, 2024
6e862a6
Move unit tests to standalone file
cy-moi Oct 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/core/test/emulate/createNewEvent.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import type { MouseEventOnElement } from '@datadog/browser-rum-core'
import type { TrustableEvent } from '../../src'
import { objectEntries } from '../../src'

export function createNewEvent(eventName: 'click', properties?: Partial<MouseEvent>): MouseEvent
export function createNewEvent(
eventName: 'pointerup',
properties?: Partial<PointerEvent>
): PointerEvent & { target: Element }
export function createNewEvent(eventName: 'pointerup', properties?: Partial<PointerEvent>): MouseEventOnElement
export function createNewEvent(eventName: 'message', properties?: Partial<MessageEvent>): MessageEvent
export function createNewEvent(
eventName: 'securitypolicyviolation',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { relativeNow } from '@datadog/browser-core'
import { mockClock } from '@datadog/browser-core/test'
import type { Clock } from '@datadog/browser-core/test'
import {
updateInteractionSelector,
getInteractionSelector,
interactionSelectorCache,
CLICK_ACTION_MAX_DURATION,
} from './interactionSelectorCache'

describe('interactionSelectorCache', () => {
let clock: Clock
beforeEach(() => {
clock = mockClock()
})

afterEach(() => {
clock.cleanup()
})

it('should delete the selector after getting it', () => {
const timestamp = relativeNow()
updateInteractionSelector(timestamp, 'selector')
expect(getInteractionSelector(timestamp)).toBe('selector')
expect(interactionSelectorCache.get(timestamp)).toBeUndefined()
})

it('should delete outdated selectors', () => {
const timestamp = relativeNow()
updateInteractionSelector(timestamp, 'selector')
expect(getInteractionSelector(timestamp)).toBe('selector')
clock.tick(CLICK_ACTION_MAX_DURATION)
expect(interactionSelectorCache.get(timestamp)).toBeUndefined()
})
})
21 changes: 21 additions & 0 deletions packages/rum-core/src/domain/action/interactionSelectorCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { elapsed, ONE_SECOND, relativeNow } from '@datadog/browser-core'
import type { RelativeTime } from '@datadog/browser-core'

// Maximum duration for click actions
export const CLICK_ACTION_MAX_DURATION = 10 * ONE_SECOND
export const interactionSelectorCache = new Map<RelativeTime, string>()

export function getInteractionSelector(relativeTimestamp: RelativeTime) {
const selector = interactionSelectorCache.get(relativeTimestamp)
interactionSelectorCache.delete(relativeTimestamp)
return selector
}

export function updateInteractionSelector(relativeTimestamp: RelativeTime, selector: string) {
interactionSelectorCache.set(relativeTimestamp, selector)
interactionSelectorCache.forEach((_, relativeTimestamp) => {
if (elapsed(relativeTimestamp, relativeNow()) > CLICK_ACTION_MAX_DURATION) {
interactionSelectorCache.delete(relativeTimestamp)
}
})
}
7 changes: 6 additions & 1 deletion packages/rum-core/src/domain/action/listenActionEvents.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { addEventListener, DOM_EVENT } from '@datadog/browser-core'
import type { RelativeTime } from '@datadog/browser-core'
import type { RumConfiguration } from '../configuration'

export type MouseEventOnElement = PointerEvent & { target: Element }
export type ExtraPointerEventFields = {
target: Element
timeStamp: RelativeTime
}
export type MouseEventOnElement = PointerEvent & ExtraPointerEventFields

export interface UserActivity {
selection: boolean
Expand Down
25 changes: 23 additions & 2 deletions packages/rum-core/src/domain/action/trackClickActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import { PAGE_ACTIVITY_VALIDATION_DELAY } from '../waitPageActivityEnd'
import type { RumConfiguration } from '../configuration'
import type { ActionContexts } from './actionCollection'
import type { ClickAction } from './trackClickActions'
import { finalizeClicks, CLICK_ACTION_MAX_DURATION, trackClickActions } from './trackClickActions'
import { finalizeClicks, trackClickActions } from './trackClickActions'
import { MAX_DURATION_BETWEEN_CLICKS } from './clickChain'
import { getInteractionSelector, CLICK_ACTION_MAX_DURATION } from './interactionSelectorCache'

// Used to wait some time after the creation of an action
const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8
Expand Down Expand Up @@ -149,7 +150,6 @@ describe('trackClickActions', () => {
clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
domMutationObservable.notify()
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent())

clock.tick(EXPIRE_DELAY)
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent())

Expand Down Expand Up @@ -425,15 +425,35 @@ describe('trackClickActions', () => {
})
})

describe('interactionSelectorCache', () => {
it('should add pointer down to the map', () => {
startClickActionsTracking()
const timeStamp = relativeNow()

emulateClick({ eventProperty: { timeStamp } })
expect(getInteractionSelector(timeStamp)).toBe('#button')
})

it('should add pointerup to the map', () => {
startClickActionsTracking()
const timeStamp = relativeNow()

emulateClick({ eventProperty: { timeStamp } })
expect(getInteractionSelector(timeStamp)).toBe('#button')
})
})

function emulateClick({
target = button,
activity,
eventProperty,
}: {
target?: HTMLElement
activity?: {
delay?: number
on?: 'pointerup' | 'click' | 'pointerdown'
}
eventProperty?: { [key: string]: any }
} = {}) {
const targetPosition = target.getBoundingClientRect()
const offsetX = targetPosition.width / 2
Expand All @@ -446,6 +466,7 @@ describe('trackClickActions', () => {
offsetY,
timeStamp: timeStampNow(),
isPrimary: true,
...eventProperty,
}
target.dispatchEvent(createNewEvent('pointerdown', eventProperties))
emulateActivityIfNeeded('pointerdown')
Expand Down
15 changes: 11 additions & 4 deletions packages/rum-core/src/domain/action/trackClickActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
ONE_MINUTE,
generateUUID,
clocksNow,
ONE_SECOND,
elapsed,
createValueHistory,
} from '@datadog/browser-core'
Expand All @@ -27,6 +26,7 @@ import { getActionNameFromElement } from './getActionNameFromElement'
import type { MouseEventOnElement, UserActivity } from './listenActionEvents'
import { listenActionEvents } from './listenActionEvents'
import { computeFrustration } from './computeFrustration'
import { CLICK_ACTION_MAX_DURATION, updateInteractionSelector } from './interactionSelectorCache'

interface ActionCounts {
errorCount: number
Expand Down Expand Up @@ -58,8 +58,6 @@ export interface ActionContexts {

type ClickActionIdHistory = ValueHistory<ClickAction['id']>

// Maximum duration for click actions
export const CLICK_ACTION_MAX_DURATION = 10 * ONE_SECOND
export const ACTION_CONTEXT_TIME_OUT_DELAY = 5 * ONE_MINUTE // arbitrary

export function trackClickActions(
Expand Down Expand Up @@ -176,6 +174,11 @@ function startClickAction(
const click = newClick(lifeCycle, history, getUserActivity, clickActionBase, startEvent)
appendClickToClickChain(click)

const selector = clickActionBase?.target?.selector
if (selector) {
updateInteractionSelector(startEvent.timeStamp, selector)
}

const { stop: stopWaitPageActivityEnd } = waitPageActivityEnd(
lifeCycle,
domMutationObservable,
Expand Down Expand Up @@ -224,13 +227,17 @@ function computeClickActionBase(
configuration: RumConfiguration
): ClickActionBase {
const rect = event.target.getBoundingClientRect()
const selector = getSelectorFromElement(event.target, configuration.actionNameAttribute)
if (selector) {
updateInteractionSelector(event.timeStamp, selector)
}

return {
type: ActionType.CLICK,
target: {
width: Math.round(rect.width),
height: Math.round(rect.height),
selector: getSelectorFromElement(event.target, configuration.actionNameAttribute),
selector,
},
position: {
// Use clientX and Y because for SVG element offsetX and Y are relatives to the <svg> element
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Duration, RelativeTime } from '@datadog/browser-core'
import { elapsed, resetExperimentalFeatures } from '@datadog/browser-core'
import { elapsed, relativeNow, resetExperimentalFeatures } from '@datadog/browser-core'
import { registerCleanupTask } from '@datadog/browser-core/test'
import {
appendElement,
Expand All @@ -16,6 +16,7 @@ import type {
RumPerformanceEventTiming,
} from '../../../browser/performanceObservable'
import { ViewLoadingType } from '../../../rawRumEvent.types'
import { getInteractionSelector, updateInteractionSelector } from '../../action/interactionSelectorCache'
import {
trackInteractionToNextPaint,
trackViewInteractionCount,
Expand Down Expand Up @@ -247,6 +248,22 @@ describe('trackInteractionToNextPaint', () => {

expect(getInteractionToNextPaint()?.targetSelector).toEqual('#bar')
})

it('should check interactionSelectorCache for entries', () => {
startINPTracking()
const startTime = relativeNow()
updateInteractionSelector(startTime, '#foo')

newInteraction({
interactionId: 1,
duration: 1 as Duration,
startTime,
target: undefined,
})

expect(getInteractionToNextPaint()?.targetSelector).toEqual('#foo')
expect(getInteractionSelector(startTime)).toBeUndefined()
})
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { RumFirstInputTiming, RumPerformanceEventTiming } from '../../../br
import { ViewLoadingType } from '../../../rawRumEvent.types'
import { getSelectorFromElement } from '../../getSelectorFromElement'
import { isElementNode } from '../../../browser/htmlDomUtils'
import { getInteractionSelector } from '../../action/interactionSelectorCache'
import type { RumConfiguration } from '../../configuration'
import { getInteractionCount, initInteractionCountPolyfill } from './interactionCountPolyfill'

Expand Down Expand Up @@ -66,14 +67,12 @@ export function trackInteractionToNextPaint(
if (newInteraction && newInteraction.duration !== interactionToNextPaint) {
interactionToNextPaint = newInteraction.duration
interactionToNextPaintStartTime = elapsed(viewStart, newInteraction.startTime)

if (newInteraction.target && isElementNode(newInteraction.target)) {
interactionToNextPaintTargetSelector = getInteractionSelector(newInteraction.startTime)
if (!interactionToNextPaintTargetSelector && newInteraction.target && isElementNode(newInteraction.target)) {
interactionToNextPaintTargetSelector = getSelectorFromElement(
newInteraction.target,
configuration.actionNameAttribute
)
} else {
interactionToNextPaintTargetSelector = undefined
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ export { isLongDataUrl, sanitizeDataUrl, MAX_ATTRIBUTE_VALUE_CHAR_LENGTH } from
export * from './domain/privacy'
export { SessionReplayState } from './domain/rumSessionManager'
export type { RumPlugin } from './domain/plugins'
export type { MouseEventOnElement } from './domain/action/listenActionEvents'
4 changes: 2 additions & 2 deletions packages/rum-core/test/createFakeClick.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { clocksNow, Observable, timeStampNow } from '@datadog/browser-core'
import { createNewEvent } from '@datadog/browser-core/test'
import type { Click } from '../src/domain/action/trackClickActions'
import type { UserActivity } from '../src/domain/action/listenActionEvents'
import type { MouseEventOnElement, UserActivity } from '../src/domain/action/listenActionEvents'

export type FakeClick = Readonly<ReturnType<typeof createFakeClick>>

Expand All @@ -14,7 +14,7 @@ export function createFakeClick({
hasError?: boolean
hasPageActivity?: boolean
userActivity?: Partial<UserActivity>
event?: Partial<PointerEvent & { target: Element }>
event?: Partial<MouseEventOnElement>
} = {}) {
const stopObservable = new Observable<void>()
let isStopped = false
Expand Down