♻️ [RUMF-1097] revamp configuration - logs

packages/core/src/domain/configuration/configuration.spec.ts

@@ -102,6 +102,11 @@ describe('validateAndBuildConfiguration', () => {
       ).toBeUndefined()
       expect(displaySpy).toHaveBeenCalledOnceWith('Sample Rate should be a number between 0 and 100')
     })
+
+    it("shouldn't display any error if the configuration is correct", () => {
+      validateAndBuildConfiguration({ clientToken: 'yes', sampleRate: 1 }, buildEnv)
+      expect(displaySpy).not.toHaveBeenCalled()
+    })
   })
 
   describe('cookie options', () => {

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

@@ -7,12 +7,13 @@ import {
   resetExperimentalFeatures,
 } from '@datadog/browser-core'
 import { Clock, deleteEventBridgeStub, initEventBridgeStub, mockClock } from '../../../core/test/specHelper'
+import { HybridInitConfiguration, LogsInitConfiguration } from '../domain/configuration'
 
 import { HandlerType, LogsMessage, StatusType } from '../domain/logger'
-import { HybridInitConfiguration, LogsPublicApi, makeLogsPublicApi, StartLogs } from './logsPublicApi'
-import { LogsInitConfiguration } from './startLogs'
+import { LogsPublicApi, makeLogsPublicApi, StartLogs } from './logsPublicApi'
 
 const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' }
+const INVALID_INIT_CONFIGURATION = {} as LogsInitConfiguration
 
 describe('logs entry', () => {
   let sendLogsSpy: jasmine.Spy<
@@ -21,7 +22,7 @@ describe('logs entry', () => {
       currentContext: Context & { view: { referrer: string; url: string } }
     ) => void
   >
-  const startLogs: StartLogs = () => (sendLogsSpy as any) as ReturnType<StartLogs>
+  let startLogs: jasmine.Spy<StartLogs>
 
   function getLoggedMessage(index: number) {
     const [message, context] = sendLogsSpy.calls.argsFor(index)
@@ -30,6 +31,7 @@ describe('logs entry', () => {
 
   beforeEach(() => {
     sendLogsSpy = jasmine.createSpy()
+    startLogs = jasmine.createSpy().and.callFake(() => sendLogsSpy)
   })
 
   it('should define the public API with init', () => {
@@ -47,15 +49,16 @@ describe('logs entry', () => {
       LOGS = makeLogsPublicApi(startLogs)
     })
 
-    it('init should log an error with no public api key', () => {
-      LOGS.init(undefined as any)
-      expect(displaySpy).toHaveBeenCalledTimes(1)
-
-      LOGS.init({ stillNoApiKey: true } as any)
-      expect(displaySpy).toHaveBeenCalledTimes(2)
+    it('should start when the configuration is valid', () => {
+      LOGS.init(DEFAULT_INIT_CONFIGURATION)
+      expect(displaySpy).not.toHaveBeenCalled()
+      expect(startLogs).toHaveBeenCalled()
+    })
 
-      LOGS.init({ clientToken: 'yeah' })
-      expect(displaySpy).toHaveBeenCalledTimes(2)
+    it('should not start when the configuration is invalid', () => {
+      LOGS.init(INVALID_INIT_CONFIGURATION)
+      expect(displaySpy).toHaveBeenCalled()
+      expect(startLogs).not.toHaveBeenCalled()
     })
 
     it('should add a `_setDebug` that works', () => {
@@ -76,41 +79,28 @@ describe('logs entry', () => {
       setDebug(false)
     })
 
-    it('init should log an error if sampleRate is invalid', () => {
-      LOGS.init({ clientToken: 'yes', sampleRate: 'foo' as any })
-      expect(displaySpy).toHaveBeenCalledTimes(1)
-
-      LOGS.init({ clientToken: 'yes', sampleRate: 200 })
-      expect(displaySpy).toHaveBeenCalledTimes(2)
-    })
-
-    it('should log an error if init is called several times', () => {
-      LOGS.init({ clientToken: 'yes', sampleRate: 1 })
-      expect(displaySpy).toHaveBeenCalledTimes(0)
-
-      LOGS.init({ clientToken: 'yes', sampleRate: 1 })
-      expect(displaySpy).toHaveBeenCalledTimes(1)
-    })
+    describe('multiple init', () => {
+      it('should log an error if init is called several times', () => {
+        LOGS.init(DEFAULT_INIT_CONFIGURATION)
+        expect(displaySpy).toHaveBeenCalledTimes(0)
 
-    it('should not log an error if init is called several times and silentMultipleInit is true', () => {
-      LOGS.init({
-        clientToken: 'yes',
-        sampleRate: 1,
-        silentMultipleInit: true,
+        LOGS.init(DEFAULT_INIT_CONFIGURATION)
+        expect(displaySpy).toHaveBeenCalledTimes(1)
       })
-      expect(displaySpy).toHaveBeenCalledTimes(0)
 
-      LOGS.init({
-        clientToken: 'yes',
-        sampleRate: 1,
-        silentMultipleInit: true,
-      })
-      expect(displaySpy).toHaveBeenCalledTimes(0)
-    })
+      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)
 
-    it("shouldn't trigger any console.error if the configuration is correct", () => {
-      LOGS.init({ clientToken: 'yes', sampleRate: 1 })
-      expect(displaySpy).toHaveBeenCalledTimes(0)
+        LOGS.init({
+          ...DEFAULT_INIT_CONFIGURATION,
+          silentMultipleInit: true,
+        })
+        expect(displaySpy).toHaveBeenCalledTimes(0)
+      })
     })
 
     describe('if event bridge present', () => {
@@ -129,6 +119,7 @@ describe('logs entry', () => {
         LOGS.init(hybridInitConfiguration as LogsInitConfiguration)
 
         expect(displaySpy).not.toHaveBeenCalled()
+        expect(startLogs).toHaveBeenCalled()
       })
     })
   })

packages/logs/src/boot/logsPublicApi.ts

@@ -3,25 +3,23 @@ import {
   combine,
   Context,
   createContextManager,
-  isPercentage,
   makePublicApi,
   monitor,
   display,
   deepClone,
   InitConfiguration,
   canUseEventBridge,
 } from '@datadog/browser-core'
+import { LogsInitConfiguration, validateAndBuildLogsConfiguration } from '../domain/configuration'
 import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger'
-import { startLogs, LogsInitConfiguration } from './startLogs'
+import { startLogs } from './startLogs'
 
 export interface LoggerConfiguration {
   level?: StatusType
   handler?: HandlerType | HandlerType[]
   context?: object
 }
 
-export type HybridInitConfiguration = Omit<LogsInitConfiguration, 'clientToken'>
-
 export type LogsPublicApi = ReturnType<typeof makeLogsPublicApi>
 
 export type StartLogs = typeof startLogs
@@ -51,7 +49,12 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
         return
       }
 
-      sendLogStrategy = startLogsImpl(initConfiguration, logger)
+      const configuration = validateAndBuildLogsConfiguration(initConfiguration)
+      if (!configuration) {
+        return
+      }
+
+      sendLogStrategy = startLogsImpl(initConfiguration, configuration, logger)
       getInitConfigurationStrategy = () => deepClone(initConfiguration)
       beforeInitSendLog.drain()
 
@@ -89,14 +92,6 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
       }
       return false
     }
-    if (!initConfiguration || !initConfiguration.clientToken) {
-      display.error('Client Token is not configured, we will not send any data.')
-      return false
-    }
-    if (initConfiguration.sampleRate !== undefined && !isPercentage(initConfiguration.sampleRate)) {
-      display.error('Sample Rate should be a number between 0 and 100')
-      return false
-    }
     return true
   }
 

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

@@ -1,5 +1,4 @@
 import {
-  Configuration,
   Context,
   DEFAULT_CONFIGURATION,
   ErrorSource,
@@ -21,11 +20,12 @@ import {
   mockClock,
   stubEndpointBuilder,
 } from '../../../core/test/specHelper'
+import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration'
 
 import { Logger, LogsMessage, StatusType } from '../domain/logger'
 import { LogsSessionManager } from '../domain/logsSessionManager'
 import { LogsEvent } from '../logsEvent.types'
-import { buildAssemble, doStartLogs, LogsInitConfiguration, startLogs as originalStartLogs } from './startLogs'
+import { buildAssemble, doStartLogs, startLogs as originalStartLogs } from './startLogs'
 
 interface SentMessage extends LogsMessage {
   logger?: { name: string }
@@ -41,7 +41,7 @@ function getLoggedMessage(server: sinon.SinonFakeServer, index: number) {
 }
 const FAKE_DATE = 123456
 const SESSION_ID = 'session-id'
-const baseConfiguration: Partial<Configuration> = {
+const baseConfiguration: Partial<LogsConfiguration> = {
   ...DEFAULT_CONFIGURATION,
   logsEndpointBuilder: stubEndpointBuilder('https://localhost/v1/input/log'),
   maxBatchSize: 1,
@@ -70,8 +70,8 @@ describe('logs', () => {
   const startLogs = ({
     errorLogger = new Logger(noop),
     configuration: configurationOverrides,
-  }: { errorLogger?: Logger; configuration?: Partial<Configuration> } = {}) => {
-    const configuration = { ...(baseConfiguration as Configuration), ...configurationOverrides }
+  }: { errorLogger?: Logger; configuration?: Partial<LogsConfiguration> } = {}) => {
+    const configuration = { ...(baseConfiguration as LogsConfiguration), ...configurationOverrides }
     return doStartLogs(configuration, errorObservable, internalMonitoring, sessionManager, errorLogger)
   }
 
@@ -180,14 +180,14 @@ describe('logs', () => {
       updateExperimentalFeatures(['event-bridge'])
       const sendSpy = spyOn(initEventBridgeStub(), 'send')
 
-      let configuration = { ...baseConfiguration, ...{ sampleRate: 0 } } as LogsInitConfiguration
-      let sendLog = originalStartLogs(configuration, new Logger(noop))
+      let configuration = { ...baseConfiguration, sampleRate: 0 } as LogsConfiguration
+      let sendLog = originalStartLogs({} as LogsInitConfiguration, configuration, new Logger(noop))
       sendLog(DEFAULT_MESSAGE, {})
 
       expect(sendSpy).not.toHaveBeenCalled()
 
-      configuration = { ...baseConfiguration, ...{ sampleRate: 100 } } as LogsInitConfiguration
-      sendLog = originalStartLogs(configuration, new Logger(noop))
+      configuration = { ...baseConfiguration, sampleRate: 100 } as LogsConfiguration
+      sendLog = originalStartLogs({} as LogsInitConfiguration, configuration, new Logger(noop))
       sendLog(DEFAULT_MESSAGE, {})
 
       expect(sendSpy).toHaveBeenCalled()
@@ -203,7 +203,7 @@ describe('logs', () => {
       assemble = buildAssemble(
         sessionManager,
         {
-          ...(baseConfiguration as Configuration),
+          ...(baseConfiguration as LogsConfiguration),
           beforeSend: (x: LogsEvent) => beforeSend(x),
         },
         noop

packages/logs/src/boot/startLogs.ts

@@ -1,38 +1,30 @@
 import {
   areCookiesAuthorized,
   combine,
-  Configuration,
   Context,
   createEventRateLimiter,
   InternalMonitoring,
   Observable,
   RawError,
   RelativeTime,
-  InitConfiguration,
   trackRuntimeError,
   trackConsoleError,
   canUseEventBridge,
   getEventBridge,
   getRelativeTime,
-  updateExperimentalFeatures,
-  buildConfiguration,
   startInternalMonitoring,
 } from '@datadog/browser-core'
 import { trackNetworkError } from '../domain/trackNetworkError'
 import { Logger, LogsMessage, StatusType } from '../domain/logger'
 import { LogsSessionManager, startLogsSessionManager, startLogsSessionManagerStub } from '../domain/logsSessionManager'
-import { LogsEvent } from '../logsEvent.types'
 import { startLoggerBatch } from '../transport/startLoggerBatch'
-import { buildEnv } from './buildEnv'
+import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration'
 
-export interface LogsInitConfiguration extends InitConfiguration {
-  forwardErrorsToLogs?: boolean | undefined
-  beforeSend?: ((event: LogsEvent) => void | boolean) | undefined
-}
-
-export function startLogs(initConfiguration: LogsInitConfiguration, errorLogger: Logger) {
-  updateExperimentalFeatures(initConfiguration.enableExperimentalFeatures)
-  const configuration = buildConfiguration(initConfiguration, buildEnv)
+export function startLogs(
+  initConfiguration: LogsInitConfiguration,
+  configuration: LogsConfiguration,
+  errorLogger: Logger
+) {
   const internalMonitoring = startInternalMonitoring(configuration)
 
   const errorObservable = new Observable<RawError>()
@@ -52,7 +44,7 @@ export function startLogs(initConfiguration: LogsInitConfiguration, errorLogger:
 }
 
 export function doStartLogs(
-  configuration: Configuration,
+  configuration: LogsConfiguration,
   errorObservable: Observable<RawError>,
   internalMonitoring: InternalMonitoring,
   sessionManager: LogsSessionManager,
@@ -111,7 +103,7 @@ export function doStartLogs(
 
 export function buildAssemble(
   sessionManager: LogsSessionManager,
-  configuration: Configuration,
+  configuration: LogsConfiguration,
   reportError: (error: RawError) => void
 ) {
   const errorRateLimiter = createEventRateLimiter(StatusType.error, configuration.maxErrorsPerMinute, reportError)

packages/logs/src/domain/configuration.ts

@@ -0,0 +1,23 @@
+import { Configuration, InitConfiguration, validateAndBuildConfiguration } from '@datadog/browser-core'
+import { buildEnv } from '../boot/buildEnv'
+import { LogsEvent } from '../logsEvent.types'
+
+export interface LogsInitConfiguration extends InitConfiguration {
+  forwardErrorsToLogs?: boolean | undefined
+  beforeSend?: ((event: LogsEvent) => void | boolean) | undefined
+}
+
+export type HybridInitConfiguration = Omit<LogsInitConfiguration, 'clientToken'>
+
+export type LogsConfiguration = Configuration
+
+export function validateAndBuildLogsConfiguration(
+  initConfiguration: LogsInitConfiguration
+): LogsConfiguration | undefined {
+  const configuration = validateAndBuildConfiguration(initConfiguration, buildEnv)
+  if (!configuration) {
+    return
+  }
+
+  return configuration
+}