♻️ [RUM-2445] split RUM and Logs public APIs modules (#2575)

* ♻️ [RUM-2445] factorize displayAlreadyInitializedError

* ♻️ [RUM-2445] split rumPublicApi

* ✅♻️ [RUM-2445] move RUM public API tests

* ♻️ [RUM-2445] split logsPublicApi

* ✅♻️ [RUM-2445] move Logs public API tests

* 👌 rename createActualStrategy in logs

* 👌 change first startView call logic

* 👌 refactor logic to remove an item from an array

* 👌 fix comments

packages/core/src/boot/displayAlreadyInitializedError.spec.ts

@@ -0,0 +1,18 @@
+import type { InitConfiguration } from '../domain/configuration'
+import { display } from '../tools/display'
+import { displayAlreadyInitializedError } from './displayAlreadyInitializedError'
+
+describe('displayAlreadyInitializedError', () => {
+  it('should display an error', () => {
+    const displayErrorSpy = spyOn(display, 'error')
+    displayAlreadyInitializedError('DD_RUM', {} as InitConfiguration)
+    expect(displayErrorSpy).toHaveBeenCalledTimes(1)
+    expect(displayErrorSpy).toHaveBeenCalledWith('DD_RUM is already initialized.')
+  })
+
+  it('should not display an error if the "silentMultipleInit" option is used', () => {
+    const displayErrorSpy = spyOn(display, 'error')
+    displayAlreadyInitializedError('DD_RUM', { silentMultipleInit: true } as InitConfiguration)
+    expect(displayErrorSpy).not.toHaveBeenCalled()
+  })
+})

packages/core/src/boot/displayAlreadyInitializedError.ts

@@ -0,0 +1,8 @@
+import type { InitConfiguration } from '../domain/configuration'
+import { display } from '../tools/display'
+
+export function displayAlreadyInitializedError(sdkName: 'DD_RUM' | 'DD_LOGS', initConfiguration: InitConfiguration) {
+  if (!initConfiguration.silentMultipleInit) {
+    display.error(`${sdkName} is already initialized.`)
+  }
+}

packages/core/src/index.ts

@@ -20,6 +20,7 @@ export {
 export { trackRuntimeError } from './domain/error/trackRuntimeError'
 export { computeStackTrace, StackTrace } from './domain/error/computeStackTrace'
 export { defineGlobal, makePublicApi } from './boot/init'
+export { displayAlreadyInitializedError } from './boot/displayAlreadyInitializedError'
 export { initReportObservable, RawReport, RawReportType } from './domain/report/reportObservable'
 export {
   startTelemetry,

packages/core/src/tools/boundedBuffer.spec.ts

@@ -27,4 +27,14 @@ describe('BoundedBuffer', () => {
     buffered.drain()
     expect(spy.calls.count()).toEqual(limit)
   })
+
+  it('removes a callback', () => {
+    const spy = jasmine.createSpy<() => void>()
+    const buffered = new BoundedBuffer()
+
+    buffered.add(spy)
+    buffered.remove(spy)
+    buffered.drain()
+    expect(spy).not.toHaveBeenCalled()
+  })
 })

packages/core/src/tools/boundedBuffer.ts

@@ -1,17 +1,23 @@
+import { removeItem } from './utils/arrayUtils'
+
 const BUFFER_LIMIT = 500
 
-export class BoundedBuffer {
-  private buffer: Array<() => void> = []
+export class BoundedBuffer<T = void> {
+  private buffer: Array<(arg: T) => void> = []
 
-  add(callback: () => void) {
+  add(callback: (arg: T) => void) {
     const length = this.buffer.push(callback)
     if (length > BUFFER_LIMIT) {
       this.buffer.splice(0, 1)
     }
   }
 
-  drain() {
-    this.buffer.forEach((callback) => callback())
+  remove(callback: (arg: T) => void) {
+    removeItem(this.buffer, callback)
+  }
+
+  drain(arg: T) {
+    this.buffer.forEach((callback) => callback(arg))
     this.buffer.length = 0
   }
 }

packages/core/src/tools/utils/arrayUtils.ts

@@ -5,3 +5,10 @@ export function removeDuplicates<T>(array: T[]) {
   array.forEach((item) => set.add(item))
   return arrayFrom(set)
 }
+
+export function removeItem<T>(array: T[], item: T) {
+  const index = array.indexOf(item)
+  if (index >= 0) {
+    array.splice(index, 1)
+  }
+}

packages/core/src/tools/valueHistory.ts

@@ -1,5 +1,6 @@
 import { setInterval, clearInterval } from './timer'
 import type { TimeoutId } from './timer'
+import { removeItem } from './utils/arrayUtils'
 import type { Duration, RelativeTime } from './utils/timeUtils'
 import { addDuration, relativeNow, ONE_MINUTE } from './utils/timeUtils'
 
@@ -40,10 +41,7 @@ export class ValueHistory<Value> {
       startTime,
       endTime: END_OF_TIMES,
       remove: () => {
-        const index = this.entries.indexOf(entry)
-        if (index >= 0) {
-          this.entries.splice(index, 1)
-        }
+        removeItem(this.entries, entry)
       },
       close: (endTime: RelativeTime) => {
         entry.endTime = endTime

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

@@ -1,16 +1,13 @@
 import type { TimeStamp } from '@datadog/browser-core'
-import { monitor, ONE_SECOND, display, removeStorageListeners } from '@datadog/browser-core'
-import type { Clock } from '@datadog/browser-core/test'
-import { deleteEventBridgeStub, initEventBridgeStub, mockClock } from '@datadog/browser-core/test'
-import type { HybridInitConfiguration, LogsInitConfiguration } from '../domain/configuration'
+import { monitor, display, removeStorageListeners } from '@datadog/browser-core'
 import type { Logger, LogsMessage } from '../domain/logger'
 import { HandlerType, StatusType } from '../domain/logger'
 import type { CommonContext } from '../rawLogsEvent.types'
-import type { LogsPublicApi, StartLogs } from './logsPublicApi'
+import type { LogsPublicApi } from './logsPublicApi'
 import { makeLogsPublicApi } from './logsPublicApi'
+import type { StartLogs } from './startLogs'
 
 const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' }
-const INVALID_INIT_CONFIGURATION = {} as LogsInitConfiguration
 
 const mockSessionId = 'some-session-id'
 const getInternalContext = () => ({ session_id: mockSessionId })
@@ -36,6 +33,26 @@ describe('logs entry', () => {
     startLogs = jasmine.createSpy().and.callFake(() => ({ handleLog: handleLogSpy, getInternalContext }))
   })
 
+  it('should add a `_setDebug` that works', () => {
+    const displaySpy = spyOn(display, 'error')
+    const LOGS = makeLogsPublicApi(startLogs)
+    const setDebug: (debug: boolean) => void = (LOGS as any)._setDebug
+    expect(!!setDebug).toEqual(true)
+
+    monitor(() => {
+      throw new Error()
+    })()
+    expect(displaySpy).toHaveBeenCalledTimes(0)
+
+    setDebug(true)
+    monitor(() => {
+      throw new Error()
+    })()
+    expect(displaySpy).toHaveBeenCalledTimes(1)
+
+    setDebug(false)
+  })
+
   it('should define the public API with init', () => {
     const LOGS = makeLogsPublicApi(startLogs)
     expect(!!LOGS).toEqual(true)
@@ -47,99 +64,6 @@ describe('logs entry', () => {
     expect(LOGS.version).toBe('test')
   })
 
-  describe('configuration validation', () => {
-    let LOGS: LogsPublicApi
-    let displaySpy: jasmine.Spy
-
-    beforeEach(() => {
-      displaySpy = spyOn(display, 'error')
-      LOGS = makeLogsPublicApi(startLogs)
-    })
-
-    it('should start when the configuration is valid', () => {
-      LOGS.init(DEFAULT_INIT_CONFIGURATION)
-      expect(displaySpy).not.toHaveBeenCalled()
-      expect(startLogs).toHaveBeenCalled()
-    })
-
-    it('should not start when the configuration is missing', () => {
-      ;(LOGS.init as () => void)()
-      expect(displaySpy).toHaveBeenCalled()
-      expect(startLogs).not.toHaveBeenCalled()
-    })
-
-    it('should not start when the configuration is invalid', () => {
-      LOGS.init(INVALID_INIT_CONFIGURATION)
-      expect(displaySpy).toHaveBeenCalled()
-      expect(startLogs).not.toHaveBeenCalled()
-    })
-
-    it("should return init configuration even if it's invalid", () => {
-      LOGS.init(INVALID_INIT_CONFIGURATION)
-      expect(LOGS.getInitConfiguration()).toEqual(INVALID_INIT_CONFIGURATION)
-    })
-
-    it('should add a `_setDebug` that works', () => {
-      const setDebug: (debug: boolean) => void = (LOGS as any)._setDebug
-      expect(!!setDebug).toEqual(true)
-
-      monitor(() => {
-        throw new Error()
-      })()
-      expect(displaySpy).toHaveBeenCalledTimes(0)
-
-      setDebug(true)
-      monitor(() => {
-        throw new Error()
-      })()
-      expect(displaySpy).toHaveBeenCalledTimes(1)
-
-      setDebug(false)
-    })
-
-    describe('multiple init', () => {
-      it('should log an error if init is called several times', () => {
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-        expect(displaySpy).toHaveBeenCalledTimes(0)
-
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-        expect(displaySpy).toHaveBeenCalledTimes(1)
-      })
-
-      it('should not log an error if init is called several times and silentMultipleInit is true', () => {
-        LOGS.init({
-          ...DEFAULT_INIT_CONFIGURATION,
-          silentMultipleInit: true,
-        })
-        expect(displaySpy).toHaveBeenCalledTimes(0)
-
-        LOGS.init({
-          ...DEFAULT_INIT_CONFIGURATION,
-          silentMultipleInit: true,
-        })
-        expect(displaySpy).toHaveBeenCalledTimes(0)
-      })
-    })
-
-    describe('if event bridge present', () => {
-      beforeEach(() => {
-        initEventBridgeStub()
-      })
-
-      afterEach(() => {
-        deleteEventBridgeStub()
-      })
-
-      it('init should accept empty client token', () => {
-        const hybridInitConfiguration: HybridInitConfiguration = {}
-        LOGS.init(hybridInitConfiguration as LogsInitConfiguration)
-
-        expect(displaySpy).not.toHaveBeenCalled()
-        expect(startLogs).toHaveBeenCalled()
-      })
-    })
-  })
-
   describe('common context', () => {
     let LOGS: LogsPublicApi
 
@@ -163,84 +87,7 @@ describe('logs entry', () => {
     })
   })
 
-  describe('pre-init API usages', () => {
-    let LOGS: LogsPublicApi
-    let clock: Clock
-
-    beforeEach(() => {
-      LOGS = makeLogsPublicApi(startLogs)
-      clock = mockClock()
-    })
-
-    afterEach(() => {
-      clock.cleanup()
-    })
-
-    it('allows sending logs', () => {
-      LOGS.logger.log('message')
-
-      expect(handleLogSpy).not.toHaveBeenCalled()
-      LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-      expect(handleLogSpy.calls.all().length).toBe(1)
-      expect(getLoggedMessage(0).message.message).toBe('message')
-    })
-
-    it('allows creating logger', () => {
-      const logger = LOGS.createLogger('foo')
-      logger.error('message')
-
-      LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-      expect(getLoggedMessage(0).logger.getContext().logger).toEqual({ name: 'foo' })
-      expect(getLoggedMessage(0).message.message).toEqual('message')
-    })
-
-    it('returns undefined initial configuration', () => {
-      expect(LOGS.getInitConfiguration()).toBeUndefined()
-    })
-
-    describe('save context when submitting a log', () => {
-      it('saves the date', () => {
-        LOGS.logger.log('message')
-        clock.tick(ONE_SECOND)
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-        expect(getLoggedMessage(0).savedDate).toEqual((Date.now() - ONE_SECOND) as TimeStamp)
-      })
-
-      it('saves the URL', () => {
-        const initialLocation = window.location.href
-        LOGS.logger.log('message')
-        location.href = `#tata${Math.random()}`
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-        expect(getLoggedMessage(0).savedCommonContext!.view.url).toEqual(initialLocation)
-      })
-
-      it('stores a deep copy of the global context', () => {
-        LOGS.setGlobalContextProperty('foo', 'bar')
-        LOGS.logger.log('message')
-        LOGS.setGlobalContextProperty('foo', 'baz')
-
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-        expect(getLoggedMessage(0).savedCommonContext!.context.foo).toEqual('bar')
-      })
-
-      it('stores a deep copy of the log context', () => {
-        const context = { foo: 'bar' }
-        LOGS.logger.log('message', context)
-        context.foo = 'baz'
-
-        LOGS.init(DEFAULT_INIT_CONFIGURATION)
-
-        expect(getLoggedMessage(0).message.context!.foo).toEqual('bar')
-      })
-    })
-  })
-
-  describe('post-init API usages', () => {
+  describe('post start API usages', () => {
     let LOGS: LogsPublicApi
 
     beforeEach(() => {
@@ -322,17 +169,8 @@ describe('logs entry', () => {
     })
 
     describe('internal context', () => {
-      let LOGS: LogsPublicApi
-
-      beforeEach(() => {
-        LOGS = makeLogsPublicApi(startLogs)
-      })
-
-      it('should return undefined if not initialized', () => {
-        expect(LOGS.getInternalContext()).toBeUndefined()
-      })
-
       it('should get the internal context', () => {
+        const LOGS = makeLogsPublicApi(startLogs)
         LOGS.init(DEFAULT_INIT_CONFIGURATION)
         expect(LOGS.getInternalContext()?.session_id).toEqual(mockSessionId)
       })