From 7934617ace020012409b9cb7c576faa7779e4f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Mon, 9 May 2022 18:29:49 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9A=97=E2=9C=A8=20[RUMF-1209]=20collect=20ra?= =?UTF-8?q?ge=20clicks=20(#1488)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [RUMF-1209] introduce rage click chains If we need to collect rage click, we need to buffer similar clicks to make sure they don't need to be represented as a single "rage click" action. This commit introduce a concept of "click chain", grouping similar clicks together, and flushing them when we are sure that the click chain is "complete". * ✨ [RUMF-1209] collect rage clicks If the flushed click chain meet rage clicks conditions, let's generate a single action instead of one for each click. * 👌 rename "click action" to "potential click action" ... and other related namings * ✅ add an E2E test * 👌 improve RAGE_CLICK_THRESHOLD naming to be more explicit * 👌 rename flush to finalize * 👌 rename 'potential click action' to 'click' * 👌 only expose `event` instead of the whole `base` * 👌 add a test case for clicks with negative duration + ff enabled * 👌 single loop on clicks when finalizing the rage click chain * ✅ rename frustration_type to frustration.type in e2e tests * 👌 remove unneeded comments * 👌 add click.isStopped * 👌 rename timeout and move clearTimeouts * 👌 rename onClick to processClick * 👌 adjust comment * 👌 adjust comment * 👌 rename state PENDING to ONGOING * 👌 reorder functions in createRageClickChain --- packages/core/test/specHelper.ts | 2 + .../action/rageClickChain.spec.ts | 221 ++++++++++++++++++ .../action/rageClickChain.ts | 115 +++++++++ .../action/trackClickActions.spec.ts | 87 +++++-- .../action/trackClickActions.ts | 100 ++++++-- test/e2e/scenario/rum/actions.scenario.ts | 25 ++ 6 files changed, 508 insertions(+), 42 deletions(-) create mode 100644 packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.spec.ts create mode 100644 packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.ts diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 5e7e7cbd28..9db84ed4d1 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -316,6 +316,8 @@ class StubXhr extends StubEventEmitter { } } +export function createNewEvent(eventName: 'click', properties?: { [name: string]: unknown }): MouseEvent +export function createNewEvent(eventName: string, properties?: { [name: string]: unknown }): Event export function createNewEvent(eventName: string, properties: { [name: string]: unknown } = {}) { let event: Event if (typeof Event === 'function') { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.spec.ts new file mode 100644 index 0000000000..afcb885e39 --- /dev/null +++ b/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.spec.ts @@ -0,0 +1,221 @@ +import { noop, ONE_SECOND, timeStampNow } from '@datadog/browser-core' +import type { Clock } from '@datadog/browser-core/test/specHelper' +import { mockClock, createNewEvent } from '@datadog/browser-core/test/specHelper' +import { FrustrationType } from '../../../rawRumEvent.types' +import type { RageClickChain } from './rageClickChain' +import { + MAX_DISTANCE_BETWEEN_CLICKS, + MAX_DURATION_BETWEEN_CLICKS, + createRageClickChain, + isRage, +} from './rageClickChain' +import type { Click } from './trackClickActions' + +describe('createRageClickChain', () => { + let clickChain: RageClickChain | undefined + let clock: Clock + + beforeEach(() => { + clock = mockClock() + }) + + afterEach(() => { + clickChain?.stop() + clock.cleanup() + }) + + it('creates a click chain', () => { + clickChain = createRageClickChain(createFakeClick()) + expect(clickChain).toEqual({ + tryAppend: jasmine.any(Function), + stop: jasmine.any(Function), + }) + }) + + it('appends a click', () => { + clickChain = createRageClickChain(createFakeClick()) + expect(clickChain.tryAppend(createFakeClick())).toBe(true) + }) + + describe('finalize', () => { + it('finalizes if we try to append a non-similar click', () => { + const firstClick = createFakeClick({ target: document.documentElement }) + clickChain = createRageClickChain(firstClick) + firstClick.stop() + clickChain.tryAppend(createFakeClick({ target: document.body })) + expect(firstClick.validate).toHaveBeenCalled() + }) + + it('does not finalize until it waited long enough to ensure no other click can be appended', () => { + const firstClick = createFakeClick() + clickChain = createRageClickChain(firstClick) + firstClick.stop() + clock.tick(MAX_DURATION_BETWEEN_CLICKS - 1) + expect(firstClick.validate).not.toHaveBeenCalled() + clock.tick(1) + expect(firstClick.validate).toHaveBeenCalled() + }) + + it('does not finalize until all clicks are stopped', () => { + const firstClick = createFakeClick() + clickChain = createRageClickChain(firstClick) + clock.tick(MAX_DURATION_BETWEEN_CLICKS) + expect(firstClick.validate).not.toHaveBeenCalled() + firstClick.stop() + expect(firstClick.validate).toHaveBeenCalled() + }) + + it('finalizes when stopping the click chain', () => { + const firstClick = createFakeClick({ target: document.documentElement }) + clickChain = createRageClickChain(firstClick) + firstClick.stop() + clickChain.stop() + expect(firstClick.validate).toHaveBeenCalled() + }) + }) + + describe('clicks similarity', () => { + it('does not accept a click if its timestamp is long after the previous one', () => { + clickChain = createRageClickChain(createFakeClick()) + clock.tick(MAX_DURATION_BETWEEN_CLICKS) + expect(clickChain.tryAppend(createFakeClick())).toBe(false) + }) + + it('does not accept a click if its target is different', () => { + clickChain = createRageClickChain(createFakeClick({ target: document.documentElement })) + expect(clickChain.tryAppend(createFakeClick({ target: document.body }))).toBe(false) + }) + + it('does not accept a click if its location is far from the previous one', () => { + clickChain = createRageClickChain(createFakeClick({ clientX: 100, clientY: 100 })) + expect( + clickChain.tryAppend(createFakeClick({ clientX: 100, clientY: 100 + MAX_DISTANCE_BETWEEN_CLICKS + 1 })) + ).toBe(false) + }) + + it('considers clicks relative to the previous one', () => { + clickChain = createRageClickChain(createFakeClick()) + clock.tick(MAX_DURATION_BETWEEN_CLICKS - 1) + clickChain.tryAppend(createFakeClick()) + clock.tick(MAX_DURATION_BETWEEN_CLICKS - 1) + expect(clickChain.tryAppend(createFakeClick())).toBe(true) + }) + }) + + describe('when rage is detected', () => { + it('discards individual clicks', () => { + const clicks = [createFakeClick(), createFakeClick(), createFakeClick()] + createValidatedRageClickChain(clicks) + clicks.forEach((click) => expect(click.discard).toHaveBeenCalled()) + }) + + it('uses a clone of the first click to represent the rage click', () => { + const clicks = [createFakeClick(), createFakeClick(), createFakeClick()] + createValidatedRageClickChain(clicks) + expect(clicks[0].clonedClick).toBeTruthy() + expect(clicks[0].clonedClick?.validate).toHaveBeenCalled() + }) + + it('the rage click should have a "rage" frustration', () => { + const clicks = [createFakeClick(), createFakeClick(), createFakeClick()] + createValidatedRageClickChain(clicks) + const expectedFrustrations = new Set() + expectedFrustrations.add(FrustrationType.RAGE) + expect(clicks[0].clonedClick?.getFrustrations()).toEqual(expectedFrustrations) + }) + + it('the rage click should contains other clicks frustration', () => { + const clicks = [createFakeClick(), createFakeClick(), createFakeClick()] + clicks[1].addFrustration(FrustrationType.DEAD) + createValidatedRageClickChain(clicks) + expect(clicks[0].clonedClick?.getFrustrations().has(FrustrationType.RAGE)).toBe(true) + }) + + function createValidatedRageClickChain(clicks: Click[]) { + clickChain = createRageClickChain(clicks[0]) + clicks.slice(1).forEach((click) => clickChain!.tryAppend(click)) + clicks.forEach((click) => click.stop()) + clock.tick(MAX_DURATION_BETWEEN_CLICKS) + } + }) +}) + +describe('isRage', () => { + let clock: Clock + + beforeEach(() => { + clock = mockClock() + }) + + afterEach(() => { + clock.cleanup() + }) + + it('considers as rage three clicks happening at the same time', () => { + expect(isRage([createFakeClick(), createFakeClick(), createFakeClick()])).toBe(true) + }) + + it('does not consider as rage two clicks happening at the same time', () => { + expect(isRage([createFakeClick(), createFakeClick()])).toBe(false) + }) + + it('does not consider as rage a first click long before two fast clicks', () => { + const clicks = [createFakeClick()] + clock.tick(ONE_SECOND * 2) + clicks.push(createFakeClick(), createFakeClick()) + + expect(isRage(clicks)).toBe(false) + }) + + it('considers as rage a first click long before three fast clicks', () => { + const clicks = [createFakeClick()] + clock.tick(ONE_SECOND * 2) + clicks.push(createFakeClick(), createFakeClick(), createFakeClick()) + + expect(isRage(clicks)).toBe(true) + }) + + it('considers as rage three fast clicks long before a last click', () => { + const clicks = [createFakeClick(), createFakeClick(), createFakeClick()] + clock.tick(ONE_SECOND * 2) + clicks.push(createFakeClick()) + + expect(isRage(clicks)).toBe(true) + }) +}) + +function createFakeClick(eventPartial?: Partial): Click & { clonedClick?: Click } { + let onStopCallback = noop + let isStopped = false + let clonedClick: Click | undefined + const frustrations = new Set() + return { + event: createNewEvent('click', { + element: document.body, + clientX: 100, + clientY: 100, + timeStamp: timeStampNow(), + ...eventPartial, + }), + onStop: (newOnStopCallback) => { + onStopCallback = newOnStopCallback + }, + isStopped: () => isStopped, + stop: () => { + isStopped = true + onStopCallback() + }, + clone: () => { + clonedClick = createFakeClick(eventPartial) + return clonedClick + }, + discard: jasmine.createSpy(), + validate: jasmine.createSpy(), + addFrustration: (frustration) => frustrations.add(frustration), + getFrustrations: () => frustrations, + + get clonedClick() { + return clonedClick + }, + } +} diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.ts b/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.ts new file mode 100644 index 0000000000..3827f25fd8 --- /dev/null +++ b/packages/rum-core/src/domain/rumEventsCollection/action/rageClickChain.ts @@ -0,0 +1,115 @@ +import { monitor, ONE_SECOND, timeStampNow } from '@datadog/browser-core' +import { FrustrationType } from '../../../rawRumEvent.types' +import type { Click } from './trackClickActions' + +export interface RageClickChain { + tryAppend: (click: Click) => boolean + stop: () => void +} + +export const MAX_DURATION_BETWEEN_CLICKS = ONE_SECOND +export const MAX_DISTANCE_BETWEEN_CLICKS = 100 + +const enum RageClickChainStatus { + WaitingForMoreClicks, + WaitingForClicksToStop, + Finalized, +} + +export function createRageClickChain(firstClick: Click): RageClickChain { + const bufferedClicks: Click[] = [] + let status = RageClickChainStatus.WaitingForMoreClicks + let maxDurationBetweenClicksTimeout: number | undefined + const rageClick = firstClick.clone() + appendClick(firstClick) + + function appendClick(click: Click) { + click.onStop(tryFinalize) + bufferedClicks.push(click) + clearTimeout(maxDurationBetweenClicksTimeout) + maxDurationBetweenClicksTimeout = setTimeout(monitor(dontAcceptMoreClick), MAX_DURATION_BETWEEN_CLICKS) + } + + function tryFinalize() { + if (status === RageClickChainStatus.WaitingForClicksToStop && bufferedClicks.every((click) => click.isStopped())) { + status = RageClickChainStatus.Finalized + finalizeClicks(bufferedClicks, rageClick) + } + } + + function dontAcceptMoreClick() { + clearTimeout(maxDurationBetweenClicksTimeout) + if (status === RageClickChainStatus.WaitingForMoreClicks) { + status = RageClickChainStatus.WaitingForClicksToStop + tryFinalize() + } + } + + return { + tryAppend: (click) => { + if (status !== RageClickChainStatus.WaitingForMoreClicks) { + return false + } + + if ( + bufferedClicks.length > 0 && + !areEventsSimilar(bufferedClicks[bufferedClicks.length - 1].event, click.event) + ) { + dontAcceptMoreClick() + return false + } + + appendClick(click) + return true + }, + stop: () => { + dontAcceptMoreClick() + }, + } +} + +/** + * Checks whether two events are similar by comparing their target, position and timestamp + */ +function areEventsSimilar(first: MouseEvent, second: MouseEvent) { + return ( + first.target === second.target && + mouseEventDistance(first, second) <= MAX_DISTANCE_BETWEEN_CLICKS && + first.timeStamp - second.timeStamp <= MAX_DURATION_BETWEEN_CLICKS + ) +} + +function mouseEventDistance(origin: MouseEvent, other: MouseEvent) { + return Math.sqrt(Math.pow(origin.clientX - other.clientX, 2) + Math.pow(origin.clientY - other.clientY, 2)) +} + +function finalizeClicks(clicks: Click[], rageClick: Click) { + if (isRage(clicks)) { + clicks.forEach((click) => { + click.discard() + click.getFrustrations().forEach((frustration) => { + rageClick.addFrustration(frustration) + }) + }) + rageClick.addFrustration(FrustrationType.RAGE) + rageClick.validate(timeStampNow()) + } else { + rageClick.discard() + clicks.forEach((click) => click.validate()) + } +} + +const MIN_CLICKS_PER_SECOND_TO_CONSIDER_RAGE = 3 + +export function isRage(clicks: Click[]) { + // TODO: this condition should be improved to avoid reporting 3-click selection as rage click + for (let i = 0; i < clicks.length - (MIN_CLICKS_PER_SECOND_TO_CONSIDER_RAGE - 1); i += 1) { + if ( + clicks[i + MIN_CLICKS_PER_SECOND_TO_CONSIDER_RAGE - 1].event.timeStamp - clicks[i].event.timeStamp <= + ONE_SECOND + ) { + return true + } + } + return false +} diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts index a541a72829..ed858a83c0 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -11,6 +11,7 @@ import { PAGE_ACTIVITY_VALIDATION_DELAY } from '../../waitIdlePage' import type { ActionContexts } from './actionCollection' import type { ClickAction } from './trackClickActions' import { CLICK_ACTION_MAX_DURATION, trackClickActions } from './trackClickActions' +import { MAX_DURATION_BETWEEN_CLICKS } from './rageClickChain' // Used to wait some time after the creation of an action const BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY = PAGE_ACTIVITY_VALIDATION_DELAY * 0.8 @@ -93,16 +94,6 @@ describe('trackClickActions', () => { ]) }) - it('discards any pending click action with a negative duration', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, button, -1) - expect(findActionId()).not.toBeUndefined() - clock.tick(EXPIRE_DELAY) - - expect(events).toEqual([]) - expect(findActionId()).toBeUndefined() - }) - it('should keep track of previously validated click actions', () => { const { domMutationObservable, clock } = setupBuilder.build() const clickActionStartTime = relativeNow() @@ -148,7 +139,17 @@ describe('trackClickActions', () => { }) describe('without frustration-signals flag', () => { - it('discards pending click action on view created', () => { + it('discards any click action with a negative duration', () => { + const { domMutationObservable, clock } = setupBuilder.build() + emulateClickWithActivity(domMutationObservable, clock, button, -1) + expect(findActionId()).not.toBeUndefined() + clock.tick(EXPIRE_DELAY) + + expect(events).toEqual([]) + expect(findActionId()).toBeUndefined() + }) + + it('discards ongoing click action on view created', () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() emulateClickWithActivity(domMutationObservable, clock) expect(findActionId()).not.toBeUndefined() @@ -175,7 +176,7 @@ describe('trackClickActions', () => { expect(events[0].startClocks.timeStamp).toBe(firstClickTimeStamp) }) - it('discards an click action when nothing happens after a click', () => { + it('discards a click action when nothing happens after a click', () => { const { clock } = setupBuilder.build() emulateClickWithoutActivity() @@ -184,7 +185,7 @@ describe('trackClickActions', () => { expect(findActionId()).toBeUndefined() }) - it('ignores an click action if it fails to find a name', () => { + it('ignores a click action if it fails to find a name', () => { const { domMutationObservable, clock } = setupBuilder.build() emulateClickWithActivity(domMutationObservable, clock, emptyElement) expect(findActionId()).toBeUndefined() @@ -213,7 +214,17 @@ describe('trackClickActions', () => { resetExperimentalFeatures() }) - it("doesn't discard pending click action on view created", () => { + it('discards any click action with a negative duration', () => { + const { domMutationObservable, clock } = setupBuilder.build() + emulateClickWithActivity(domMutationObservable, clock, button, -1) + expect(findActionId()!.length).toEqual(2) + clock.tick(EXPIRE_DELAY) + + expect(events).toEqual([]) + expect(findActionId()).toEqual([]) + }) + + it("doesn't discard ongoing click action on view created", () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() emulateClickWithActivity(domMutationObservable, clock) expect(findActionId()).not.toBeUndefined() @@ -269,9 +280,48 @@ describe('trackClickActions', () => { expect(events.length).toBe(1) }) + describe('rage clicks', () => { + it('considers a chain of three clicks or more as a single action with "rage" frustration type', () => { + const { domMutationObservable, clock } = setupBuilder.build() + const firstClickTimeStamp = timeStampNow() + const actionDuration = 5 + emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) + emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) + emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) + + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].startClocks.timeStamp).toBe(firstClickTimeStamp) + expect(events[0].frustrationTypes).toEqual([FrustrationType.RAGE]) + expect(events[0].duration).toBe((MAX_DURATION_BETWEEN_CLICKS + 2 * actionDuration) as Duration) + }) + + it('aggregates frustrationTypes from all clicks', () => { + const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() + + // Dead + emulateClickWithoutActivity() + clock.tick(PAGE_ACTIVITY_VALIDATION_DELAY) + + // Error + emulateClickWithActivity(domMutationObservable, clock) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, RAW_ERROR_EVENT) + clock.tick(PAGE_ACTIVITY_VALIDATION_DELAY) + + // Third click to make a rage click + emulateClickWithActivity(domMutationObservable, clock) + + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].frustrationTypes).toEqual( + jasmine.arrayWithExactContents([FrustrationType.DEAD, FrustrationType.ERROR, FrustrationType.RAGE]) + ) + }) + }) + describe('error clicks', () => { // eslint-disable-next-line max-len - it('considers a "click with activity" followed by an error as an click action with "error" frustration type', () => { + it('considers a "click with activity" followed by an error as a click action with "error" frustration type', () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() emulateClickWithActivity(domMutationObservable, clock) @@ -283,7 +333,7 @@ describe('trackClickActions', () => { }) // eslint-disable-next-line max-len - it('considers a "click without activity" followed by an error as an click action with "error" (and "dead") frustration type', () => { + it('considers a "click without activity" followed by an error as a click action with "error" (and "dead") frustration type', () => { const { lifeCycle, clock } = setupBuilder.build() emulateClickWithoutActivity() @@ -327,8 +377,9 @@ describe('trackClickActions', () => { target.dispatchEvent( createNewEvent('click', { target, - clientX: targetPosition.x + targetPosition.width / 2, - clientY: targetPosition.y + targetPosition.height / 2, + clientX: targetPosition.left + targetPosition.width / 2, + clientY: targetPosition.top + targetPosition.height / 2, + timeStamp: timeStampNow(), }) ) } diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index a508d5cf7d..566d534c62 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -1,6 +1,7 @@ import type { Duration, ClocksState, RelativeTime, TimeStamp, Subscription } from '@datadog/browser-core' import { setToArray, + noop, Observable, assign, isExperimentalFeatureEnabled, @@ -20,6 +21,8 @@ import type { LifeCycle } from '../../lifeCycle' import { LifeCycleEventType } from '../../lifeCycle' import { trackEventCounts } from '../../trackEventCounts' import { waitIdlePage } from '../../waitIdlePage' +import type { RageClickChain } from './rageClickChain' +import { createRageClickChain } from './rageClickChain' import { getActionNameFromElement } from './getActionNameFromElement' interface ActionCounts { @@ -35,7 +38,7 @@ export interface ClickAction { startClocks: ClocksState duration?: Duration counts: ActionCounts - event: Event + event: MouseEvent frustrationTypes: FrustrationType[] } @@ -58,12 +61,19 @@ export function trackClickActions( const collectFrustrations = isExperimentalFeatureEnabled('frustration-signals') const history: ClickActionIdHistory = new ContextHistory(ACTION_CONTEXT_TIME_OUT_DELAY) const stopObservable = new Observable() + let currentRageClickChain: RageClickChain | undefined lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { history.reset() }) - const { stop: stopListener } = listenClickEvents(onClick) + lifeCycle.subscribe(LifeCycleEventType.BEFORE_UNLOAD, () => { + if (currentRageClickChain) { + currentRageClickChain.stop() + } + }) + + const { stop: stopListener } = listenClickEvents(processClick) const actionContexts: ActionContexts = { findActionId: (startTime?: RelativeTime) => @@ -72,13 +82,16 @@ export function trackClickActions( return { stop: () => { + if (currentRageClickChain) { + currentRageClickChain.stop() + } stopObservable.notify() stopListener() }, actionContexts, } - function onClick(event: MouseEvent & { target: Element }) { + function processClick(event: MouseEvent & { target: Element }) { if (!collectFrustrations && history.find()) { // TODO: remove this in a future major version. To keep retrocompatibility, ignore any new // action if another one is already occurring. @@ -94,12 +107,16 @@ export function trackClickActions( const startClocks = clocksNow() - const potentialClickAction = newPotentialClickAction(lifeCycle, history, collectFrustrations, { + const click = newClick(lifeCycle, history, collectFrustrations, { name, event, startClocks, }) + if (collectFrustrations && (!currentRageClickChain || !currentRageClickChain.tryAppend(click))) { + currentRageClickChain = createRageClickChain(click) + } + const { stop: stopWaitingIdlePage } = waitIdlePage( lifeCycle, domMutationObservable, @@ -108,17 +125,20 @@ export function trackClickActions( // If it has no activity, consider it as a dead click. // TODO: this will yield a lot of false positive. We'll need to refine it in the future. if (collectFrustrations) { - potentialClickAction.addFrustration(FrustrationType.DEAD) - potentialClickAction.validate() + click.addFrustration(FrustrationType.DEAD) + click.stop() } else { - potentialClickAction.discard() + click.discard() } } else if (idleEvent.end < startClocks.timeStamp) { - // If the clock is looking weird, just discard the action - potentialClickAction.discard() + // If the clock is looking weird, just discard the click + click.discard() + } else if (collectFrustrations) { + // If we collect frustrations, let's stop the click, but validate it later + click.stop(idleEvent.end) } else { - // Else validate the potential click action at the end of the page activity - potentialClickAction.validate(idleEvent.end) + // Else just validate it now + click.validate(idleEvent.end) } stopClickProcessing() }, @@ -127,8 +147,8 @@ export function trackClickActions( let viewCreatedSubscription: Subscription | undefined if (!collectFrustrations) { - // TODO: remove this in a future major version. To keep retrocompatibility, end the potential - // click action on a new view is created. + // TODO: remove this in a future major version. To keep backward compatibility, end the click when a + // new view is created. viewCreatedSubscription = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, stopClickProcessing) } @@ -136,7 +156,7 @@ export function trackClickActions( function stopClickProcessing() { // Cleanup any ongoing process - potentialClickAction.discard() + click.stop() if (viewCreatedSubscription) { viewCreatedSubscription.unsubscribe() } @@ -159,7 +179,23 @@ function listenClickEvents(callback: (clickEvent: MouseEvent & { target: Element ) } -function newPotentialClickAction( +const enum ClickStatus { + // Initial state, the click is still ongoing. + ONGOING, + // The click is no more ongoing but still needs to be validated or discarded. + STOPPED, + // Final state, the click has been stopped and validated or discarded. + FINALIZED, +} + +type ClickState = + | { status: ClickStatus.ONGOING } + | { status: ClickStatus.STOPPED; endTime?: TimeStamp } + | { status: ClickStatus.FINALIZED } + +export type Click = ReturnType + +function newClick( lifeCycle: LifeCycle, history: ClickActionIdHistory, collectFrustrations: boolean, @@ -168,18 +204,22 @@ function newPotentialClickAction( const id = generateUUID() const historyEntry = history.add(id, base.startClocks.relative) const eventCountsSubscription = trackEventCounts(lifeCycle) - let isStopped = false - + let state: ClickState = { status: ClickStatus.ONGOING } const frustrations = new Set() + let onStopCallback = noop function stop(endTime?: TimeStamp) { - isStopped = true + if (state.status !== ClickStatus.ONGOING) { + return + } + state = { status: ClickStatus.STOPPED, endTime } if (endTime) { historyEntry.close(getRelativeTime(endTime)) } else { historyEntry.remove() } eventCountsSubscription.stop() + onStopCallback() } function addFrustration(frustration: FrustrationType) { @@ -189,13 +229,26 @@ function newPotentialClickAction( } return { + event: base.event, addFrustration, + stop, + + getFrustrations: () => frustrations, + + onStop: (newOnStopCallback: () => void) => { + onStopCallback = newOnStopCallback + }, + + isStopped: () => state.status === ClickStatus.STOPPED || state.status === ClickStatus.FINALIZED, + + clone: () => newClick(lifeCycle, history, collectFrustrations, base), validate: (endTime?: TimeStamp) => { - if (isStopped) { + stop(endTime) + if (state.status !== ClickStatus.STOPPED) { return } - stop(endTime) + if (eventCountsSubscription.eventCounts.errorCount > 0) { addFrustration(FrustrationType.ERROR) } @@ -204,7 +257,7 @@ function newPotentialClickAction( const clickAction: ClickAction = assign( { type: ActionType.CLICK as const, - duration: endTime && elapsed(base.startClocks.timeStamp, endTime), + duration: state.endTime && elapsed(base.startClocks.timeStamp, state.endTime), id, frustrationTypes: setToArray(frustrations), counts: { @@ -216,13 +269,12 @@ function newPotentialClickAction( base ) lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, clickAction) + state = { status: ClickStatus.FINALIZED } }, discard: () => { - if (isStopped) { - return - } stop() + state = { status: ClickStatus.FINALIZED } }, } } diff --git a/test/e2e/scenario/rum/actions.scenario.ts b/test/e2e/scenario/rum/actions.scenario.ts index 9fb2851be1..77367b3370 100644 --- a/test/e2e/scenario/rum/actions.scenario.ts +++ b/test/e2e/scenario/rum/actions.scenario.ts @@ -138,6 +138,31 @@ describe('action collection', () => { expect(serverEvents.rumViews[0].view.frustration!.count).toBe(1) }) + createTest('collect a "rage click"') + .withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] }) + .withBody( + html` + + + ` + ) + .run(async ({ serverEvents }) => { + const button = await $('button') + await button.click() + await button.click() + await button.click() + await flushEvents() + const actionEvents = serverEvents.rumActions + + expect(actionEvents.length).toBe(1) + expect(actionEvents[0].action.frustration!.type).toEqual(['rage']) + }) + createTest('collect multiple frustrations in one action') .withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] }) .withBody(