Skip to content

Commit

Permalink
✨ [RUMF-1510] Warn the user when a heavy context is used (#2120)
Browse files Browse the repository at this point in the history
  • Loading branch information
amortemousque authored Apr 3, 2023
1 parent ff79c34 commit 198e9e5
Show file tree
Hide file tree
Showing 18 changed files with 276 additions and 109 deletions.
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export * from './tools/urlPolyfill'
export * from './tools/timeUtils'
export * from './tools/utils'
export * from './tools/sanitize'
export * from './tools/getGlobalObject'
export * from './domain/eventRateLimiter/createEventRateLimiter'
export * from './tools/browserDetection'
export { sendToExtension } from './tools/sendToExtension'
Expand Down Expand Up @@ -91,6 +92,7 @@ export { initConsoleObservable, ConsoleLog } from './domain/console/consoleObser
export { BoundedBuffer } from './tools/boundedBuffer'
export { catchUserErrors } from './tools/catchUserErrors'
export { createContextManager, ContextManager } from './tools/contextManager'
export { warnIfCustomerDataLimitReached, CustomerDataType } from './tools/heavyCustomerDataWarning'
export { limitModification } from './tools/limitModification'
export { ContextHistory, ContextHistoryEntry, CLEAR_OLD_CONTEXTS_INTERVAL } from './tools/contextHistory'
export { readBytesFromStream } from './tools/readBytesFromStream'
Expand Down
105 changes: 77 additions & 28 deletions packages/core/src/tools/contextManager.spec.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,58 @@
import { createContextManager } from './contextManager'
import type { Clock } from '../../test'
import { mockClock } from '../../test'
import { BYTES_COMPUTATION_THROTTLING_DELAY, createContextManager } from './contextManager'
import { display } from './display'
import { CustomerDataType, CUSTOMER_DATA_BYTES_LIMIT } from './heavyCustomerDataWarning'

describe('createContextManager', () => {
let clock: Clock

let displaySpy: jasmine.Spy<typeof display.warn>

beforeEach(() => {
clock = mockClock()
displaySpy = spyOn(display, 'warn')
})

afterEach(() => {
clock.cleanup()
})

it('starts with an empty context', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
expect(manager.get()).toEqual({})
})

it('updates the context', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.set({ bar: 'foo' })

expect(manager.get()).toEqual({ bar: 'foo' })
})

it('updates the context without copy', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
const context = {}
manager.set(context)
expect(manager.get()).toBe(context)
})

it('completely replaces the context', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.set({ a: 'foo' })
expect(manager.get()).toEqual({ a: 'foo' })
manager.set({ b: 'foo' })
expect(manager.get()).toEqual({ b: 'foo' })
})

it('sets a context value', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.add('foo', 'bar')
expect(manager.get()).toEqual({ foo: 'bar' })
})

it('removes a context value', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.set({ a: 'foo', b: 'bar' })
manager.remove('a')
expect(manager.get()).toEqual({ b: 'bar' })
Expand All @@ -43,60 +61,91 @@ describe('createContextManager', () => {
})

it('should get a clone of the context from getContext', () => {
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
expect(manager.getContext()).toEqual(manager.getContext())
expect(manager.getContext()).not.toBe(manager.getContext())
})

it('should set a clone of context via setContext', () => {
const nestedObject = { foo: 'bar' }
const context = { nested: nestedObject }
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.setContext(context)
expect(manager.getContext().nested).toEqual(nestedObject)
expect(manager.getContext().nested).not.toBe(nestedObject)
})

it('should set a clone of the property via setContextProperty', () => {
const nestedObject = { foo: 'bar' }
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.setContextProperty('nested', nestedObject)
expect(manager.getContext().nested).toEqual(nestedObject)
expect(manager.getContext().nested).not.toBe(nestedObject)
})

it('should clear context object via clearContext', () => {
const context = { foo: 'bar' }
const manager = createContextManager()
const manager = createContextManager(CustomerDataType.User)
manager.setContext(context)
expect(manager.getContext()).toEqual(context)
manager.clearContext()
expect(manager.getContext()).toEqual({})
})

it('should compute the bytes count only if the context has been updated', () => {
const computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1)
const manager = createContextManager(computeBytesCountStub)
describe('bytes count computation', () => {
it('should be done every time the context is updated', () => {
const computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1)
const manager = createContextManager(CustomerDataType.User, computeBytesCountStub)

manager.getBytesCount()
manager.add('foo', 'bar')
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.remove('foo')
manager.getBytesCount()
manager.remove('foo')
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.set({ foo: 'bar' })
manager.getBytesCount()
manager.set({ foo: 'bar' })
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.removeContextProperty('foo')
manager.getBytesCount()
manager.setContextProperty('foo', 'bar')
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.setContext({ foo: 'bar' })
manager.getBytesCount()
manager.removeContextProperty('foo')
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.clearContext()
manager.getBytesCount()
const bytesCount = manager.getBytesCount()
manager.setContext({ foo: 'bar' })
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

manager.clearContext()
const bytesCount = manager.getBytesCount()

expect(bytesCount).toEqual(0)
expect(computeBytesCountStub).toHaveBeenCalledTimes(6)
})

it('should be throttled to minimize the impact on performance', () => {
const computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1)
const manager = createContextManager(CustomerDataType.User, computeBytesCountStub)

manager.setContextProperty('1', 'foo') // leading call executed synchronously
manager.setContextProperty('2', 'bar') // ignored
manager.setContextProperty('3', 'bar') // trailing call executed after BYTES_COMPUTATION_THROTTLING_DELAY
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

expect(computeBytesCountStub).toHaveBeenCalledTimes(2)
})
})

it('should warn once if the context bytes limit is reached', () => {
const computeBytesCountStub = jasmine
.createSpy('computeBytesCountStub')
.and.returnValue(CUSTOMER_DATA_BYTES_LIMIT + 1)
const manager = createContextManager(CustomerDataType.User, computeBytesCountStub)

manager.setContext({})
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
manager.setContext({})
clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)

expect(bytesCount).toEqual(1)
expect(computeBytesCountStub).toHaveBeenCalledTimes(6)
expect(displaySpy).toHaveBeenCalledTimes(1)
})
})
41 changes: 25 additions & 16 deletions packages/core/src/tools/contextManager.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,49 @@
import { ExperimentalFeature, isExperimentalFeatureEnabled } from './experimentalFeatures'
import { computeBytesCount, deepClone, jsonStringify } from './utils'
import { computeBytesCount, deepClone, jsonStringify, throttle } from './utils'
import type { Context, ContextValue } from './context'
import type { CustomerDataType } from './heavyCustomerDataWarning'
import { warnIfCustomerDataLimitReached } from './heavyCustomerDataWarning'
import { sanitize } from './sanitize'

export const BYTES_COMPUTATION_THROTTLING_DELAY = 200

export type ContextManager = ReturnType<typeof createContextManager>

export function createContextManager(computeBytesCountImpl = computeBytesCount) {
export function createContextManager(customerDataType: CustomerDataType, computeBytesCountImpl = computeBytesCount) {
let context: Context = {}
let bytesCountCache: number | undefined
let bytesCountCache: number
let alreadyWarned = false

// Throttle the bytes computation to minimize the impact on performance.
// Especially useful if the user call context APIs synchronously multiple times in a row
const { throttled: computeBytesCountThrottled } = throttle((context: Context) => {
bytesCountCache = computeBytesCountImpl(jsonStringify(context)!)
if (!alreadyWarned) {
alreadyWarned = warnIfCustomerDataLimitReached(bytesCountCache, customerDataType)
}
}, BYTES_COMPUTATION_THROTTLING_DELAY)

return {
getBytesCount: () => {
if (bytesCountCache === undefined) {
bytesCountCache = computeBytesCountImpl(jsonStringify(context)!)
}
return bytesCountCache
},
getBytesCount: () => bytesCountCache,
/** @deprecated use getContext instead */
get: () => context,

/** @deprecated use setContextProperty instead */
add: (key: string, value: any) => {
context[key] = value as ContextValue
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

/** @deprecated renamed to removeContextProperty */
remove: (key: string) => {
delete context[key]
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

/** @deprecated use setContext instead */
set: (newContext: object) => {
context = newContext as Context
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

getContext: () => deepClone(context),
Expand All @@ -43,24 +52,24 @@ export function createContextManager(computeBytesCountImpl = computeBytesCount)
context = isExperimentalFeatureEnabled(ExperimentalFeature.SANITIZE_INPUTS)
? sanitize(newContext)
: deepClone(newContext)
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

setContextProperty: (key: string, property: any) => {
context[key] = isExperimentalFeatureEnabled(ExperimentalFeature.SANITIZE_INPUTS)
? sanitize(property)
: deepClone(property)
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

removeContextProperty: (key: string) => {
delete context[key]
bytesCountCache = undefined
computeBytesCountThrottled(context)
},

clearContext: () => {
context = {}
bytesCountCache = undefined
bytesCountCache = 0
},
}
}
31 changes: 31 additions & 0 deletions packages/core/src/tools/getGlobalObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* inspired by https://mathiasbynens.be/notes/globalthis
*/

export function getGlobalObject<T>(): T {
if (typeof globalThis === 'object') {
return globalThis as unknown as T
}
Object.defineProperty(Object.prototype, '_dd_temp_', {
get() {
return this as object
},
configurable: true,
})
// @ts-ignore _dd_temp is defined using defineProperty
let globalObject: unknown = _dd_temp_
// @ts-ignore _dd_temp is defined using defineProperty
delete Object.prototype._dd_temp_
if (typeof globalObject !== 'object') {
// on safari _dd_temp_ is available on window but not globally
// fallback on other browser globals check
if (typeof self === 'object') {
globalObject = self
} else if (typeof window === 'object') {
globalObject = window
} else {
globalObject = {}
}
}
return globalObject as T
}
4 changes: 3 additions & 1 deletion packages/core/src/tools/getZoneJsOriginalValue.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getGlobalObject } from './getGlobalObject'

export interface BrowserWindowWithZoneJs extends Window {
Zone?: {
// All Zone.js versions expose the __symbol__ method, but we observed that some website have a
Expand All @@ -24,7 +26,7 @@ export function getZoneJsOriginalValue<Target, Name extends keyof Target & strin
target: Target,
name: Name
): Target[Name] {
const browserWindow = window as BrowserWindowWithZoneJs
const browserWindow = getGlobalObject<BrowserWindowWithZoneJs>()
let original: Target[Name] | undefined
if (browserWindow.Zone && typeof browserWindow.Zone.__symbol__ === 'function') {
original = (target as any)[browserWindow.Zone.__symbol__(name)]
Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/tools/heavyCustomerDataWarning.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { display } from './display'
import { CustomerDataType, CUSTOMER_DATA_BYTES_LIMIT, warnIfCustomerDataLimitReached } from './heavyCustomerDataWarning'

describe('warnIfCustomerDataLimitReached', () => {
let displaySpy: jasmine.Spy<typeof display.warn>
beforeEach(() => {
displaySpy = spyOn(display, 'warn')
})

it('should warn when the customer data reach the limit', () => {
const warned = warnIfCustomerDataLimitReached(CUSTOMER_DATA_BYTES_LIMIT + 1, CustomerDataType.User)
expect(warned).toEqual(true)
expect(displaySpy).toHaveBeenCalledWith(
"The user data is over 3KiB. On low connectivity, the SDK has the potential to exhaust the user's upload bandwidth."
)
})

it('should not warn when the customer data does not reach the limit', () => {
const warned = warnIfCustomerDataLimitReached(CUSTOMER_DATA_BYTES_LIMIT - 1, CustomerDataType.User)
expect(warned).toEqual(false)
expect(displaySpy).not.toHaveBeenCalled()
})
})
26 changes: 26 additions & 0 deletions packages/core/src/tools/heavyCustomerDataWarning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { display } from './display'
import { ONE_KIBI_BYTE } from './utils'

// RUM and logs batch bytes limit is 16KB
// ensure that we leave room for other event attributes and maintain a decent amount of event per batch
// (3KB (customer data) + 1KB (other attributes)) * 4 (events per batch) = 16KB
export const CUSTOMER_DATA_BYTES_LIMIT = 3 * ONE_KIBI_BYTE

export const enum CustomerDataType {
FeatureFlag = 'feature flag evaluation',
User = 'user',
GlobalContext = 'global context',
LoggerContext = 'logger context',
}

export function warnIfCustomerDataLimitReached(bytesCount: number, customerDataType: CustomerDataType): boolean {
if (bytesCount > CUSTOMER_DATA_BYTES_LIMIT) {
display.warn(
`The ${customerDataType} data is over ${
CUSTOMER_DATA_BYTES_LIMIT / ONE_KIBI_BYTE
}KiB. On low connectivity, the SDK has the potential to exhaust the user's upload bandwidth.`
)
return true
}
return false
}
Loading

0 comments on commit 198e9e5

Please sign in to comment.