🔉 [RUM-253] adjust heavy customer data warning when compression is en…

…abled (#2529) * ♻️ [RUM-253] refactor customer data tracking Use a single component used by both feature flags and context manager. * ♻️ [RUM-253] refactor tests No need to test for customer data warning in every spec, move related tests to `trackCustomerData` spec. * ♻️ [RUM-253] expose customerDataTracker in ContextManager This allows to reuse the same customerDataTracker in the "stored context manager" logic, so we don't show the same warning twice. It simplifies a bit the code. * 🔊 [RUM-253] adjust warning limit when compression is enabled Have a bigger limit when the compression is enabled. Because the compression "status" is only known at init, we need to wait until before showing the warning. * 👌 rename module * 👌 rename `maybeWarn` to something more explicit * 👌 adjust documentation URL * 👌 use a "customer data tracker manager" * 👌 use global limit for all contexts * ♻️ do not store data type in trackers * ♻️ simplify customer data telemetry
DataDog · Jan 9, 2024 · 0f7ea73 · 0f7ea73
1 parent cfcc00e
commit 0f7ea73
Show file tree

Hide file tree

Showing 26 changed files with 605 additions and 369 deletions.
diff --git a/packages/core/src/domain/context/contextConstants.ts b/packages/core/src/domain/context/contextConstants.ts
@@ -2,5 +2,4 @@ export const enum CustomerDataType {
   FeatureFlag,
   User,
   GlobalContext,
-  LoggerContext,
 }
diff --git a/packages/core/src/domain/context/contextManager.spec.ts b/packages/core/src/domain/context/contextManager.spec.ts
@@ -1,52 +1,36 @@
-import type { Clock } from '../../../test'
-import { mockClock } from '../../../test'
-import { display } from '../../tools/display'
-import { BYTES_COMPUTATION_THROTTLING_DELAY, createContextManager } from './contextManager'
-import { CUSTOMER_DATA_BYTES_LIMIT } from './heavyCustomerDataWarning'
-import { CustomerDataType } from './contextConstants'
+import { noop } from '../../tools/utils/functionUtils'
+import { createContextManager } from './contextManager'
+import { createCustomerDataTracker } from './customerDataTracker'
 
 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(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     expect(manager.getContext()).toEqual({})
   })
 
   it('updates the context', () => {
-    const manager = createContextManager(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContext({ bar: 'foo' })
 
     expect(manager.getContext()).toEqual({ bar: 'foo' })
   })
 
   it('completely replaces the context', () => {
-    const manager = createContextManager(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContext({ a: 'foo' })
     expect(manager.getContext()).toEqual({ a: 'foo' })
     manager.setContext({ b: 'foo' })
     expect(manager.getContext()).toEqual({ b: 'foo' })
   })
 
   it('sets a context value', () => {
-    const manager = createContextManager(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContextProperty('foo', 'bar')
     expect(manager.getContext()).toEqual({ foo: 'bar' })
   })
 
   it('removes a context value', () => {
-    const manager = createContextManager(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContext({ a: 'foo', b: 'bar' })
     manager.removeContextProperty('a')
     expect(manager.getContext()).toEqual({ b: 'bar' })
@@ -55,39 +39,39 @@ describe('createContextManager', () => {
   })
 
   it('should get a clone of the context from getContext', () => {
-    const manager = createContextManager(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     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(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     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(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     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(CustomerDataType.User)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContext(context)
     expect(manager.getContext()).toEqual(context)
     manager.clearContext()
     expect(manager.getContext()).toEqual({})
   })
 
   it('should prevent setting non object values', () => {
-    const manager = createContextManager(CustomerDataType.GlobalContext)
+    const manager = createContextManager(createCustomerDataTracker(noop))
     manager.setContext(null as any)
     expect(manager.getContext()).toEqual({})
     manager.setContext(undefined as any)
@@ -96,67 +80,28 @@ describe('createContextManager', () => {
     expect(manager.getContext()).toEqual({})
   })
 
-  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.setContextProperty('foo', 'bar')
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
-
-      manager.removeContextProperty('foo')
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
-
-      manager.setContext({ foo: 'bar' })
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
-
-      manager.setContextProperty('foo', 'bar')
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
-
-      manager.removeContextProperty('foo')
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
-
-      manager.setContext({ foo: 'bar' })
-      clock.tick(BYTES_COMPUTATION_THROTTLING_DELAY)
+  it('should notify customer data tracker when the context is updated', () => {
+    const customerDataTracker = createCustomerDataTracker(noop)
+    const updateCustomerDataSpy = spyOn(customerDataTracker, 'updateCustomerData')
+    const resetCustomerDataSpy = spyOn(customerDataTracker, 'resetCustomerData')
+    const manager = createContextManager(customerDataTracker)
 
-      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)
+    manager.setContextProperty('foo', 'bar')
+    manager.removeContextProperty('foo')
+    manager.setContext({ foo: 'bar' })
+    manager.setContextProperty('foo', 'bar')
+    manager.removeContextProperty('foo')
+    manager.setContext({ foo: 'bar' })
+    manager.clearContext()
 
-    expect(displaySpy).toHaveBeenCalledTimes(1)
+    expect(updateCustomerDataSpy).toHaveBeenCalledTimes(6)
+    expect(resetCustomerDataSpy).toHaveBeenCalledTimes(1)
   })
 
   describe('changeObservable', () => {
     it('should notify on context changes', () => {
       const changeSpy = jasmine.createSpy('change')
-      const manager = createContextManager(CustomerDataType.GlobalContext)
+      const manager = createContextManager(createCustomerDataTracker(noop))
       manager.changeObservable.subscribe(changeSpy)
 
       manager.getContext()

diff --git a/packages/core/src/domain/context/contextManager.ts b/packages/core/src/domain/context/contextManager.ts
@@ -1,42 +1,23 @@
-import { computeBytesCount } from '../../tools/utils/byteUtils'
-import { throttle } from '../../tools/utils/functionUtils'
 import { deepClone } from '../../tools/mergeInto'
 import { getType } from '../../tools/utils/typeUtils'
-import { jsonStringify } from '../../tools/serialisation/jsonStringify'
 import { sanitize } from '../../tools/serialisation/sanitize'
 import type { Context } from '../../tools/serialisation/context'
 import { Observable } from '../../tools/observable'
-import { warnIfCustomerDataLimitReached } from './heavyCustomerDataWarning'
-import type { CustomerDataType } from './contextConstants'
-
-export const BYTES_COMPUTATION_THROTTLING_DELAY = 200
+import type { CustomerDataTracker } from './customerDataTracker'
 
 export type ContextManager = ReturnType<typeof createContextManager>
 
-export function createContextManager(customerDataType: CustomerDataType, computeBytesCountImpl = computeBytesCount) {
+export function createContextManager(customerDataTracker: CustomerDataTracker) {
   let context: Context = {}
-  let bytesCountCache: number
-  let alreadyWarned = false
   const changeObservable = new Observable<void>()
 
-  // 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)
-
   const contextManager = {
-    getBytesCount: () => bytesCountCache,
-
     getContext: () => deepClone(context),
 
     setContext: (newContext: Context) => {
       if (getType(newContext) === 'object') {
         context = sanitize(newContext)
-        computeBytesCountThrottled(context)
+        customerDataTracker.updateCustomerData(context)
       } else {
         contextManager.clearContext()
       }
@@ -45,19 +26,19 @@ export function createContextManager(customerDataType: CustomerDataType, compute
 
     setContextProperty: (key: string, property: any) => {
       context[key] = sanitize(property)
-      computeBytesCountThrottled(context)
+      customerDataTracker.updateCustomerData(context)
       changeObservable.notify()
     },
 
     removeContextProperty: (key: string) => {
       delete context[key]
-      computeBytesCountThrottled(context)
+      customerDataTracker.updateCustomerData(context)
       changeObservable.notify()
     },
 
     clearContext: () => {
       context = {}
-      bytesCountCache = 0
+      customerDataTracker.resetCustomerData()
       changeObservable.notify()
     },