Merge branch 'aymeric/hooks-logs-account' into staging-23

Author: amortemousque

packages/core/src/domain/context/defineContextMethod.ts

@@ -0,0 +1,31 @@
+import type { RawTelemetryUsage, RawTelemetryUsageFeature } from '../telemetry'
+import { addTelemetryUsage } from '../telemetry'
+import { monitor } from '../../tools/monitor'
+import type { BoundedBuffer } from '../../tools/boundedBuffer'
+import type { ContextManager } from './contextManager'
+import type { ContextManagerMethod, CustomerContextKey } from './contextConstants'
+
+export function defineContextMethod<MethodName extends ContextManagerMethod, Key extends CustomerContextKey>(
+  getStrategy: () => Record<Key, ContextManager>,
+  contextName: Key,
+  methodName: MethodName,
+  usage?: RawTelemetryUsageFeature
+): ContextManager[MethodName] {
+  return monitor((...args: any[]) => {
+    if (usage) {
+      addTelemetryUsage({ feature: usage } as RawTelemetryUsage)
+    }
+    return (getStrategy()[contextName][methodName] as (...args: unknown[]) => unknown)(...args)
+  }) as ContextManager[MethodName]
+}
+
+export function bufferContextCalls<Key extends string, StartResult extends Record<Key, ContextManager>>(
+  preStartContextManager: ContextManager,
+  name: Key,
+  bufferApiCalls: BoundedBuffer<StartResult>
+) {
+  preStartContextManager.changeObservable.subscribe(() => {
+    const context = preStartContextManager.getContext()
+    bufferApiCalls.add((startResult) => startResult[name].setContext(context))
+  })
+}

…c/domain/contexts/accountContext.spec.ts …c/domain/contexts/accountContext.spec.tspackages/rum-core/src/domain/contexts/accountContext.spec.ts renamed to packages/core/src/domain/contexts/accountContext.spec.ts packages/rum-core/src/domain/contexts/accountContext.spec.ts renamed to packages/core/src/domain/contexts/accountContext.spec.ts

@@ -1,9 +1,11 @@
-import type { ContextManager, RelativeTime } from '@datadog/browser-core'
-import { display, HookNames, removeStorageListeners } from '@datadog/browser-core'
-import { registerCleanupTask } from '@datadog/browser-core/test'
-import { mockRumConfiguration } from '../../../test'
-import type { Hooks } from '../hooks'
-import { createHooks } from '../hooks'
+import type { Hooks } from '../../../test'
+import { createHooks, registerCleanupTask } from '../../../test'
+import { mockRumConfiguration } from '../../../../rum-core/test'
+import type { ContextManager } from '../context/contextManager'
+import { display } from '../../tools/display'
+import type { RelativeTime } from '../../tools/utils/timeUtils'
+import { HookNames } from '../../tools/abstractHooks'
+import { removeStorageListeners } from '../context/storeContextManager'
 import { startAccountContext } from './accountContext'
 
 describe('account context', () => {
@@ -13,10 +15,9 @@ describe('account context', () => {
 
   beforeEach(() => {
     hooks = createHooks()
-
     displaySpy = spyOn(display, 'warn')
 
-    accountContext = startAccountContext(hooks, mockRumConfiguration())
+    accountContext = startAccountContext(hooks, mockRumConfiguration(), 'some_product_key')
   })
 
   it('should warn when the account.id is missing', () => {
@@ -37,12 +38,10 @@ describe('account context', () => {
       accountContext.setContext({ id: '123', foo: 'bar' })
 
       const defaultRumEventAttributes = hooks.triggerHook(HookNames.Assemble, {
-        eventType: 'view',
         startTime: 0 as RelativeTime,
       })
 
       expect(defaultRumEventAttributes).toEqual({
-        type: 'view',
         account: {
           id: '123',
           foo: 'bar',
@@ -75,18 +74,26 @@ describe('account context across pages', () => {
   })
 
   it('when disabled, should store contexts only in memory', () => {
-    accountContext = startAccountContext(hooks, mockRumConfiguration({ storeContextsAcrossPages: false }))
+    accountContext = startAccountContext(
+      hooks,
+      mockRumConfiguration({ storeContextsAcrossPages: false }),
+      'some_product_key'
+    )
     accountContext.setContext({ id: '123' })
 
     expect(accountContext.getContext()).toEqual({ id: '123' })
     expect(localStorage.getItem('_dd_c_rum_4')).toBeNull()
   })
 
   it('when enabled, should maintain the account in local storage', () => {
-    accountContext = startAccountContext(hooks, mockRumConfiguration({ storeContextsAcrossPages: true }))
+    accountContext = startAccountContext(
+      hooks,
+      mockRumConfiguration({ storeContextsAcrossPages: true }),
+      'some_product_key'
+    )
 
     accountContext.setContext({ id: 'foo', qux: 'qix' })
     expect(accountContext.getContext()).toEqual({ id: 'foo', qux: 'qix' })
-    expect(localStorage.getItem('_dd_c_rum_4')).toBe('{"id":"foo","qux":"qix"}')
+    expect(localStorage.getItem('_dd_c_some_product_key_4')).toBe('{"id":"foo","qux":"qix"}')
   })
 })

packages/core/src/domain/contexts/accountContext.ts

@@ -0,0 +1,39 @@
+import type { Configuration } from '../configuration'
+import { CustomerDataType } from '../context/contextConstants'
+import { storeContextManager } from '../context/storeContextManager'
+import { HookNames, SKIPPED } from '../../tools/abstractHooks'
+import type { AbstractHooks } from '../../tools/abstractHooks'
+import type { Account } from '../account.types'
+import { isEmptyObject } from '../../tools/utils/objectUtils'
+import { createContextManager } from '../context/contextManager'
+
+export function startAccountContext(hooks: AbstractHooks, configuration: Configuration, productKey: string) {
+  const accountContextManager = buildAccountContextManager()
+
+  if (configuration.storeContextsAcrossPages) {
+    storeContextManager(configuration, accountContextManager, productKey, CustomerDataType.Account)
+  }
+
+  hooks.register(HookNames.Assemble, () => {
+    const account = accountContextManager.getContext() as Account
+
+    if (isEmptyObject(account) || !account.id) {
+      return SKIPPED
+    }
+
+    return {
+      account,
+    }
+  })
+
+  return accountContextManager
+}
+
+export function buildAccountContextManager() {
+  return createContextManager('account', {
+    propertiesConfig: {
+      id: { type: 'string', required: true },
+      name: { type: 'string' },
+    },
+  })
+}

packages/core/src/index.ts

@@ -124,7 +124,9 @@ export { createBoundedBuffer } from './tools/boundedBuffer'
 export { catchUserErrors } from './tools/catchUserErrors'
 export type { ContextManager } from './domain/context/contextManager'
 export { createContextManager } from './domain/context/contextManager'
+export { defineContextMethod, bufferContextCalls } from './domain/context/defineContextMethod'
 export { storeContextManager, removeStorageListeners } from './domain/context/storeContextManager'
+export { startAccountContext, buildAccountContextManager } from './domain/contexts/accountContext'
 export { CustomerDataType, CustomerContextKey, ContextManagerMethod } from './domain/context/contextConstants'
 export type { ValueHistory, ValueHistoryEntry } from './tools/valueHistory'
 export { createValueHistory, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory'

packages/core/src/tools/abstractHooks.ts

@@ -26,6 +26,8 @@ export const SKIPPED = 'SKIPPED'
 export type DISCARDED = typeof DISCARDED
 export type SKIPPED = typeof SKIPPED
 
+export type AbstractHooks = ReturnType<typeof abstractHooks>
+
 export function abstractHooks<T extends { [K in HookNames]: (...args: any[]) => any }, E>() {
   const callbacks: { [K in HookNames]?: Array<T[K]> } = {}
 

packages/core/test/createHooks.ts

@@ -0,0 +1,11 @@
+import type { HookNames } from '../src/tools/abstractHooks'
+import { abstractHooks } from '../src/tools/abstractHooks'
+
+export type Hooks = ReturnType<typeof createHooks>
+
+export const createHooks = abstractHooks<
+  {
+    [HookNames.Assemble]: (...args: any[]) => any
+  },
+  { [key: string]: any }
+>

packages/core/test/index.ts

@@ -26,3 +26,4 @@ export * from './disableJasmineUncaughtExceptionTracking'
 export * from './instrumentation'
 export * from './wait'
 export * from './consoleLog'
+export * from './createHooks'

packages/logs/src/boot/logsPublicApi.spec.ts

@@ -84,7 +84,6 @@ describe('logs entry', () => {
         },
         context: { foo: 'bar' },
         user: {},
-        account: {},
       })
     })
   })
@@ -321,44 +320,44 @@ describe('logs entry', () => {
     })
 
     describe('setAccount', () => {
-      let logsPublicApi: LogsPublicApi
       let displaySpy: jasmine.Spy<() => void>
+      let logsPublicApi: LogsPublicApi
 
       beforeEach(() => {
         displaySpy = spyOn(display, 'error')
         logsPublicApi = makeLogsPublicApi(startLogs)
-        logsPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       })
 
-      it('should store account in common context', () => {
+      it('should attach valid objects', () => {
         const account = { id: 'foo', name: 'bar', foo: { bar: 'qux' } }
         logsPublicApi.setAccount(account)
 
-        const getCommonContext = startLogs.calls.mostRecent().args[2]
-        expect(getCommonContext().account).toEqual({
+        expect(logsPublicApi.getAccount()).toEqual({
           foo: { bar: 'qux' },
           id: 'foo',
           name: 'bar',
         })
+        expect(displaySpy).not.toHaveBeenCalled()
       })
 
       it('should sanitize predefined properties', () => {
         const account = { id: false, name: 2 }
         logsPublicApi.setAccount(account as any)
-        const getCommonContext = startLogs.calls.mostRecent().args[2]
-        expect(getCommonContext().account).toEqual({
+
+        expect(logsPublicApi.getAccount()).toEqual({
           id: 'false',
           name: '2',
         })
+        expect(displaySpy).not.toHaveBeenCalled()
       })
 
-      it('should clear a previously set account', () => {
-        const account = { id: 'foo', name: 'bar', foo: 'qux' }
+      it('should remove the account', () => {
+        const account = { id: 'foo', name: 'bar' }
         logsPublicApi.setAccount(account)
         logsPublicApi.clearAccount()
 
-        const getCommonContext = startLogs.calls.mostRecent().args[2]
-        expect(getCommonContext().account).toEqual({})
+        expect(logsPublicApi.getAccount()).toEqual({})
+        expect(displaySpy).not.toHaveBeenCalled()
       })
 
       it('should reject non object input', () => {
@@ -374,7 +373,6 @@ describe('logs entry', () => {
 
       beforeEach(() => {
         logsPublicApi = makeLogsPublicApi(startLogs)
-        logsPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       })
 
       it('should return empty object if no account has been set', () => {
@@ -401,7 +399,6 @@ describe('logs entry', () => {
 
       beforeEach(() => {
         logsPublicApi = makeLogsPublicApi(startLogs)
-        logsPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       })
 
       it('should add attribute', () => {
@@ -444,9 +441,7 @@ describe('logs entry', () => {
 
       beforeEach(() => {
         logsPublicApi = makeLogsPublicApi(startLogs)
-        logsPublicApi.init(DEFAULT_INIT_CONFIGURATION)
       })
-
       it('should remove property', () => {
         const account = { id: 'foo', name: 'bar', email: 'qux', foo: { bar: 'qux' } }
 

packages/logs/src/boot/logsPublicApi.ts

@@ -1,5 +1,7 @@
-import type { Account, Context, TrackingConsent, User, PublicApi } from '@datadog/browser-core'
+import type { Account, Context, TrackingConsent, User, PublicApi, ContextManager } from '@datadog/browser-core'
 import {
+  ContextManagerMethod,
+  CustomerContextKey,
   addTelemetryUsage,
   CustomerDataType,
   createContextManager,
@@ -10,6 +12,7 @@ import {
   displayAlreadyInitializedError,
   deepClone,
   createTrackingConsentState,
+  defineContextMethod,
 } from '@datadog/browser-core'
 import type { LogsInitConfiguration } from '../domain/configuration'
 import type { HandlerType } from '../domain/logger'
@@ -197,6 +200,7 @@ const LOGS_STORAGE_KEY = 'logs'
 export interface Strategy {
   init: (initConfiguration: LogsInitConfiguration) => void
   initConfiguration: LogsInitConfiguration | undefined
+  accountContext: ContextManager
   getInternalContext: StartLogsResult['getInternalContext']
   handleLog: StartLogsResult['handleLog']
 }
@@ -210,23 +214,16 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
       email: { type: 'string' },
     },
   })
-  const accountContextManager = createContextManager('account', {
-    propertiesConfig: {
-      id: { type: 'string', required: true },
-      name: { type: 'string' },
-    },
-  })
   const trackingConsentState = createTrackingConsentState()
 
   function getCommonContext() {
-    return buildCommonContext(globalContextManager, userContextManager, accountContextManager)
+    return buildCommonContext(globalContextManager, userContextManager)
   }
 
   let strategy = createPreStartStrategy(getCommonContext, trackingConsentState, (initConfiguration, configuration) => {
     if (initConfiguration.storeContextsAcrossPages) {
       storeContextManager(configuration, globalContextManager, LOGS_STORAGE_KEY, CustomerDataType.GlobalContext)
       storeContextManager(configuration, userContextManager, LOGS_STORAGE_KEY, CustomerDataType.User)
-      storeContextManager(configuration, accountContextManager, LOGS_STORAGE_KEY, CustomerDataType.Account)
     }
 
     const startLogsResult = startLogsImpl(initConfiguration, configuration, getCommonContext, trackingConsentState)
@@ -235,6 +232,8 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
     return startLogsResult
   })
 
+  const getStrategy = () => strategy
+
   const customLoggers: { [name: string]: Logger | undefined } = {}
 
   const mainLogger = new Logger((...params) => strategy.handleLog(...params))
@@ -287,15 +286,27 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
 
     clearUser: monitor(userContextManager.clearContext),
 
-    setAccount: monitor(accountContextManager.setContext),
+    setAccount: defineContextMethod(getStrategy, CustomerContextKey.accountContext, ContextManagerMethod.setContext),
 
-    getAccount: monitor(accountContextManager.getContext),
+    getAccount: defineContextMethod(getStrategy, CustomerContextKey.accountContext, ContextManagerMethod.getContext),
 
-    setAccountProperty: monitor(accountContextManager.setContextProperty),
+    setAccountProperty: defineContextMethod(
+      getStrategy,
+      CustomerContextKey.accountContext,
+      ContextManagerMethod.setContextProperty
+    ),
 
-    removeAccountProperty: monitor(accountContextManager.removeContextProperty),
+    removeAccountProperty: defineContextMethod(
+      getStrategy,
+      CustomerContextKey.accountContext,
+      ContextManagerMethod.removeContextProperty
+    ),
 
-    clearAccount: monitor(accountContextManager.clearContext),
+    clearAccount: defineContextMethod(
+      getStrategy,
+      CustomerContextKey.accountContext,
+      ContextManagerMethod.clearContext
+    ),
   })
 }
 

packages/logs/src/boot/preStartLogs.ts

@@ -8,6 +8,9 @@ import {
   initFetchObservable,
   noop,
   timeStampNow,
+  buildAccountContextManager,
+  CustomerContextKey,
+  bufferContextCalls,
 } from '@datadog/browser-core'
 import {
   validateAndBuildLogsConfiguration,
@@ -24,6 +27,11 @@ export function createPreStartStrategy(
   doStartLogs: (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult
 ): Strategy {
   const bufferApiCalls = createBoundedBuffer<StartLogsResult>()
+
+  // TODO next major: remove the  accountContextManager from preStartStrategy and use an empty context instead
+  const accountContext = buildAccountContextManager()
+  bufferContextCalls(accountContext, CustomerContextKey.accountContext, bufferApiCalls)
+
   let cachedInitConfiguration: LogsInitConfiguration | undefined
   let cachedConfiguration: LogsConfiguration | undefined
   const trackingConsentStateSubscription = trackingConsentState.observable.subscribe(tryStartLogs)
@@ -80,6 +88,8 @@ export function createPreStartStrategy(
       return cachedInitConfiguration
     },
 
+    accountContext,
+
     getInternalContext: noop as () => undefined,
 
     handleLog(message, statusType, handlingStack, context = getCommonContext(), date = timeStampNow()) {