From 46b5a73417b220a10bb08da1dedb0510cd9d3543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Dec 2021 15:55:14 +0100 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=9A=9A=20[RUMF-1097][logs]=20move=20L?= =?UTF-8?q?ogsInitConfiguration=20and=20HybridConfiguration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/logsPublicApi.spec.ts | 4 ++-- packages/logs/src/boot/logsPublicApi.ts | 5 ++--- packages/logs/src/boot/startLogs.spec.ts | 3 ++- packages/logs/src/domain/configuration.ts | 9 +++++++++ packages/logs/src/entries/main.ts | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 packages/logs/src/domain/configuration.ts diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index 8d2074bc7c..df3c46384b 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -7,10 +7,10 @@ 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' } diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index 52cb3a7ee3..b698d0d0f5 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -11,8 +11,9 @@ import { InitConfiguration, canUseEventBridge, } from '@datadog/browser-core' +import { LogsInitConfiguration } 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 @@ -20,8 +21,6 @@ export interface LoggerConfiguration { context?: object } -export type HybridInitConfiguration = Omit - export type LogsPublicApi = ReturnType export type StartLogs = typeof startLogs diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index a6a695358c..a55370d12e 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -21,11 +21,12 @@ import { mockClock, stubEndpointBuilder, } from '../../../core/test/specHelper' +import { 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 } diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts new file mode 100644 index 0000000000..0497b56156 --- /dev/null +++ b/packages/logs/src/domain/configuration.ts @@ -0,0 +1,9 @@ +import { InitConfiguration } from '@datadog/browser-core' +import { LogsEvent } from '../logsEvent.types' + +export interface LogsInitConfiguration extends InitConfiguration { + forwardErrorsToLogs?: boolean | undefined + beforeSend?: ((event: LogsEvent) => void | boolean) | undefined +} + +export type HybridInitConfiguration = Omit diff --git a/packages/logs/src/entries/main.ts b/packages/logs/src/entries/main.ts index f7fbb69a11..674a51e1c0 100644 --- a/packages/logs/src/entries/main.ts +++ b/packages/logs/src/entries/main.ts @@ -4,7 +4,7 @@ import { startLogs } from '../boot/startLogs' export { Logger, LogsMessage, StatusType, HandlerType } from '../domain/logger' export { LoggerConfiguration, LogsPublicApi as LogsGlobal } from '../boot/logsPublicApi' -export { LogsInitConfiguration } from '../boot/startLogs' +export { LogsInitConfiguration } from '../domain/configuration' export { LogsEvent } from '../logsEvent.types' export const datadogLogs = makeLogsPublicApi(startLogs) From 93ed1ec9d8276a26d2e1e00e47a5e9c5fe8227ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Dec 2021 15:57:07 +0100 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20=20[RUMF-1097][logs?= =?UTF-8?q?]=20expose=20a=20new=20LogsConfiguration=20replacing=20Configur?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/startLogs.spec.ts | 15 +++++------ packages/logs/src/boot/startLogs.ts | 6 ++--- packages/logs/src/domain/configuration.ts | 4 ++- .../src/domain/logsSessionManager.spec.ts | 26 +++++++++---------- .../logs/src/domain/logsSessionManager.ts | 11 ++++---- .../logs/src/domain/trackNetworkError.spec.ts | 7 ++--- packages/logs/src/domain/trackNetworkError.ts | 6 ++--- .../logs/src/transport/startLoggerBatch.ts | 5 ++-- 8 files changed, 42 insertions(+), 38 deletions(-) diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index a55370d12e..01d46f34f3 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -1,5 +1,4 @@ import { - Configuration, Context, DEFAULT_CONFIGURATION, ErrorSource, @@ -21,7 +20,7 @@ import { mockClock, stubEndpointBuilder, } from '../../../core/test/specHelper' -import { LogsInitConfiguration } from '../domain/configuration' +import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' import { Logger, LogsMessage, StatusType } from '../domain/logger' import { LogsSessionManager } from '../domain/logsSessionManager' @@ -42,7 +41,7 @@ function getLoggedMessage(server: sinon.SinonFakeServer, index: number) { } const FAKE_DATE = 123456 const SESSION_ID = 'session-id' -const baseConfiguration: Partial = { +const baseConfiguration: Partial = { ...DEFAULT_CONFIGURATION, logsEndpointBuilder: stubEndpointBuilder('https://localhost/v1/input/log'), maxBatchSize: 1, @@ -71,8 +70,8 @@ describe('logs', () => { const startLogs = ({ errorLogger = new Logger(noop), configuration: configurationOverrides, - }: { errorLogger?: Logger; configuration?: Partial } = {}) => { - const configuration = { ...(baseConfiguration as Configuration), ...configurationOverrides } + }: { errorLogger?: Logger; configuration?: Partial } = {}) => { + const configuration = { ...(baseConfiguration as LogsConfiguration), ...configurationOverrides } return doStartLogs(configuration, errorObservable, internalMonitoring, sessionManager, errorLogger) } @@ -181,13 +180,13 @@ describe('logs', () => { updateExperimentalFeatures(['event-bridge']) const sendSpy = spyOn(initEventBridgeStub(), 'send') - let configuration = { ...baseConfiguration, ...{ sampleRate: 0 } } as LogsInitConfiguration + let configuration = { ...baseConfiguration, sampleRate: 0 } as LogsInitConfiguration let sendLog = originalStartLogs(configuration, new Logger(noop)) sendLog(DEFAULT_MESSAGE, {}) expect(sendSpy).not.toHaveBeenCalled() - configuration = { ...baseConfiguration, ...{ sampleRate: 100 } } as LogsInitConfiguration + configuration = { ...baseConfiguration, sampleRate: 100 } as LogsInitConfiguration sendLog = originalStartLogs(configuration, new Logger(noop)) sendLog(DEFAULT_MESSAGE, {}) @@ -204,7 +203,7 @@ describe('logs', () => { assemble = buildAssemble( sessionManager, { - ...(baseConfiguration as Configuration), + ...(baseConfiguration as LogsConfiguration), beforeSend: (x: LogsEvent) => beforeSend(x), }, noop diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index b00e1cc15e..dd476fcd15 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -1,7 +1,6 @@ import { areCookiesAuthorized, combine, - Configuration, Context, createEventRateLimiter, InternalMonitoring, @@ -23,6 +22,7 @@ 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 { LogsConfiguration } from '../domain/configuration' import { buildEnv } from './buildEnv' export interface LogsInitConfiguration extends InitConfiguration { @@ -52,7 +52,7 @@ export function startLogs(initConfiguration: LogsInitConfiguration, errorLogger: } export function doStartLogs( - configuration: Configuration, + configuration: LogsConfiguration, errorObservable: Observable, internalMonitoring: InternalMonitoring, sessionManager: LogsSessionManager, @@ -111,7 +111,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) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 0497b56156..f43fb3806c 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -1,4 +1,4 @@ -import { InitConfiguration } from '@datadog/browser-core' +import { Configuration, InitConfiguration } from '@datadog/browser-core' import { LogsEvent } from '../logsEvent.types' export interface LogsInitConfiguration extends InitConfiguration { @@ -7,3 +7,5 @@ export interface LogsInitConfiguration extends InitConfiguration { } export type HybridInitConfiguration = Omit + +export type LogsConfiguration = Configuration diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index 978fbd4d86..81adca7f35 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -1,5 +1,4 @@ import { - Configuration, COOKIE_ACCESS_DELAY, getCookie, SESSION_COOKIE_NAME, @@ -10,6 +9,7 @@ import { } from '@datadog/browser-core' import { Clock, mockClock } from '../../../core/test/specHelper' +import { LogsConfiguration } from './configuration' import { LOGS_SESSION_KEY, LoggerTrackingType, @@ -19,7 +19,7 @@ import { describe('logs session manager', () => { const DURATION = 123456 - const configuration: Partial = { sampleRate: 0.5 } + const configuration: Partial = { sampleRate: 0.5 } let clock: Clock let tracked: boolean @@ -40,7 +40,7 @@ describe('logs session manager', () => { it('when tracked should store tracking type and session id', () => { tracked = true - startLogsSessionManager(configuration as Configuration) + startLogsSessionManager(configuration as LogsConfiguration) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/) @@ -49,7 +49,7 @@ describe('logs session manager', () => { it('when not tracked should store tracking type', () => { tracked = false - startLogsSessionManager(configuration as Configuration) + startLogsSessionManager(configuration as LogsConfiguration) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=') @@ -58,7 +58,7 @@ describe('logs session manager', () => { it('when tracked should keep existing tracking type and session id', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=1', DURATION) - startLogsSessionManager(configuration as Configuration) + startLogsSessionManager(configuration as LogsConfiguration) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.TRACKED}`) expect(getCookie(SESSION_COOKIE_NAME)).toContain('id=abcdef') @@ -67,13 +67,13 @@ describe('logs session manager', () => { it('when not tracked should keep existing tracking type', () => { setCookie(SESSION_COOKIE_NAME, 'logs=0', DURATION) - startLogsSessionManager(configuration as Configuration) + startLogsSessionManager(configuration as LogsConfiguration) expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`) }) it('should renew on activity after expiration', () => { - startLogsSessionManager(configuration as Configuration) + startLogsSessionManager(configuration as LogsConfiguration) setCookie(SESSION_COOKIE_NAME, '', DURATION) expect(getCookie(SESSION_COOKIE_NAME)).toBeUndefined() @@ -89,18 +89,18 @@ describe('logs session manager', () => { describe('findSession', () => { it('should return the current session', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=1', DURATION) - const logsSessionManager = startLogsSessionManager(configuration as Configuration) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) expect(logsSessionManager.findTrackedSession()!.id).toBe('abcdef') }) it('should return undefined if the session is not tracked', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=0', DURATION) - const logsSessionManager = startLogsSessionManager(configuration as Configuration) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) expect(logsSessionManager.findTrackedSession()).toBe(undefined) }) it('should return undefined if the session has expired', () => { - const logsSessionManager = startLogsSessionManager(configuration as Configuration) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) setCookie(SESSION_COOKIE_NAME, '', DURATION) clock.tick(COOKIE_ACCESS_DELAY) expect(logsSessionManager.findTrackedSession()).toBe(undefined) @@ -108,7 +108,7 @@ describe('logs session manager', () => { it('should return session corresponding to start time', () => { setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=1', DURATION) - const logsSessionManager = startLogsSessionManager(configuration as Configuration) + const logsSessionManager = startLogsSessionManager(configuration as LogsConfiguration) clock.tick(10 * ONE_SECOND) setCookie(SESSION_COOKIE_NAME, '', DURATION) clock.tick(COOKIE_ACCESS_DELAY) @@ -120,11 +120,11 @@ describe('logs session manager', () => { describe('logger session stub', () => { it('isTracked is computed at each init and getId is always undefined', () => { - const firstLogsSessionManager = startLogsSessionManagerStub({ sampleRate: 100 } as Configuration) + const firstLogsSessionManager = startLogsSessionManagerStub({ sampleRate: 100 } as LogsConfiguration) expect(firstLogsSessionManager.findTrackedSession()).toBeDefined() expect(firstLogsSessionManager.findTrackedSession()!.id).toBeUndefined() - const secondLogsSessionManager = startLogsSessionManagerStub({ sampleRate: 0 } as Configuration) + const secondLogsSessionManager = startLogsSessionManagerStub({ sampleRate: 0 } as LogsConfiguration) expect(secondLogsSessionManager.findTrackedSession()).toBeUndefined() }) }) diff --git a/packages/logs/src/domain/logsSessionManager.ts b/packages/logs/src/domain/logsSessionManager.ts index 9bcf2f7f50..21e3b1f6e7 100644 --- a/packages/logs/src/domain/logsSessionManager.ts +++ b/packages/logs/src/domain/logsSessionManager.ts @@ -1,4 +1,5 @@ -import { Configuration, performDraw, startSessionManager, RelativeTime } from '@datadog/browser-core' +import { performDraw, startSessionManager, RelativeTime } from '@datadog/browser-core' +import { LogsConfiguration } from './configuration' export const LOGS_SESSION_KEY = 'logs' @@ -15,7 +16,7 @@ export enum LoggerTrackingType { TRACKED = '1', } -export function startLogsSessionManager(configuration: Configuration): LogsSessionManager { +export function startLogsSessionManager(configuration: LogsConfiguration): LogsSessionManager { const sessionManager = startSessionManager(configuration.cookieOptions, LOGS_SESSION_KEY, (rawTrackingType) => computeSessionState(configuration, rawTrackingType) ) @@ -31,7 +32,7 @@ export function startLogsSessionManager(configuration: Configuration): LogsSessi } } -export function startLogsSessionManagerStub(configuration: Configuration): LogsSessionManager { +export function startLogsSessionManagerStub(configuration: LogsConfiguration): LogsSessionManager { const isTracked = computeTrackingType(configuration) === LoggerTrackingType.TRACKED const session = isTracked ? {} : undefined return { @@ -39,14 +40,14 @@ export function startLogsSessionManagerStub(configuration: Configuration): LogsS } } -function computeTrackingType(configuration: Configuration) { +function computeTrackingType(configuration: LogsConfiguration) { if (!performDraw(configuration.sampleRate)) { return LoggerTrackingType.NOT_TRACKED } return LoggerTrackingType.TRACKED } -function computeSessionState(configuration: Configuration, rawSessionType?: string) { +function computeSessionState(configuration: LogsConfiguration, rawSessionType?: string) { const trackingType = hasValidLoggerSession(rawSessionType) ? rawSessionType : computeTrackingType(configuration) return { trackingType, diff --git a/packages/logs/src/domain/trackNetworkError.spec.ts b/packages/logs/src/domain/trackNetworkError.spec.ts index 460bda5212..84527ef1cc 100644 --- a/packages/logs/src/domain/trackNetworkError.spec.ts +++ b/packages/logs/src/domain/trackNetworkError.spec.ts @@ -1,5 +1,6 @@ -import { Configuration, isIE, Observable, RawError } from '@datadog/browser-core' +import { isIE, Observable, RawError } from '@datadog/browser-core' import { FetchStub, FetchStubManager, SPEC_ENDPOINTS, stubFetch } from '../../../core/test/specHelper' +import { LogsConfiguration } from './configuration' import { trackNetworkError } from './trackNetworkError' @@ -8,7 +9,7 @@ describe('network error tracker', () => { let fetchStub: FetchStub let fetchStubManager: FetchStubManager let stopNetworkErrorTracking: () => void - let configuration: Configuration + let configuration: LogsConfiguration let errorObservable: Observable const FAKE_URL = 'http://fake.com/' const DEFAULT_REQUEST = { @@ -29,7 +30,7 @@ describe('network error tracker', () => { configuration = { requestErrorResponseLengthLimit: 32, ...SPEC_ENDPOINTS, - } as Configuration + } as LogsConfiguration fetchStubManager = stubFetch() ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration, errorObservable)) diff --git a/packages/logs/src/domain/trackNetworkError.ts b/packages/logs/src/domain/trackNetworkError.ts index 3c5638cd41..c48caee555 100644 --- a/packages/logs/src/domain/trackNetworkError.ts +++ b/packages/logs/src/domain/trackNetworkError.ts @@ -1,5 +1,4 @@ import { - Configuration, ErrorSource, FetchCompleteContext, initXhrObservable, @@ -9,8 +8,9 @@ import { initFetchObservable, XhrCompleteContext, } from '@datadog/browser-core' +import { LogsConfiguration } from './configuration' -export function trackNetworkError(configuration: Configuration, errorObservable: Observable) { +export function trackNetworkError(configuration: LogsConfiguration, errorObservable: Observable) { const xhrSubscription = initXhrObservable().subscribe((context) => { if (context.state === 'complete') { handleCompleteRequest(RequestType.XHR, context) @@ -54,7 +54,7 @@ function isServerError(request: { status: number }) { return request.status >= 500 } -function truncateResponseText(responseText: string | undefined, configuration: Configuration) { +function truncateResponseText(responseText: string | undefined, configuration: LogsConfiguration) { if (responseText && responseText.length > configuration.requestErrorResponseLengthLimit) { return `${responseText.substring(0, configuration.requestErrorResponseLengthLimit)}...` } diff --git a/packages/logs/src/transport/startLoggerBatch.ts b/packages/logs/src/transport/startLoggerBatch.ts index 089054aa5d..3466b68ff1 100644 --- a/packages/logs/src/transport/startLoggerBatch.ts +++ b/packages/logs/src/transport/startLoggerBatch.ts @@ -1,6 +1,7 @@ -import { Batch, Configuration, Context, HttpRequest, EndpointBuilder } from '@datadog/browser-core' +import { Batch, Context, HttpRequest, EndpointBuilder } from '@datadog/browser-core' +import { LogsConfiguration } from '../domain/configuration' -export function startLoggerBatch(configuration: Configuration) { +export function startLoggerBatch(configuration: LogsConfiguration) { const primaryBatch = createLoggerBatch(configuration.logsEndpointBuilder) let replicaBatch: Batch | undefined From 57cc29088e8d53b61590edfe855fa6939c5d7e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Dec 2021 15:21:27 +0100 Subject: [PATCH 3/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUMF-1097][logs]=20mo?= =?UTF-8?q?ve=20configuration=20build=20into=20init()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will help using a future "validateAndBuildConfiguration", and makes the Logs implementation closer to the RUM one. --- packages/logs/src/boot/logsPublicApi.ts | 8 +++++++- packages/logs/src/boot/startLogs.spec.ts | 8 ++++---- packages/logs/src/boot/startLogs.ts | 11 +++++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index b698d0d0f5..622027dd59 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -10,9 +10,12 @@ import { deepClone, InitConfiguration, canUseEventBridge, + updateExperimentalFeatures, + buildConfiguration, } from '@datadog/browser-core' import { LogsInitConfiguration } from '../domain/configuration' import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger' +import { buildEnv } from './buildEnv' import { startLogs } from './startLogs' export interface LoggerConfiguration { @@ -50,7 +53,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { return } - sendLogStrategy = startLogsImpl(initConfiguration, logger) + updateExperimentalFeatures(initConfiguration.enableExperimentalFeatures) + const configuration = buildConfiguration(initConfiguration, buildEnv) + + sendLogStrategy = startLogsImpl(initConfiguration, configuration, logger) getInitConfigurationStrategy = () => deepClone(initConfiguration) beforeInitSendLog.drain() diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 01d46f34f3..e9bbaa96f6 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -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() diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index dd476fcd15..f79bb1e791 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -13,8 +13,6 @@ import { canUseEventBridge, getEventBridge, getRelativeTime, - updateExperimentalFeatures, - buildConfiguration, startInternalMonitoring, } from '@datadog/browser-core' import { trackNetworkError } from '../domain/trackNetworkError' @@ -23,16 +21,17 @@ import { LogsSessionManager, startLogsSessionManager, startLogsSessionManagerStu import { LogsEvent } from '../logsEvent.types' import { startLoggerBatch } from '../transport/startLoggerBatch' import { LogsConfiguration } from '../domain/configuration' -import { buildEnv } from './buildEnv' 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() From 262acfcf6f671d7c8b7e2e40b8333bb2ea849cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Dec 2021 15:36:55 +0100 Subject: [PATCH 4/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUMF-1097][logs]=20us?= =?UTF-8?q?e=20the=20core=20validateAndBuildConfiguration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../configuration/configuration.spec.ts | 5 ++ packages/logs/src/boot/logsPublicApi.spec.ts | 71 ++++++++----------- packages/logs/src/boot/logsPublicApi.ts | 20 ++---- packages/logs/src/boot/startLogs.ts | 9 +-- packages/logs/src/domain/configuration.ts | 14 +++- 5 files changed, 55 insertions(+), 64 deletions(-) diff --git a/packages/core/src/domain/configuration/configuration.spec.ts b/packages/core/src/domain/configuration/configuration.spec.ts index da652e642b..723082aa4c 100644 --- a/packages/core/src/domain/configuration/configuration.spec.ts +++ b/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', () => { diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index df3c46384b..d5136bf3e5 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -13,6 +13,7 @@ import { HandlerType, LogsMessage, StatusType } from '../domain/logger' 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 + let startLogs: jasmine.Spy 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() }) }) }) diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index 622027dd59..790581a7ee 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -3,19 +3,15 @@ import { combine, Context, createContextManager, - isPercentage, makePublicApi, monitor, display, deepClone, InitConfiguration, canUseEventBridge, - updateExperimentalFeatures, - buildConfiguration, } from '@datadog/browser-core' -import { LogsInitConfiguration } from '../domain/configuration' +import { LogsInitConfiguration, validateAndBuildLogsConfiguration } from '../domain/configuration' import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger' -import { buildEnv } from './buildEnv' import { startLogs } from './startLogs' export interface LoggerConfiguration { @@ -53,8 +49,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { return } - updateExperimentalFeatures(initConfiguration.enableExperimentalFeatures) - const configuration = buildConfiguration(initConfiguration, buildEnv) + const configuration = validateAndBuildLogsConfiguration(initConfiguration) + if (!configuration) { + return + } sendLogStrategy = startLogsImpl(initConfiguration, configuration, logger) getInitConfigurationStrategy = () => deepClone(initConfiguration) @@ -94,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 } diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index f79bb1e791..e1a690e464 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -7,7 +7,6 @@ import { Observable, RawError, RelativeTime, - InitConfiguration, trackRuntimeError, trackConsoleError, canUseEventBridge, @@ -18,14 +17,8 @@ import { 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 { LogsConfiguration } from '../domain/configuration' - -export interface LogsInitConfiguration extends InitConfiguration { - forwardErrorsToLogs?: boolean | undefined - beforeSend?: ((event: LogsEvent) => void | boolean) | undefined -} +import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' export function startLogs( initConfiguration: LogsInitConfiguration, diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index f43fb3806c..d4a18c8853 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -1,4 +1,5 @@ -import { Configuration, InitConfiguration } from '@datadog/browser-core' +import { Configuration, InitConfiguration, validateAndBuildConfiguration } from '@datadog/browser-core' +import { buildEnv } from '../boot/buildEnv' import { LogsEvent } from '../logsEvent.types' export interface LogsInitConfiguration extends InitConfiguration { @@ -9,3 +10,14 @@ export interface LogsInitConfiguration extends InitConfiguration { export type HybridInitConfiguration = Omit export type LogsConfiguration = Configuration + +export function validateAndBuildLogsConfiguration( + initConfiguration: LogsInitConfiguration +): LogsConfiguration | undefined { + const configuration = validateAndBuildConfiguration(initConfiguration, buildEnv) + if (!configuration) { + return + } + + return configuration +} From 67e0ea9564881d0f7b5fdae4d67651e48575a2fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Dec 2021 16:03:37 +0100 Subject: [PATCH 5/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUMF-1097][logs]=20ad?= =?UTF-8?q?d=20forwardErrorToLogs=20to=20LogsConfiguration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So we can keep usages of LogsInitConfiguration inside the init() function --- packages/logs/src/boot/logsPublicApi.ts | 2 +- packages/logs/src/boot/startLogs.spec.ts | 6 ++--- packages/logs/src/boot/startLogs.ts | 10 +++----- .../logs/src/domain/configuration.spec.ts | 25 +++++++++++++++++++ packages/logs/src/domain/configuration.ts | 19 +++++++++++--- 5 files changed, 48 insertions(+), 14 deletions(-) create mode 100644 packages/logs/src/domain/configuration.spec.ts diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index 790581a7ee..512ea58de1 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -54,7 +54,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { return } - sendLogStrategy = startLogsImpl(initConfiguration, configuration, logger) + sendLogStrategy = startLogsImpl(configuration, logger) getInitConfigurationStrategy = () => deepClone(initConfiguration) beforeInitSendLog.drain() diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index e9bbaa96f6..1892580c22 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -20,7 +20,7 @@ import { mockClock, stubEndpointBuilder, } from '../../../core/test/specHelper' -import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' +import { LogsConfiguration } from '../domain/configuration' import { Logger, LogsMessage, StatusType } from '../domain/logger' import { LogsSessionManager } from '../domain/logsSessionManager' @@ -181,13 +181,13 @@ describe('logs', () => { const sendSpy = spyOn(initEventBridgeStub(), 'send') let configuration = { ...baseConfiguration, sampleRate: 0 } as LogsConfiguration - let sendLog = originalStartLogs({} as LogsInitConfiguration, configuration, new Logger(noop)) + let sendLog = originalStartLogs(configuration, new Logger(noop)) sendLog(DEFAULT_MESSAGE, {}) expect(sendSpy).not.toHaveBeenCalled() configuration = { ...baseConfiguration, sampleRate: 100 } as LogsConfiguration - sendLog = originalStartLogs({} as LogsInitConfiguration, configuration, new Logger(noop)) + sendLog = originalStartLogs(configuration, new Logger(noop)) sendLog(DEFAULT_MESSAGE, {}) expect(sendSpy).toHaveBeenCalled() diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index e1a690e464..18a63ec26a 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -18,18 +18,14 @@ import { trackNetworkError } from '../domain/trackNetworkError' import { Logger, LogsMessage, StatusType } from '../domain/logger' import { LogsSessionManager, startLogsSessionManager, startLogsSessionManagerStub } from '../domain/logsSessionManager' import { startLoggerBatch } from '../transport/startLoggerBatch' -import { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' +import { LogsConfiguration } from '../domain/configuration' -export function startLogs( - initConfiguration: LogsInitConfiguration, - configuration: LogsConfiguration, - errorLogger: Logger -) { +export function startLogs(configuration: LogsConfiguration, errorLogger: Logger) { const internalMonitoring = startInternalMonitoring(configuration) const errorObservable = new Observable() - if (initConfiguration.forwardErrorsToLogs !== false) { + if (configuration.forwardErrorsToLogs) { trackConsoleError(errorObservable) trackRuntimeError(errorObservable) trackNetworkError(configuration, errorObservable) diff --git a/packages/logs/src/domain/configuration.spec.ts b/packages/logs/src/domain/configuration.spec.ts new file mode 100644 index 0000000000..6fcfb6d088 --- /dev/null +++ b/packages/logs/src/domain/configuration.spec.ts @@ -0,0 +1,25 @@ +import { validateAndBuildLogsConfiguration } from './configuration' + +const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' } + +describe('validateAndBuildLogsConfiguration', () => { + describe('forwardErrorsToLogs', () => { + it('defaults to false if the option is not provided', () => { + expect(validateAndBuildLogsConfiguration(DEFAULT_INIT_CONFIGURATION)!.forwardErrorsToLogs).toBeFalse() + }) + + it('is set to provided value', () => { + expect( + validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: true })! + .forwardErrorsToLogs + ).toBeTrue() + }) + + it('the provided value is cast to boolean', () => { + expect( + validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: 'foo' as any })! + .forwardErrorsToLogs + ).toBeTrue() + }) + }) +}) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index d4a18c8853..c1b0cee35f 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -9,15 +9,28 @@ export interface LogsInitConfiguration extends InitConfiguration { export type HybridInitConfiguration = Omit -export type LogsConfiguration = Configuration +const DEFAULT_LOGS_CONFIGURATION = { + forwardErrorsToLogs: false, +} + +export type LogsConfiguration = Configuration & typeof DEFAULT_LOGS_CONFIGURATION export function validateAndBuildLogsConfiguration( initConfiguration: LogsInitConfiguration ): LogsConfiguration | undefined { - const configuration = validateAndBuildConfiguration(initConfiguration, buildEnv) - if (!configuration) { + const baseConfiguration = validateAndBuildConfiguration(initConfiguration, buildEnv) + if (!baseConfiguration) { return } + const configuration: LogsConfiguration = { + ...baseConfiguration, + ...DEFAULT_LOGS_CONFIGURATION, + } + + if (initConfiguration.forwardErrorsToLogs !== undefined) { + configuration.forwardErrorsToLogs = !!initConfiguration.forwardErrorsToLogs + } + return configuration } From 2703613a0c05a7c2c3e7db9e860c9eef3f0588d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 15 Dec 2021 15:12:14 +0100 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=91=8C=E2=9C=85=20[RUMF-1097]=20add?= =?UTF-8?q?=20a=20test=20case=20for=20invalid=20sample=20rate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/src/domain/configuration/configuration.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/core/src/domain/configuration/configuration.spec.ts b/packages/core/src/domain/configuration/configuration.spec.ts index 723082aa4c..8daf181039 100644 --- a/packages/core/src/domain/configuration/configuration.spec.ts +++ b/packages/core/src/domain/configuration/configuration.spec.ts @@ -101,6 +101,12 @@ describe('validateAndBuildConfiguration', () => { validateAndBuildConfiguration(({ clientToken, sampleRate: 'foo' } as unknown) as InitConfiguration, buildEnv) ).toBeUndefined() expect(displaySpy).toHaveBeenCalledOnceWith('Sample Rate should be a number between 0 and 100') + + displaySpy.calls.reset() + expect( + validateAndBuildConfiguration(({ clientToken, sampleRate: 200 } as unknown) as InitConfiguration, buildEnv) + ).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", () => {