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-854] Enable beforeSend to dismiss events #743

Merged
merged 10 commits into from
Feb 23, 2021
2 changes: 1 addition & 1 deletion packages/core/src/domain/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export type Configuration = typeof DEFAULT_CONFIGURATION &
cookieOptions: CookieOptions

service?: string
beforeSend?: (event: any) => void
beforeSend?: (event: any) => void | boolean

isEnabled: (feature: string) => boolean
}
Expand Down
39 changes: 27 additions & 12 deletions packages/core/src/tools/limitModification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ describe('limitModification', () => {
candidate.qux = 'modified2'
}

const result = limitModification(object, ['foo.bar', 'qux'], modifier)
limitModification(object, ['foo.bar', 'qux'], modifier)

expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'modified1' },
qux: 'modified2',
})
Expand All @@ -23,9 +23,9 @@ describe('limitModification', () => {
candidate.qux = 'modified2'
}

const result = limitModification(object, ['foo.bar'], modifier)
limitModification(object, ['foo.bar'], modifier)

expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'modified1' },
qux: 'qux',
})
Expand All @@ -39,9 +39,9 @@ describe('limitModification', () => {
candidate.qix = 'modified3'
}

const result = limitModification(object, ['foo.bar', 'qux', 'qix'], modifier)
limitModification(object, ['foo.bar', 'qux', 'qix'], modifier)

expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'modified1' },
qux: 'modified2',
})
Expand All @@ -54,9 +54,9 @@ describe('limitModification', () => {
candidate.qux = 1234
}

const result = limitModification(object, ['foo.bar', 'qux'], modifier)
limitModification(object, ['foo.bar', 'qux'], modifier)

expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'bar' },
qux: 'qux',
})
Expand All @@ -70,14 +70,29 @@ describe('limitModification', () => {
delete candidate.qux
}

const result = limitModification(object, ['foo.bar', 'qux'], modifier)
limitModification(object, ['foo.bar', 'qux'], modifier)

expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'bar' },
qux: 'qux',
})
})

it('should return the return of the modifier', () => {
webNeat marked this conversation as resolved.
Show resolved Hide resolved
const object = { foo: { bar: 'bar' } }
const modifier = (candidate: any) => {
candidate.foo.bar = 'qux'
return false
}

const result = limitModification(object, ['foo.bar', 'qux'], modifier)

expect(result).toBe(false)
expect(object).toEqual({
foo: { bar: 'qux' },
})
})

it('should catch and log modifier exception', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
Expand All @@ -86,10 +101,10 @@ describe('limitModification', () => {
}
const errorSpy = spyOn(console, 'error')

const result = limitModification(object, ['foo.bar', 'qux'], modifier)
limitModification(object, ['foo.bar', 'qux'], modifier)

expect(errorSpy).toHaveBeenCalled()
expect(result).toEqual({
expect(object).toEqual({
foo: { bar: 'bar' },
qux: 'qux',
})
Expand Down
13 changes: 7 additions & 6 deletions packages/core/src/tools/limitModification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ import { Context, deepClone } from './context'
* - field path do not support array, 'a.b.c' only
* - modifiable fields type must be string
*/
export function limitModification<T extends Context>(
export function limitModification<T extends Context, Result>(
object: T,
modifiableFieldPaths: string[],
modifier: (object: T) => void
): T {
modifier: (object: T) => Result
): Result | undefined {
const clone = deepClone(object)
let result
try {
modifier(clone)
result = modifier(clone)
} catch (e) {
console.error(e)
return object
return
}
modifiableFieldPaths.forEach((path) => {
const originalValue = get(object, path)
Expand All @@ -24,7 +25,7 @@ export function limitModification<T extends Context>(
set(object, path, newValue)
}
})
return object
return result
}

function get(object: unknown, path: string) {
Expand Down
7 changes: 6 additions & 1 deletion packages/logs/src/boot/logs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('logs', () => {

describe('assemble', () => {
let assemble: (message: LogsMessage, currentContext: Context) => Context | undefined
let beforeSend: (event: LogsEvent) => void
let beforeSend: (event: LogsEvent) => void | boolean

beforeEach(() => {
beforeSend = noop
Expand All @@ -148,6 +148,11 @@ describe('logs', () => {
expect(assemble(DEFAULT_MESSAGE, { foo: 'from-current-context' })).toBeUndefined()
})

it('should not assemble if beforeSend returned false', () => {
beforeSend = () => false
expect(assemble(DEFAULT_MESSAGE, { foo: 'from-current-context' })).toBeUndefined()
})

it('add default, current and RUM context to message', () => {
spyOn(window.DD_RUM!, 'getInternalContext').and.returnValue({
view: { url: 'http://from-rum-context.com', id: 'view-id' },
Expand Down
5 changes: 4 additions & 1 deletion packages/logs/src/boot/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,14 @@ export function buildAssemble(session: LoggerSession, configuration: Configurati
message
)
if (configuration.beforeSend) {
limitModification(
const result = limitModification(
bcaudan marked this conversation as resolved.
Show resolved Hide resolved
contextualizedMessage as LogsEvent & Context,
FIELDS_WITH_SENSITIVE_DATA,
configuration.beforeSend
)
if (result === false) {
return undefined
}
}
return contextualizedMessage as Context
}
Expand Down
50 changes: 50 additions & 0 deletions packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,56 @@ describe('rum assembly', () => {

expect(serverRumEvents[0].view.id).toBe('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee')
})

it('should allow dismissing events other than views', () => {
beforeSend = () => false

lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, {
rawRumEvent: createRawRumEvent(RumEventType.ACTION, {
view: { id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' },
}),
startTime: 0,
})

lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, {
rawRumEvent: createRawRumEvent(RumEventType.ERROR, {
view: { id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' },
}),
startTime: 0,
})

lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, {
rawRumEvent: createRawRumEvent(RumEventType.LONG_TASK, {
view: { id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' },
}),
startTime: 0,
})

lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, {
rawRumEvent: createRawRumEvent(RumEventType.RESOURCE, {
view: { id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' },
}),
startTime: 0,
})

expect(serverRumEvents.length).toBe(0)
})

it('should not allow dismissing view events', () => {
beforeSend = () => false
const consoleWarnSpy = spyOn(console, 'warn')
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW, {
view: { id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' },
}),
startTime: 0,
})

expect(serverRumEvents[0].view.id).toBe('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee')
expect(consoleWarnSpy).toHaveBeenCalledWith(
`Can't dismiss view events using beforeSend, use onNewLocation instead!`
)
})
})

describe('rum context', () => {
Expand Down
19 changes: 15 additions & 4 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,27 @@ export function startRumAssembly(
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
;(serverRumEvent.usr as RumEvent['usr']) = commonContext.user as User & Context
}

if (configuration.beforeSend) {
limitModification(serverRumEvent, FIELDS_WITH_SENSITIVE_DATA, configuration.beforeSend)
if (shouldSend(serverRumEvent, configuration.beforeSend)) {
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, serverRumEvent)
}
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, serverRumEvent)
}
}
)
}

function shouldSend(event: RumEvent & Context, beforeSend?: (event: any) => any) {
if (beforeSend) {
const result = limitModification(event, FIELDS_WITH_SENSITIVE_DATA, beforeSend)
if (result === false && event.type !== RumEventType.VIEW) {
return false
}
if (result === false) {
console.warn(`Can't dismiss view events using beforeSend, use onNewLocation instead!`)
}
}
return true
}

function needToAssembleWithAction(
event: RawRumEvent
): event is RawRumErrorEvent | RawRumResourceEvent | RawRumLongTaskEvent {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DOM_EVENT } from '@datadog/browser-core'
import { createRawRumEvent } from '../../../../test/fixtures'
import { Context, DOM_EVENT } from '@datadog/browser-core'
import { RumEvent } from '../../../../../rum/src'
import { setup, TestSetupBuilder } from '../../../../test/specHelper'
import { RumEventType, ActionType } from '../../../rawRumEvent.types'
import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
Expand Down Expand Up @@ -174,21 +174,18 @@ describe('newAction', () => {

it('counts errors occurring during the action', () => {
const { lifeCycle, clock } = setupBuilder.build()
const collectedRawRumEvent = {
rawRumEvent: createRawRumEvent(RumEventType.ERROR),
startTime: 0,
}
const collectedRumEvent = { type: RumEventType.ERROR } as RumEvent & Context
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, pushEvent)

newClick('test-1')

lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, collectedRawRumEvent)
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, collectedRumEvent)
clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY)
lifeCycle.notify(LifeCycleEventType.DOM_MUTATED)
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, collectedRawRumEvent)
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, collectedRumEvent)

clock.tick(EXPIRE_DELAY)
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, collectedRawRumEvent)
lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, collectedRumEvent)

expect(events.length).toBe(1)
const action = events[0]
Expand Down
Loading