From 860c6df93d258c83a417d52f26046d031852a4ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 10 May 2022 17:41:21 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9A=97=E2=9C=A8=20[RUMF-1210]=20add=20a=20`t?= =?UTF-8?q?rackFrustrations`=20initialization=20parameter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum-core/src/domain/configuration.spec.ts | 53 ++++++++++++++++++- packages/rum-core/src/domain/configuration.ts | 7 ++- .../action/trackClickActions.spec.ts | 9 ++-- .../action/trackClickActions.ts | 29 +++++----- 4 files changed, 73 insertions(+), 25 deletions(-) diff --git a/packages/rum-core/src/domain/configuration.spec.ts b/packages/rum-core/src/domain/configuration.spec.ts index 3d32be7dec..f7ed3441e6 100644 --- a/packages/rum-core/src/domain/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration.spec.ts @@ -1,4 +1,9 @@ -import { DefaultPrivacyLevel, display } from '@datadog/browser-core' +import { + DefaultPrivacyLevel, + display, + resetExperimentalFeatures, + updateExperimentalFeatures, +} from '@datadog/browser-core' import { validateAndBuildRumConfiguration } from './configuration' const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx', applicationId: 'xxx' } @@ -91,6 +96,52 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('trackFrustrations', () => { + describe('without frustration-signals flag', () => { + it('defaults to false', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackFrustrations).toBeFalse() + }) + + it('the initialization parameter is ignored, no matter its value', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: true })! + .trackFrustrations + ).toBeFalse() + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: 'foo' as any })! + .trackFrustrations + ).toBeFalse() + }) + }) + + describe('with frustration-signals flag', () => { + beforeEach(() => { + updateExperimentalFeatures(['frustration-signals']) + }) + afterEach(() => { + resetExperimentalFeatures() + }) + + it('defaults to false', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackFrustrations).toBeFalse() + }) + + it('the initialization parameter is set to provided value', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: true })! + .trackFrustrations + ).toBeTrue() + }) + + it('the initialization parameter the provided value is cast to boolean', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: 'foo' as any })! + .trackFrustrations + ).toBeTrue() + }) + }) + }) + describe('trackViewsManually', () => { it('defaults to false', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackViewsManually).toBeFalse() diff --git a/packages/rum-core/src/domain/configuration.ts b/packages/rum-core/src/domain/configuration.ts index 26aaa8d7d4..251933b415 100644 --- a/packages/rum-core/src/domain/configuration.ts +++ b/packages/rum-core/src/domain/configuration.ts @@ -1,5 +1,5 @@ -import type { Configuration, InitConfiguration } from '@datadog/browser-core' -import { +import type { Configuration, InitConfiguration} from '@datadog/browser-core'; +import { isExperimentalFeatureEnabled , assign, DefaultPrivacyLevel, display, @@ -24,6 +24,7 @@ export interface RumInitConfiguration extends InitConfiguration { // action options trackInteractions?: boolean | undefined + trackFrustrations?: boolean | undefined actionNameAttribute?: string | undefined // view options @@ -40,6 +41,7 @@ export interface RumConfiguration extends Configuration { defaultPrivacyLevel: DefaultPrivacyLevel replaySampleRate: number trackInteractions: boolean + trackFrustrations: boolean trackViewsManually: boolean version?: string } @@ -81,6 +83,7 @@ export function validateAndBuildRumConfiguration( replaySampleRate: initConfiguration.replaySampleRate ?? 100, allowedTracingOrigins: initConfiguration.allowedTracingOrigins ?? [], trackInteractions: !!initConfiguration.trackInteractions, + trackFrustrations: isExperimentalFeatureEnabled('frustration-signals') && !!initConfiguration.trackFrustrations, trackViewsManually: !!initConfiguration.trackViewsManually, defaultPrivacyLevel: objectHasValue(DefaultPrivacyLevel, initConfiguration.defaultPrivacyLevel) ? initConfiguration.defaultPrivacyLevel 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 ed858a83c0..d7b4826467 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -138,7 +138,7 @@ describe('trackClickActions', () => { expect(events[0].name).toBe('test-1') }) - describe('without frustration-signals flag', () => { + describe('without tracking frustrations', () => { it('discards any click action with a negative duration', () => { const { domMutationObservable, clock } = setupBuilder.build() emulateClickWithActivity(domMutationObservable, clock, button, -1) @@ -206,12 +206,9 @@ describe('trackClickActions', () => { }) }) - describe('with frustration-signals flag', () => { + describe('when tracking frustrations', () => { beforeEach(() => { - updateExperimentalFeatures(['frustration-signals']) - }) - afterEach(() => { - resetExperimentalFeatures() + setupBuilder.withConfiguration({ trackFrustrations: true }) }) it('discards any click action with a negative duration', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index 566d534c62..3cafd3c624 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -4,7 +4,6 @@ import { noop, Observable, assign, - isExperimentalFeatureEnabled, getRelativeTime, ONE_MINUTE, ContextHistory, @@ -55,10 +54,8 @@ export const ACTION_CONTEXT_TIME_OUT_DELAY = 5 * ONE_MINUTE // arbitrary export function trackClickActions( lifeCycle: LifeCycle, domMutationObservable: Observable, - { actionNameAttribute }: RumConfiguration + { actionNameAttribute, trackFrustrations }: RumConfiguration ) { - // TODO: this will be changed when we introduce a proper initialization parameter for it - const collectFrustrations = isExperimentalFeatureEnabled('frustration-signals') const history: ClickActionIdHistory = new ContextHistory(ACTION_CONTEXT_TIME_OUT_DELAY) const stopObservable = new Observable() let currentRageClickChain: RageClickChain | undefined @@ -77,7 +74,7 @@ export function trackClickActions( const actionContexts: ActionContexts = { findActionId: (startTime?: RelativeTime) => - collectFrustrations ? history.findAll(startTime) : history.find(startTime), + trackFrustrations ? history.findAll(startTime) : history.find(startTime), } return { @@ -92,14 +89,14 @@ export function trackClickActions( } function processClick(event: MouseEvent & { target: Element }) { - if (!collectFrustrations && history.find()) { + if (!trackFrustrations && history.find()) { // TODO: remove this in a future major version. To keep retrocompatibility, ignore any new // action if another one is already occurring. return } const name = getActionNameFromElement(event.target, actionNameAttribute) - if (!collectFrustrations && !name) { + if (!trackFrustrations && !name) { // TODO: remove this in a future major version. To keep retrocompatibility, ignore any action // with a blank name return @@ -107,13 +104,13 @@ export function trackClickActions( const startClocks = clocksNow() - const click = newClick(lifeCycle, history, collectFrustrations, { + const click = newClick(lifeCycle, history, trackFrustrations, { name, event, startClocks, }) - if (collectFrustrations && (!currentRageClickChain || !currentRageClickChain.tryAppend(click))) { + if (trackFrustrations && (!currentRageClickChain || !currentRageClickChain.tryAppend(click))) { currentRageClickChain = createRageClickChain(click) } @@ -124,7 +121,7 @@ export function trackClickActions( if (!idleEvent.hadActivity) { // 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) { + if (trackFrustrations) { click.addFrustration(FrustrationType.DEAD) click.stop() } else { @@ -133,8 +130,8 @@ export function trackClickActions( } else if (idleEvent.end < startClocks.timeStamp) { // 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 + } else if (trackFrustrations) { + // If we track frustrations, let's stop the click, but validate it later click.stop(idleEvent.end) } else { // Else just validate it now @@ -146,7 +143,7 @@ export function trackClickActions( ) let viewCreatedSubscription: Subscription | undefined - if (!collectFrustrations) { + if (!trackFrustrations) { // 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) @@ -198,7 +195,7 @@ export type Click = ReturnType function newClick( lifeCycle: LifeCycle, history: ClickActionIdHistory, - collectFrustrations: boolean, + trackFrustrations: boolean, base: Pick ) { const id = generateUUID() @@ -223,7 +220,7 @@ function newClick( } function addFrustration(frustration: FrustrationType) { - if (collectFrustrations) { + if (trackFrustrations) { frustrations.add(frustration) } } @@ -241,7 +238,7 @@ function newClick( isStopped: () => state.status === ClickStatus.STOPPED || state.status === ClickStatus.FINALIZED, - clone: () => newClick(lifeCycle, history, collectFrustrations, base), + clone: () => newClick(lifeCycle, history, trackFrustrations, base), validate: (endTime?: TimeStamp) => { stop(endTime)