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

⚗✨ [RUMF-1210] add a trackFrustrations initialization parameter #1524

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
78 changes: 77 additions & 1 deletion packages/rum-core/src/domain/configuration.spec.ts
Original file line number Diff line number Diff line change
@@ -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' }
Expand Down Expand Up @@ -81,6 +86,9 @@ describe('validateAndBuildRumConfiguration', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackInteractions: true })!.trackInteractions
).toBeTrue()
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackInteractions: false })!.trackInteractions
).toBeFalse()
})

it('the provided value is cast to boolean', () => {
Expand All @@ -91,6 +99,70 @@ 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()
})

it('does not impact "trackInteractions"', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: true })!
.trackInteractions
).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()
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: false })!
.trackFrustrations
).toBeFalse()
})

it('the initialization parameter the provided value is cast to boolean', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: 'foo' as any })!
.trackFrustrations
).toBeTrue()
})

it('implies "trackInteractions"', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackFrustrations: true })!
.trackInteractions
).toBeTrue()
})
bcaudan marked this conversation as resolved.
Show resolved Hide resolved
})
})

describe('trackViewsManually', () => {
it('defaults to false', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackViewsManually).toBeFalse()
Expand All @@ -101,6 +173,10 @@ describe('validateAndBuildRumConfiguration', () => {
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackViewsManually: true })!
.trackViewsManually
).toBeTrue()
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackViewsManually: false })!
.trackViewsManually
).toBeFalse()
})

it('the provided value is cast to boolean', () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/rum-core/src/domain/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Configuration, InitConfiguration } from '@datadog/browser-core'
import {
isExperimentalFeatureEnabled,
assign,
DefaultPrivacyLevel,
display,
Expand All @@ -24,6 +25,7 @@ export interface RumInitConfiguration extends InitConfiguration {

// action options
trackInteractions?: boolean | undefined
trackFrustrations?: boolean | undefined
actionNameAttribute?: string | undefined

// view options
Expand All @@ -40,6 +42,7 @@ export interface RumConfiguration extends Configuration {
defaultPrivacyLevel: DefaultPrivacyLevel
replaySampleRate: number
trackInteractions: boolean
trackFrustrations: boolean
trackViewsManually: boolean
version?: string
}
Expand Down Expand Up @@ -73,14 +76,17 @@ export function validateAndBuildRumConfiguration(
return
}

const trackFrustrations = isExperimentalFeatureEnabled('frustration-signals') && !!initConfiguration.trackFrustrations

return assign(
{
applicationId: initConfiguration.applicationId,
version: initConfiguration.version,
actionNameAttribute: initConfiguration.actionNameAttribute,
replaySampleRate: initConfiguration.replaySampleRate ?? 100,
allowedTracingOrigins: initConfiguration.allowedTracingOrigins ?? [],
trackInteractions: !!initConfiguration.trackInteractions,
trackInteractions: !!initConfiguration.trackInteractions || trackFrustrations,
bcaudan marked this conversation as resolved.
Show resolved Hide resolved
trackFrustrations,
trackViewsManually: !!initConfiguration.trackViewsManually,
defaultPrivacyLevel: objectHasValue(DefaultPrivacyLevel, initConfiguration.defaultPrivacyLevel)
? initConfiguration.defaultPrivacyLevel
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Context, ClocksState, Observable, Duration } from '@datadog/browser-core'
import { timeStampNow, resetExperimentalFeatures, updateExperimentalFeatures, relativeNow } from '@datadog/browser-core'
import { timeStampNow, relativeNow } from '@datadog/browser-core'
import type { Clock } from '../../../../../core/test/specHelper'
import { createNewEvent } from '../../../../../core/test/specHelper'
import type { TestSetupBuilder } from '../../../../test/specHelper'
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
noop,
Observable,
assign,
isExperimentalFeatureEnabled,
getRelativeTime,
ONE_MINUTE,
ContextHistory,
Expand Down Expand Up @@ -55,10 +54,8 @@ export const ACTION_CONTEXT_TIME_OUT_DELAY = 5 * ONE_MINUTE // arbitrary
export function trackClickActions(
lifeCycle: LifeCycle,
domMutationObservable: Observable<void>,
{ 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<void>()
let currentRageClickChain: RageClickChain | undefined
Expand All @@ -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 {
Expand All @@ -92,28 +89,28 @@ 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
}

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

Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -198,7 +195,7 @@ export type Click = ReturnType<typeof newClick>
function newClick(
lifeCycle: LifeCycle,
history: ClickActionIdHistory,
collectFrustrations: boolean,
trackFrustrations: boolean,
base: Pick<ClickAction, 'startClocks' | 'event' | 'name'>
) {
const id = generateUUID()
Expand All @@ -223,7 +220,7 @@ function newClick(
}

function addFrustration(frustration: FrustrationType) {
if (collectFrustrations) {
if (trackFrustrations) {
frustrations.add(frustration)
}
}
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/scenario/rum/actions.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('action collection', () => {
})

createTest('collect an "error click"')
.withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(
html`
<button>click me</button>
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('action collection', () => {
})

createTest('collect a "dead click"')
.withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(html` <button>click me</button> `)
.run(async ({ serverEvents }) => {
const button = await $('button')
Expand All @@ -139,7 +139,7 @@ describe('action collection', () => {
})

createTest('collect a "rage click"')
.withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(
html`
<button>click me</button>
Expand All @@ -164,7 +164,7 @@ describe('action collection', () => {
})

createTest('collect multiple frustrations in one action')
.withRum({ trackInteractions: true, enableExperimentalFeatures: ['frustration-signals'] })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['frustration-signals'] })
.withBody(
html`
<button>click me</button>
Expand Down