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-772] add beforeSend API #644

Merged
merged 15 commits into from
Dec 17, 2020
Merged
3 changes: 3 additions & 0 deletions packages/core/src/domain/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface UserConfiguration {
silentMultipleInit?: boolean
trackInteractions?: boolean
proxyHost?: string
beforeSend?: (event: any) => void

service?: string
env?: string
Expand Down Expand Up @@ -77,6 +78,7 @@ export type Configuration = typeof DEFAULT_CONFIGURATION & {
proxyHost?: string

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

isEnabled: (feature: string) => boolean
isIntakeUrl: (url: string) => boolean
Expand Down Expand Up @@ -140,6 +142,7 @@ export function buildConfiguration(userConfiguration: UserConfiguration, buildEn
const intakeType: IntakeType = userConfiguration.useAlternateIntakeDomains ? 'alternate' : 'classic'
const intakeUrls = getIntakeUrls(intakeType, transportConfiguration, userConfiguration.replica !== undefined)
const configuration: Configuration = {
beforeSend: userConfiguration.beforeSend,
cookieOptions: buildCookieOptions(userConfiguration),
isEnabled: (feature: string) => {
return includes(enableExperimentalFeatures, feature)
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@ export {
} from './browser/fetchProxy'
export { BoundedBuffer } from './tools/boundedBuffer'
export { createContextManager } from './tools/contextManager'
export { limitModification } from './tools/limitModification'

export * from './tools/specHelper'
98 changes: 98 additions & 0 deletions packages/core/src/tools/limitModification.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { limitModification } from './limitModification'

// tslint:disable: no-unsafe-any
describe('limitModification', () => {
it('should allow modifications on modifiable field', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.foo.bar = 'modified1'
candidate.qux = 'modified2'
}

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

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

it('should not allow modifications on non modifiable field', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.foo.bar = 'modified1'
candidate.qux = 'modified2'
}

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

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

it('should not allow to add a modifiable fields not present on the original object', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.foo.bar = 'modified1'
candidate.qux = 'modified2'
candidate.qix = 'modified3'
}

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

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

it('should not allow non string value on modifiable field', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.foo.bar = undefined
candidate.qux = 1234
}

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

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

it('should not allow structural change of the object', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.foo.bar = { qux: 'qux' }
candidate.bar = 'bar'
delete candidate.qux
}

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

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

it('should catch and log modifier exception', () => {
const object = { foo: { bar: 'bar' }, qux: 'qux' }
const modifier = (candidate: any) => {
candidate.qux = 'modified'
candidate.foo.qux.bar = 'will throw'
}
const errorSpy = spyOn(console, 'error')

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

expect(errorSpy).toHaveBeenCalled()
expect(result).toEqual({
foo: { bar: 'bar' },
qux: 'qux',
})
})
})
59 changes: 59 additions & 0 deletions packages/core/src/tools/limitModification.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Context, deepClone } from './context'

/**
* Current limitations:
* - field path do not support array, 'a.b.c' only
* - modifiable fields type must be string
*/
export function limitModification<T extends Context>(
object: T,
modifiableFieldPaths: string[],
modifier: (object: T) => void
): T {
const clone = deepClone(object)
try {
modifier(clone)
} catch (e) {
console.error(e)
return object
}
modifiableFieldPaths.forEach((path) => {
const originalValue = get(object, path)
const newValue = get(clone, path)
if (typeof originalValue === 'string' && typeof newValue === 'string') {
set(object, path, newValue)
}
})
return object
}

function get(object: unknown, path: string) {
let current = object
for (const field of path.split('.')) {
if (!isValidObjectContaining(current, field)) {
return
}
current = current[field]
}
return current
}

function set(object: unknown, path: string, value: string) {
let current = object
const fields = path.split('.')
for (let i = 0; i < fields.length; i += 1) {
const field = fields[i]
if (!isValidObjectContaining(current, field)) {
return
}
if (i !== fields.length - 1) {
current = current[field]
} else {
current[field] = value
}
}
}

function isValidObjectContaining(object: unknown, field: string): object is { [key: string]: unknown } {
return typeof object === 'object' && object !== null && field in object
}
3 changes: 2 additions & 1 deletion packages/logs/src/boot/logs.entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import {
UserConfiguration,
} from '@datadog/browser-core'
import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger'
import { LogsEvent } from '../logsEvent.types'
import { startLogs } from './logs'

export interface LogsUserConfiguration extends UserConfiguration {
forwardErrorsToLogs?: boolean
beforeSend?: (event: LogsEvent) => void
}

export interface LoggerConfiguration {
Expand Down Expand Up @@ -45,7 +47,6 @@ export function makeLogsGlobal(startLogsImpl: StartLogs) {
let sendLogStrategy = (message: LogsMessage, currentContext: Context) => {
beforeInitSendLog.add([message, currentContext])
}

const logger = new Logger(sendLog)

return makeGlobal({
Expand Down
95 changes: 60 additions & 35 deletions packages/logs/src/boot/logs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
import sinon from 'sinon'

import { Logger, LogsMessage, StatusType } from '../domain/logger'
import { assembleMessageContexts, doStartLogs } from './logs'
import { LogsEvent } from '../logsEvent.types'
import { buildAssemble, doStartLogs } from './logs'

interface SentMessage extends LogsMessage {
logger?: { name: string }
Expand Down Expand Up @@ -126,16 +127,35 @@ describe('logs', () => {
})
})

describe('assembleMessageContexts', () => {
it('assembles various contexts', () => {
expect(
assembleMessageContexts(
{ session_id: SESSION_ID, service: 'Service' },
{ foo: 'from-current-context' },
{ view: { url: 'http://from-rum-context.com', id: 'view-id' } },
DEFAULT_MESSAGE
)
).toEqual({
describe('assemble', () => {
let assemble: (message: LogsMessage, currentContext: Context) => Context | undefined
let beforeSend: (event: LogsEvent) => void

beforeEach(() => {
beforeSend = noop
assemble = buildAssemble(session, {
...(baseConfiguration as Configuration),
beforeSend: (x: LogsEvent) => beforeSend(x),
})
window.DD_RUM = {
getInternalContext: noop,
}
})

it('should not assemble when session is not tracked', () => {
sessionIsTracked = 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' },
})

const assembledMessage = assemble(DEFAULT_MESSAGE, { foo: 'from-current-context' })

expect(assembledMessage).toEqual({
foo: 'from-current-context',
message: DEFAULT_MESSAGE.message,
service: 'Service',
Expand All @@ -146,36 +166,41 @@ describe('logs', () => {
})

it('message context should take precedence over RUM context', () => {
expect(
assembleMessageContexts(
{},
{ session_id: 'from-rum-context' },
{},
{ ...DEFAULT_MESSAGE, session_id: 'from-message-context' }
).session_id
).toBe('from-message-context')
spyOn(window.DD_RUM!, 'getInternalContext').and.returnValue({ session_id: 'from-rum-context' })

const assembledMessage = assemble({ ...DEFAULT_MESSAGE, session_id: 'from-message-context' }, {})

expect(assembledMessage!.session_id).toBe('from-message-context')
})

it('RUM context should take precedence over current context', () => {
expect(
assembleMessageContexts(
{},
{ session_id: 'from-current-context' },
{ session_id: 'from-rum-context' },
DEFAULT_MESSAGE
).session_id
).toBe('from-rum-context')
spyOn(window.DD_RUM!, 'getInternalContext').and.returnValue({ session_id: 'from-rum-context' })

const assembledMessage = assemble(DEFAULT_MESSAGE, { session_id: 'from-current-context' })

expect(assembledMessage!.session_id).toBe('from-rum-context')
})

it('current context should take precedence over default context', () => {
expect(
assembleMessageContexts(
{ service: 'from-default-context' },
{ service: 'from-current-context' },
undefined,
DEFAULT_MESSAGE
).service
).toBe('from-current-context')
const assembledMessage = assemble(DEFAULT_MESSAGE, { service: 'from-current-context' })

expect(assembledMessage!.service).toBe('from-current-context')
})

it('should allow modification on sensitive field', () => {
beforeSend = (event: LogsEvent) => (event.message = 'modified')

const assembledMessage = assemble(DEFAULT_MESSAGE, {})

expect(assembledMessage!.message).toBe('modified')
})

it('should reject modification on non sensitive field', () => {
beforeSend = (event: LogsEvent) => ((event.service as any) = 'modified')

const assembledMessage = assemble(DEFAULT_MESSAGE, {})

expect(assembledMessage!.service).toBe('Service')
})
})

Expand Down
Loading