From 43b94ab23339830315a3e6427acf2d08ff87d17b Mon Sep 17 00:00:00 2001 From: "chain-pr[bot]" Date: Fri, 8 May 2026 19:44:38 +0200 Subject: [PATCH] [chain-pr 8/12] Migrate Logs SDK to async SessionManager and bufferedData consumers --- packages/logs/src/boot/logsPublicApi.spec.ts | 44 +++- packages/logs/src/boot/logsPublicApi.ts | 15 +- packages/logs/src/boot/preStartLogs.spec.ts | 79 +++--- packages/logs/src/boot/preStartLogs.ts | 59 +++-- packages/logs/src/boot/startLogs.spec.ts | 107 +------- packages/logs/src/boot/startLogs.ts | 29 +-- .../logs/src/domain/configuration.spec.ts | 33 +-- packages/logs/src/domain/configuration.ts | 24 +- .../domain/console/consoleCollection.spec.ts | 168 ++++++------- .../src/domain/console/consoleCollection.ts | 22 +- .../domain/contexts/internalContext.spec.ts | 6 +- .../src/domain/contexts/internalContext.ts | 5 +- .../domain/contexts/sessionContext.spec.ts | 41 +-- .../src/domain/contexts/sessionContext.ts | 26 +- .../createErrorFieldFromRawError.spec.ts | 30 ++- packages/logs/src/domain/logger.spec.ts | 15 +- .../networkErrorCollection.spec.ts | 237 +++++++----------- .../networkError/networkErrorCollection.ts | 42 ++-- .../runtimeErrorCollection.spec.ts | 8 +- .../runtimeError/runtimeErrorCollection.ts | 2 +- packages/logs/src/domainContext.types.ts | 1 - packages/logs/src/transport/startLogsBatch.ts | 7 +- 22 files changed, 424 insertions(+), 576 deletions(-) diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index 24a0a6df94..5b09b32c17 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -1,8 +1,21 @@ import type { ContextManager } from '@datadog/browser-core' -import { monitor, display, createContextManager, TrackingConsent, startTelemetry } from '@datadog/browser-core' +import { + monitor, + display, + createContextManager, + TrackingConsent, + startTelemetry, + startSessionManager, +} from '@datadog/browser-core' +import { + collectAsyncCalls, + createFakeTelemetryObject, + replaceMockable, + replaceMockableWithSpy, + createStartSessionManagerMock, +} from '@datadog/browser-core/test' import { HandlerType } from '../domain/logger' import { StatusType } from '../domain/logger/isAuthorized' -import { createFakeTelemetryObject, replaceMockable, replaceMockableWithSpy } from '../../../core/test' import type { LogsPublicApi } from './logsPublicApi' import { makeLogsPublicApi } from './logsPublicApi' import type { StartLogs, StartLogsResult } from './startLogs' @@ -49,15 +62,16 @@ describe('logs entry', () => { let logsPublicApi: LogsPublicApi let startLogsSpy: jasmine.Spy - beforeEach(() => { + beforeEach(async () => { ;({ logsPublicApi, startLogsSpy } = makeLogsPublicApiWithDefaults()) logsPublicApi.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(startLogsSpy, 1) }) it('should have the current date, view and global context', () => { logsPublicApi.setGlobalContextProperty('foo', 'bar') - const getCommonContext = startLogsSpy.calls.mostRecent().args[1] + const getCommonContext = startLogsSpy.calls.mostRecent().args[2] expect(getCommonContext()).toEqual({ view: { referrer: document.referrer, @@ -67,22 +81,38 @@ describe('logs entry', () => { }) }) + it('buffered calls should be replayed before microtasks scheduled after init', async () => { + const { logsPublicApi, handleLogSpy, getLoggedMessage } = makeLogsPublicApiWithDefaults() + logsPublicApi.logger.log('before_init') + logsPublicApi.init(DEFAULT_INIT_CONFIGURATION) + + // Simulate user code scheduling a microtask right after init + void Promise.resolve().then(() => logsPublicApi.logger.log('after_init')) + + await collectAsyncCalls(handleLogSpy, 2) + + expect(getLoggedMessage(0).message.message).toBe('before_init') + expect(getLoggedMessage(1).message.message).toBe('after_init') + }) + describe('post start API usages', () => { let logsPublicApi: LogsPublicApi let getLoggedMessage: ReturnType['getLoggedMessage'] + let startLogsSpy: jasmine.Spy let userContext: ContextManager let accountContext: ContextManager - beforeEach(() => { + beforeEach(async () => { userContext = createContextManager('mock') accountContext = createContextManager('mock') - ;({ logsPublicApi, getLoggedMessage } = makeLogsPublicApiWithDefaults({ + ;({ logsPublicApi, getLoggedMessage, startLogsSpy } = makeLogsPublicApiWithDefaults({ startLogsResult: { userContext, accountContext, }, })) logsPublicApi.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(startLogsSpy, 1) }) it('main logger logs a message', () => { @@ -240,9 +270,11 @@ function makeLogsPublicApiWithDefaults({ } replaceMockable(startTelemetry, createFakeTelemetryObject) + replaceMockable(startSessionManager, createStartSessionManagerMock()) return { startLogsSpy, + handleLogSpy, logsPublicApi: makeLogsPublicApi(), getLoggedMessage, } diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index f8ec2363b4..fac3a02d2d 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -274,16 +274,16 @@ export function makeLogsPublicApi(): LogsPublicApi { let strategy = createPreStartStrategy( buildCommonContext, trackingConsentState, - (initConfiguration, configuration, hooks) => { + (configuration, logsSessionManager, hooks) => { const startLogsResult = mockable(startLogs)( configuration, + logsSessionManager, buildCommonContext, - trackingConsentState, bufferedDataObservable, hooks ) - strategy = createPostStartStrategy(initConfiguration, startLogsResult) + strategy = createPostStartStrategy(strategy, startLogsResult) return startLogsResult } ) @@ -396,12 +396,15 @@ export function makeLogsPublicApi(): LogsPublicApi { }) } -function createPostStartStrategy(initConfiguration: LogsInitConfiguration, startLogsResult: StartLogsResult): Strategy { +function createPostStartStrategy(preStartStrategy: Strategy, startLogsResult: StartLogsResult): Strategy { return { + ...preStartStrategy, init: (initConfiguration: LogsInitConfiguration) => { displayAlreadyInitializedError('DD_LOGS', initConfiguration) }, - initConfiguration, - ...startLogsResult, + getInternalContext: startLogsResult.getInternalContext, + globalContext: startLogsResult.globalContext, + userContext: startLogsResult.userContext, + accountContext: startLogsResult.accountContext, } } diff --git a/packages/logs/src/boot/preStartLogs.spec.ts b/packages/logs/src/boot/preStartLogs.spec.ts index 395b4f2f68..e0ef424fb1 100644 --- a/packages/logs/src/boot/preStartLogs.spec.ts +++ b/packages/logs/src/boot/preStartLogs.spec.ts @@ -1,13 +1,23 @@ import { - callbackAddsInstrumentation, + collectAsyncCalls, type Clock, mockClock, mockEventBridge, + waitNextMicrotask, createFakeTelemetryObject, + replaceMockable, replaceMockableWithSpy, + createStartSessionManagerMock, } from '@datadog/browser-core/test' import type { TrackingConsentState } from '@datadog/browser-core' -import { ONE_SECOND, TrackingConsent, createTrackingConsentState, display, startTelemetry } from '@datadog/browser-core' +import { + ONE_SECOND, + TrackingConsent, + createTrackingConsentState, + display, + startTelemetry, + startSessionManager, +} from '@datadog/browser-core' import type { CommonContext } from '../rawLogsEvent.types' import type { HybridInitConfiguration, LogsInitConfiguration } from '../domain/configuration' import type { Logger } from '../domain/logger' @@ -37,9 +47,10 @@ describe('preStartLogs', () => { displaySpy = spyOn(display, 'error') }) - it('should start when the configuration is valid', () => { + it('should start when the configuration is valid', async () => { strategy.init(DEFAULT_INIT_CONFIGURATION) expect(displaySpy).not.toHaveBeenCalled() + await collectAsyncCalls(doStartLogsSpy, 1) expect(doStartLogsSpy).toHaveBeenCalled() }) @@ -89,17 +100,27 @@ describe('preStartLogs', () => { mockEventBridge() }) - it('init should accept empty client token', () => { + it('init should accept empty client token', async () => { const hybridInitConfiguration: HybridInitConfiguration = {} strategy.init(hybridInitConfiguration as LogsInitConfiguration) + await collectAsyncCalls(doStartLogsSpy, 1) expect(displaySpy).not.toHaveBeenCalled() expect(doStartLogsSpy).toHaveBeenCalled() }) }) }) - it('allows sending logs', () => { + it('should not start when session manager initialization fails', async () => { + const { strategy, doStartLogsSpy } = createPreStartStrategyWithDefaults({ + startSessionManagerMock: () => Promise.reject(new Error('Session init failed')), + }) + strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(doStartLogsSpy, 0) + expect(doStartLogsSpy).not.toHaveBeenCalled() + }) + + it('allows sending logs', async () => { const { strategy, handleLogSpy, getLoggedMessage } = createPreStartStrategyWithDefaults() strategy.handleLog( { @@ -111,6 +132,7 @@ describe('preStartLogs', () => { expect(handleLogSpy).not.toHaveBeenCalled() strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(handleLogSpy, 1) expect(handleLogSpy.calls.all().length).toBe(1) expect(getLoggedMessage(0).message.message).toBe('message') @@ -122,8 +144,9 @@ describe('preStartLogs', () => { }) describe('save context when submitting a log', () => { - it('saves the date', () => { - const { strategy, getLoggedMessage } = createPreStartStrategyWithDefaults() + it('saves the date', async () => { + mockEventBridge() + const { strategy, getLoggedMessage, handleLogSpy } = createPreStartStrategyWithDefaults() strategy.handleLog( { status: StatusType.info, @@ -133,12 +156,13 @@ describe('preStartLogs', () => { ) clock.tick(ONE_SECOND) strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(handleLogSpy, 1) expect(getLoggedMessage(0).savedDate).toEqual(Date.now() - ONE_SECOND) }) - it('saves the URL', () => { - const { strategy, getLoggedMessage, getCommonContextSpy } = createPreStartStrategyWithDefaults() + it('saves the URL', async () => { + const { strategy, getLoggedMessage, getCommonContextSpy, handleLogSpy } = createPreStartStrategyWithDefaults() getCommonContextSpy.and.returnValue({ view: { url: 'url' } } as unknown as CommonContext) strategy.handleLog( { @@ -149,11 +173,12 @@ describe('preStartLogs', () => { ) strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(handleLogSpy, 1) expect(getLoggedMessage(0).savedCommonContext!.view?.url).toEqual('url') }) - it('saves the log context', () => { - const { strategy, getLoggedMessage } = createPreStartStrategyWithDefaults() + it('saves the log context', async () => { + const { strategy, getLoggedMessage, handleLogSpy } = createPreStartStrategyWithDefaults() const context = { foo: 'bar' } strategy.handleLog( { @@ -166,6 +191,7 @@ describe('preStartLogs', () => { context.foo = 'baz' strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(handleLogSpy, 1) expect(getLoggedMessage(0).message.context!.foo).toEqual('bar') }) @@ -188,21 +214,6 @@ describe('preStartLogs', () => { ;({ strategy, doStartLogsSpy } = createPreStartStrategyWithDefaults({ trackingConsentState })) }) - describe('basic methods instrumentation', () => { - it('should instrument fetch even if tracking consent is not granted', () => { - expect( - callbackAddsInstrumentation(() => { - strategy.init({ - ...DEFAULT_INIT_CONFIGURATION, - trackingConsent: TrackingConsent.NOT_GRANTED, - }) - }) - .toMethod(window, 'fetch') - .whenCalled() - ).toBeTrue() - }) - }) - it('does not start logs if tracking consent is not granted at init', () => { strategy.init({ ...DEFAULT_INIT_CONFIGURATION, @@ -211,12 +222,13 @@ describe('preStartLogs', () => { expect(doStartLogsSpy).not.toHaveBeenCalled() }) - it('starts logs if tracking consent is granted before init', () => { + it('starts logs if tracking consent is granted before init', async () => { trackingConsentState.update(TrackingConsent.GRANTED) strategy.init({ ...DEFAULT_INIT_CONFIGURATION, trackingConsent: TrackingConsent.NOT_GRANTED, }) + await collectAsyncCalls(doStartLogsSpy, 1) expect(doStartLogsSpy).toHaveBeenCalledTimes(1) }) @@ -229,24 +241,27 @@ describe('preStartLogs', () => { expect(doStartLogsSpy).not.toHaveBeenCalled() }) - it('do not call startLogs when tracking consent state is updated after init', () => { + it('do not call startLogs when tracking consent state is updated after init', async () => { strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(doStartLogsSpy, 1) doStartLogsSpy.calls.reset() trackingConsentState.update(TrackingConsent.GRANTED) + await waitNextMicrotask() expect(doStartLogsSpy).not.toHaveBeenCalled() }) }) describe('telemetry', () => { - it('starts telemetry during init() by default', () => { + it('starts telemetry during init() by default', async () => { const { strategy, startTelemetrySpy } = createPreStartStrategyWithDefaults() strategy.init(DEFAULT_INIT_CONFIGURATION) + await collectAsyncCalls(startTelemetrySpy, 1) expect(startTelemetrySpy).toHaveBeenCalledTimes(1) }) - it('does not start telemetry until consent is granted', () => { + it('does not start telemetry until consent is granted', async () => { const trackingConsentState = createTrackingConsentState() const { strategy, startTelemetrySpy } = createPreStartStrategyWithDefaults({ trackingConsentState, @@ -260,6 +275,7 @@ describe('preStartLogs', () => { expect(startTelemetrySpy).not.toHaveBeenCalled() trackingConsentState.update(TrackingConsent.GRANTED) + await collectAsyncCalls(startTelemetrySpy, 1) expect(startTelemetrySpy).toHaveBeenCalledTimes(1) }) @@ -268,8 +284,10 @@ describe('preStartLogs', () => { function createPreStartStrategyWithDefaults({ trackingConsentState = createTrackingConsentState(), + startSessionManagerMock = createStartSessionManagerMock(), }: { trackingConsentState?: TrackingConsentState + startSessionManagerMock?: typeof startSessionManager } = {}) { const handleLogSpy = jasmine.createSpy() const doStartLogsSpy = jasmine.createSpy().and.returnValue({ @@ -277,6 +295,7 @@ function createPreStartStrategyWithDefaults({ } as unknown as StartLogsResult) const getCommonContextSpy = jasmine.createSpy<() => CommonContext>() const startTelemetrySpy = replaceMockableWithSpy(startTelemetry).and.callFake(createFakeTelemetryObject) + replaceMockable(startSessionManager, startSessionManagerMock) return { strategy: createPreStartStrategy(getCommonContextSpy, trackingConsentState, doStartLogsSpy), diff --git a/packages/logs/src/boot/preStartLogs.ts b/packages/logs/src/boot/preStartLogs.ts index 8db0f5cd77..0ca4e263b4 100644 --- a/packages/logs/src/boot/preStartLogs.ts +++ b/packages/logs/src/boot/preStartLogs.ts @@ -1,34 +1,39 @@ -import type { TrackingConsentState } from '@datadog/browser-core' +import type { TrackingConsentState, SessionManager } from '@datadog/browser-core' import { - createBoundedBuffer, + BufferedObservable, canUseEventBridge, display, displayAlreadyInitializedError, initFeatureFlags, - initFetchObservable, + monitorError, noop, timeStampNow, buildAccountContextManager, CustomerContextKey, bufferContextCalls, addTelemetryConfiguration, + addTelemetryDebug, buildGlobalContextManager, buildUserContextManager, + startSessionManager, + startSessionManagerStub, startTelemetry, TelemetryService, mockable, + startTelemetrySessionContext, } from '@datadog/browser-core' import type { Hooks } from '../domain/hooks' import { createHooks } from '../domain/hooks' import type { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' import { serializeLogsConfiguration, validateAndBuildLogsConfiguration } from '../domain/configuration' import type { CommonContext } from '../rawLogsEvent.types' +import { startTrackingConsentContext } from '../domain/contexts/trackingConsentContext' import type { Strategy } from './logsPublicApi' import type { StartLogsResult } from './startLogs' export type DoStartLogs = ( - initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration, + sessionManager: SessionManager, hooks: Hooks ) => StartLogsResult @@ -37,7 +42,11 @@ export function createPreStartStrategy( trackingConsentState: TrackingConsentState, doStartLogs: DoStartLogs ): Strategy { - const bufferApiCalls = createBoundedBuffer() + const BUFFER_LIMIT = 500 + const bufferApiCalls = new BufferedObservable<(startLogsResult: StartLogsResult) => void>(BUFFER_LIMIT, (count) => { + // monitor-until: 2026-10-14 + addTelemetryDebug('preStartLogs buffer data lost', { count }) + }) // TODO next major: remove the globalContext, accountContextManager, userContext from preStartStrategy and use an empty context instead const globalContext = buildGlobalContextManager() @@ -51,20 +60,20 @@ export function createPreStartStrategy( let cachedInitConfiguration: LogsInitConfiguration | undefined let cachedConfiguration: LogsConfiguration | undefined + let sessionManager: SessionManager | undefined const hooks = createHooks() const trackingConsentStateSubscription = trackingConsentState.observable.subscribe(tryStartLogs) function tryStartLogs() { - if (!cachedConfiguration || !cachedInitConfiguration || !trackingConsentState.isGranted()) { + if (!cachedConfiguration || !cachedInitConfiguration || !sessionManager) { return } - mockable(startTelemetry)(TelemetryService.LOGS, cachedConfiguration, hooks) - trackingConsentStateSubscription.unsubscribe() - const startLogsResult = doStartLogs(cachedInitConfiguration, cachedConfiguration, hooks) + const startLogsResult = doStartLogs(cachedConfiguration, sessionManager, hooks) - bufferApiCalls.drain(startLogsResult) + bufferApiCalls.subscribe((callback) => callback(startLogsResult)) + bufferApiCalls.unbuffer() } return { @@ -82,7 +91,6 @@ export function createPreStartStrategy( // Expose the initial configuration regardless of initialization success. cachedInitConfiguration = initConfiguration - addTelemetryConfiguration(serializeLogsConfiguration(initConfiguration)) if (cachedConfiguration) { displayAlreadyInitializedError('DD_LOGS', initConfiguration) @@ -96,14 +104,27 @@ export function createPreStartStrategy( cachedConfiguration = configuration - // Instrument fetch to track network requests - // This is needed in case the consent is not granted and some customer - // library (Apollo Client) is storing uninstrumented fetch to be used later - // The subscrption is needed so that the instrumentation process is completed - initFetchObservable().subscribe(noop) - trackingConsentState.tryToInit(configuration.trackingConsent) - tryStartLogs() + + trackingConsentState.onGrantedOnce(() => { + startTrackingConsentContext(hooks, trackingConsentState) + mockable(startTelemetry)(TelemetryService.LOGS, configuration, hooks) + const sessionManagerPromise = canUseEventBridge() + ? startSessionManagerStub() + : mockable(startSessionManager)(configuration, trackingConsentState) + + void sessionManagerPromise + .then((newSessionManager) => { + if (!newSessionManager) { + return + } + sessionManager = newSessionManager + startTelemetrySessionContext(hooks, sessionManager) + addTelemetryConfiguration(serializeLogsConfiguration(initConfiguration)) + tryStartLogs() + }) + .catch(monitorError) + }) }, get initConfiguration() { @@ -117,7 +138,7 @@ export function createPreStartStrategy( getInternalContext: noop as () => undefined, handleLog(message, statusType, handlingStack, context = getCommonContext(), date = timeStampNow()) { - bufferApiCalls.add((startLogsResult) => + bufferApiCalls.notify((startLogsResult) => startLogsResult.handleLog(message, statusType, handlingStack, context, date) ) }, diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index 12874091b5..d8d2e91a95 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -1,28 +1,15 @@ import type { BufferedData, Payload } from '@datadog/browser-core' -import { - ErrorSource, - display, - stopSessionManager, - getCookie, - SESSION_STORE_KEY, - createTrackingConsentState, - TrackingConsent, - setCookie, - STORAGE_POLL_DELAY, - ONE_MINUTE, - BufferedObservable, - FLUSH_DURATION_LIMIT, -} from '@datadog/browser-core' +import { ErrorSource, display, BufferedObservable, FLUSH_DURATION_LIMIT } from '@datadog/browser-core' import type { Clock, Request } from '@datadog/browser-core/test' import { interceptRequests, mockEndpointBuilder, mockEventBridge, - mockSyntheticsWorkerValues, registerCleanupTask, mockClock, - expireCookie, DEFAULT_FETCH_MOCK, + createSessionManagerMock, + MOCK_SESSION_ID, } from '@datadog/browser-core/test' import type { LogsConfiguration } from '../domain/configuration' @@ -53,19 +40,17 @@ const COMMON_CONTEXT = { } const DEFAULT_PAYLOAD = {} as Payload -function startLogsWithDefaults( - { configuration }: { configuration?: Partial } = {}, - trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED) -) { +function startLogsWithDefaults({ configuration }: { configuration?: Partial } = {}) { const endpointBuilder = mockEndpointBuilder('https://localhost/v1/input/log') + const sessionManager = createSessionManagerMock() const { handleLog, stop, globalContext, accountContext, userContext } = startLogs( { ...validateAndBuildLogsConfiguration({ clientToken: 'xxx', service: 'service', telemetrySampleRate: 0 })!, logsEndpointBuilder: endpointBuilder, ...configuration, }, + sessionManager, () => COMMON_CONTEXT, - trackingConsentState, new BufferedObservable(100), createHooks() ) @@ -74,7 +59,7 @@ function startLogsWithDefaults( const logger = new Logger(handleLog) - return { handleLog, logger, endpointBuilder, globalContext, accountContext, userContext } + return { handleLog, logger, endpointBuilder, globalContext, accountContext, userContext, sessionManager } } describe('logs', () => { @@ -90,7 +75,6 @@ describe('logs', () => { afterEach(() => { delete window.DD_RUM - stopSessionManager() }) describe('request', () => { @@ -165,30 +149,6 @@ describe('logs', () => { }) }) - describe('sampling', () => { - it('should be applied when event bridge is present (rate 0)', () => { - const sendSpy = spyOn(mockEventBridge(), 'send') - - const { handleLog, logger } = startLogsWithDefaults({ - configuration: { sessionSampleRate: 0 }, - }) - handleLog(DEFAULT_MESSAGE, logger) - - expect(sendSpy).not.toHaveBeenCalled() - }) - - it('should be applied when event bridge is present (rate 100)', () => { - const sendSpy = spyOn(mockEventBridge(), 'send') - - const { handleLog, logger } = startLogsWithDefaults({ - configuration: { sessionSampleRate: 100 }, - }) - handleLog(DEFAULT_MESSAGE, logger) - - expect(sendSpy).toHaveBeenCalled() - }) - }) - it('should not print the log twice when console handler is enabled', () => { const consoleLogSpy = spyOn(console, 'log') const displayLogSpy = spyOn(display, 'log') @@ -203,36 +163,15 @@ describe('logs', () => { expect(displayLogSpy).not.toHaveBeenCalled() }) - describe('logs session creation', () => { - it('creates a session on normal conditions', () => { - startLogsWithDefaults() - expect(getCookie(SESSION_STORE_KEY)).toBeDefined() - }) - - it('does not create a session if event bridge is present', () => { - mockEventBridge() - startLogsWithDefaults() - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() - }) - - it('does not create a session if synthetics worker will inject RUM', () => { - mockSyntheticsWorkerValues({ injectsRum: true }) - startLogsWithDefaults() - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() - }) - }) - describe('session lifecycle', () => { it('sends logs without session id when the session expires ', async () => { - setCookie(SESSION_STORE_KEY, 'id=foo&logs=1', ONE_MINUTE) - const { handleLog, logger } = startLogsWithDefaults() + const { handleLog, logger, sessionManager } = startLogsWithDefaults() interceptor.withFetch(DEFAULT_FETCH_MOCK, DEFAULT_FETCH_MOCK) handleLog({ status: StatusType.info, message: 'message 1' }, logger) - expireCookie() - clock.tick(STORAGE_POLL_DELAY * 2) + sessionManager.expire() handleLog({ status: StatusType.info, message: 'message 2' }, logger) @@ -244,7 +183,7 @@ describe('logs', () => { expect(requests.length).toEqual(2) expect(firstRequest.message).toEqual('message 1') - expect(firstRequest.session_id).toEqual('foo') + expect(firstRequest.session_id).toEqual(MOCK_SESSION_ID) expect(secondRequest.message).toEqual('message 2') expect(secondRequest.session_id).toBeUndefined() @@ -305,30 +244,4 @@ describe('logs', () => { expect(firstRequest.view.url).toEqual('from-rum-context') }) }) - - describe('tracking consent', () => { - it('should not send logs after tracking consent is revoked', async () => { - const trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED) - const { handleLog, logger } = startLogsWithDefaults({}, trackingConsentState) - - // Log a message with consent granted - should be sent - handleLog({ status: StatusType.info, message: 'message before revocation' }, logger) - - clock.tick(FLUSH_DURATION_LIMIT) - await interceptor.waitForAllFetchCalls() - expect(requests.length).toEqual(1) - expect(getLoggedMessage(requests, 0).message).toBe('message before revocation') - - // Revoke consent - trackingConsentState.update(TrackingConsent.NOT_GRANTED) - - // Log another message - should not be sent - handleLog({ status: StatusType.info, message: 'message after revocation' }, logger) - - clock.tick(FLUSH_DURATION_LIMIT) - await interceptor.waitForAllFetchCalls() - // Should still only have the first request - expect(requests.length).toEqual(1) - }) - }) }) diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index e30aa27e22..77720a870c 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -1,15 +1,13 @@ -import type { TrackingConsentState, BufferedObservable, BufferedData } from '@datadog/browser-core' +import type { BufferedObservable, BufferedData, SessionManager } from '@datadog/browser-core' import { sendToExtension, createPageMayExitObservable, - willSyntheticsInjectRum, canUseEventBridge, startAccountContext, startGlobalContext, startUserContext, startTabContext, } from '@datadog/browser-core' -import { startLogsSessionManager, startLogsSessionManagerStub } from '../domain/logsSessionManager' import type { LogsConfiguration } from '../domain/configuration' import { startLogsAssembly } from '../domain/assembly' import { startConsoleCollection } from '../domain/console/consoleCollection' @@ -26,7 +24,6 @@ import type { CommonContext } from '../rawLogsEvent.types' import type { Hooks } from '../domain/hooks' import { startRUMInternalContext } from '../domain/contexts/rumInternalContext' import { startSessionContext } from '../domain/contexts/sessionContext' -import { startTrackingConsentContext } from '../domain/contexts/trackingConsentContext' const LOGS_STORAGE_KEY = 'logs' @@ -35,12 +32,8 @@ export type StartLogsResult = ReturnType export function startLogs( configuration: LogsConfiguration, + sessionManager: SessionManager, getCommonContext: () => CommonContext, - - // `startLogs` and its subcomponents assume tracking consent is granted initially and starts - // collecting logs unconditionally. As such, `startLogs` should be called with a - // `trackingConsentState` set to "granted". - trackingConsentState: TrackingConsentState, bufferedDataObservable: BufferedObservable, hooks: Hooks ) { @@ -52,24 +45,18 @@ export function startLogs( const reportError = startReportError(lifeCycle) const pageMayExitObservable = createPageMayExitObservable(configuration) - const session = - configuration.sessionStoreStrategyType && !canUseEventBridge() && !willSyntheticsInjectRum() - ? startLogsSessionManager(configuration, trackingConsentState) - : startLogsSessionManagerStub(configuration) - - startTrackingConsentContext(hooks, trackingConsentState) // Start user and account context first to allow overrides from global context - startSessionContext(hooks, configuration, session) + startSessionContext(hooks, configuration, sessionManager) const accountContext = startAccountContext(hooks, configuration, LOGS_STORAGE_KEY) - const userContext = startUserContext(hooks, configuration, session, LOGS_STORAGE_KEY) + const userContext = startUserContext(hooks, configuration, sessionManager, LOGS_STORAGE_KEY) const globalContext = startGlobalContext(hooks, configuration, LOGS_STORAGE_KEY, false) startRUMInternalContext(hooks) startTabContext(hooks) - startNetworkErrorCollection(configuration, lifeCycle) + startNetworkErrorCollection(configuration, lifeCycle, bufferedDataObservable) startRuntimeErrorCollection(configuration, lifeCycle, bufferedDataObservable) + startConsoleCollection(configuration, lifeCycle, bufferedDataObservable) bufferedDataObservable.unbuffer() - startConsoleCollection(configuration, lifeCycle) startReportCollection(configuration, lifeCycle) const { handleLog } = startLoggerCollection(lifeCycle) @@ -81,14 +68,14 @@ export function startLogs( lifeCycle, reportError, pageMayExitObservable, - session + sessionManager ) cleanupTasks.push(() => stopLogsBatch()) } else { startLogsBridge(lifeCycle) } - const internalContext = startInternalContext(session) + const internalContext = startInternalContext(sessionManager) return { handleLog, diff --git a/packages/logs/src/domain/configuration.spec.ts b/packages/logs/src/domain/configuration.spec.ts index f9d52412af..b4872220a3 100644 --- a/packages/logs/src/domain/configuration.spec.ts +++ b/packages/logs/src/domain/configuration.spec.ts @@ -57,41 +57,28 @@ describe('validateAndBuildLogsConfiguration', () => { }) describe('forwardConsoleLogs', () => { - it('contains "error" when forwardErrorsToLogs is enabled', () => { + it('does not contain "error" when forwardConsoleLogs is disabled and forwardErrorsToLogs is explicitly enabled', () => { expect( validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: true })! .forwardConsoleLogs - ).toEqual(['error']) + ).not.toContain('error') }) - it('contains "error" once when both forwardErrorsToLogs and forwardConsoleLogs are enabled', () => { + it('does not contain "error" when forwardConsoleLogs is disabled and forwardErrorsToLogs is omitted', () => { + expect(validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION })!.forwardConsoleLogs).not.toContain( + 'error' + ) + }) + + it('contains "error" when forwardConsoleLogs contains "error"', () => { expect( validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: ['error'], - forwardErrorsToLogs: true, })!.forwardConsoleLogs ).toEqual(['error']) }) }) - - describe('PCI compliant intake option', () => { - let warnSpy: jasmine.Spy - - beforeEach(() => { - warnSpy = spyOn(display, 'warn') - }) - it('should display warning with wrong PCI intake configuration', () => { - validateAndBuildLogsConfiguration({ - ...DEFAULT_INIT_CONFIGURATION, - site: 'us3.datadoghq.com', - usePciIntake: true, - }) - expect(warnSpy).toHaveBeenCalledOnceWith( - 'PCI compliance for Logs is only available for Datadog organizations in the US1 site. Default intake will be used.' - ) - }) - }) }) describe('validateAndBuildForwardOption', () => { @@ -137,7 +124,6 @@ describe('serializeLogsConfiguration', () => { forwardErrorsToLogs: true, forwardConsoleLogs: 'all', forwardReports: 'all', - usePciIntake: false, } type MapLogsInitConfigurationKey = Key extends keyof InitConfiguration @@ -155,7 +141,6 @@ describe('serializeLogsConfiguration', () => { forward_errors_to_logs: true, forward_console_logs: 'all', forward_reports: 'all', - use_pci_intake: false, }) }) }) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 976c6a0629..156a2a3bba 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -48,10 +48,13 @@ export interface LogsInitConfiguration extends InitConfiguration { beforeSend?: LogsBeforeSend | undefined /** - * Forward console.error logs, uncaught exceptions and network errors to Datadog. + * Forward uncaught exceptions and network errors to Datadog. + * + * To capture `console.error` calls, use {@link forwardConsoleLogs} with `"error"` (or `"all"`). * * @category Data Collection * @defaultValue true + * @see forwardConsoleLogs */ forwardErrorsToLogs?: boolean | undefined @@ -68,14 +71,6 @@ export interface LogsInitConfiguration extends InitConfiguration { * @category Data Collection */ forwardReports?: RawReportType[] | 'all' | undefined - - /** - * Use PCI-compliant intake. See [PCI DSS Compliance](https://docs.datadoghq.com/data_security/pci_compliance/?tab=logmanagement) for further information. - * - * @category Privacy - * @defaultValue false - */ - usePciIntake?: boolean } /** @@ -105,12 +100,6 @@ export function validateAndBuildLogsConfiguration( initConfiguration: LogsInitConfiguration, errorStack?: string ): LogsConfiguration | undefined { - if (initConfiguration.usePciIntake === true && initConfiguration.site && initConfiguration.site !== 'datadoghq.com') { - display.warn( - 'PCI compliance for Logs is only available for Datadog organizations in the US1 site. Default intake will be used.' - ) - } - const baseConfiguration = validateAndBuildConfiguration(initConfiguration, errorStack) const forwardConsoleLogs = validateAndBuildForwardOption( @@ -129,10 +118,6 @@ export function validateAndBuildLogsConfiguration( return } - if (initConfiguration.forwardErrorsToLogs && !forwardConsoleLogs.includes(ConsoleApiName.error)) { - forwardConsoleLogs.push(ConsoleApiName.error) - } - return { forwardErrorsToLogs: initConfiguration.forwardErrorsToLogs !== false, forwardConsoleLogs, @@ -166,7 +151,6 @@ export function serializeLogsConfiguration(configuration: LogsInitConfiguration) forward_errors_to_logs: configuration.forwardErrorsToLogs, forward_console_logs: configuration.forwardConsoleLogs, forward_reports: configuration.forwardReports, - use_pci_intake: configuration.usePciIntake, ...baseSerializedInitConfiguration, } satisfies RawTelemetryConfiguration } diff --git a/packages/logs/src/domain/console/consoleCollection.spec.ts b/packages/logs/src/domain/console/consoleCollection.spec.ts index dcc7c7892a..32ae4d04d1 100644 --- a/packages/logs/src/domain/console/consoleCollection.spec.ts +++ b/packages/logs/src/domain/console/consoleCollection.spec.ts @@ -1,5 +1,14 @@ -import type { Context, ErrorWithCause } from '@datadog/browser-core' -import { ErrorHandling, ErrorSource, noop, objectEntries } from '@datadog/browser-core' +import type { BufferedData, ConsoleLog, RawError } from '@datadog/browser-core' +import { + BufferedDataType, + ConsoleApiName, + ErrorHandling, + ErrorSource, + Observable, + clocksNow, + noop, + objectEntries, +} from '@datadog/browser-core' import type { RawConsoleLogsEvent } from '../../rawLogsEvent.types' import { validateAndBuildLogsConfiguration } from '../configuration' import type { RawLogsEventCollectedData } from '../lifeCycle' @@ -8,25 +17,38 @@ import { startConsoleCollection, LogStatusForApi } from './consoleCollection' describe('console collection', () => { const initConfiguration = { clientToken: 'xxx', service: 'service' } - let consoleSpies: { [key: string]: jasmine.Spy } let stopConsoleCollection: () => void let lifeCycle: LifeCycle let rawLogsEvents: Array> + let bufferedDataObservable: Observable + + function notifyConsole(log: ConsoleLog) { + bufferedDataObservable.notify({ type: BufferedDataType.CONSOLE, data: log }) + } + + function makeRawError(overrides: Partial = {}): RawError { + return { + startClocks: clocksNow(), + message: 'error message', + source: ErrorSource.CONSOLE, + handling: ErrorHandling.HANDLED, + type: undefined, + stack: undefined, + causes: undefined, + fingerprint: undefined, + context: undefined, + ...overrides, + } + } beforeEach(() => { rawLogsEvents = [] lifeCycle = new LifeCycle() + bufferedDataObservable = new Observable() lifeCycle.subscribe(LifeCycleEventType.RAW_LOG_COLLECTED, (rawLogsEvent) => rawLogsEvents.push(rawLogsEvent as RawLogsEventCollectedData) ) stopConsoleCollection = noop - consoleSpies = { - log: spyOn(console, 'log').and.callFake(() => true), - debug: spyOn(console, 'debug').and.callFake(() => true), - info: spyOn(console, 'info').and.callFake(() => true), - warn: spyOn(console, 'warn').and.callFake(() => true), - error: spyOn(console, 'error').and.callFake(() => true), - } }) afterEach(() => { @@ -37,11 +59,16 @@ describe('console collection', () => { it(`should collect ${status} logs from console.${api}`, () => { ;({ stop: stopConsoleCollection } = startConsoleCollection( validateAndBuildLogsConfiguration({ ...initConfiguration, forwardConsoleLogs: 'all' })!, - lifeCycle + lifeCycle, + bufferedDataObservable )) - /* eslint-disable-next-line no-console */ - console[api as keyof typeof LogStatusForApi]('foo', 'bar') + notifyConsole({ + api: api as ConsoleApiName, + message: 'foo bar', + handlingStack: 'at foo', + error: undefined, + } as ConsoleLog) expect(rawLogsEvents[0].rawLogsEvent).toEqual({ date: jasmine.any(Number), @@ -54,19 +81,39 @@ describe('console collection', () => { expect(rawLogsEvents[0].domainContext).toEqual({ handlingStack: jasmine.any(String), }) + }) + + it(`should not collect logs from console.${api} if not configured`, () => { + ;({ stop: stopConsoleCollection } = startConsoleCollection( + validateAndBuildLogsConfiguration({ ...initConfiguration, forwardConsoleLogs: [] })!, + lifeCycle, + bufferedDataObservable + )) + + notifyConsole({ + api: api as ConsoleApiName, + message: 'foo bar', + handlingStack: 'at foo', + error: undefined, + } as ConsoleLog) - expect(consoleSpies[api]).toHaveBeenCalled() + expect(rawLogsEvents.length).toBe(0) }) }) it('console error should have an error object defined', () => { ;({ stop: stopConsoleCollection } = startConsoleCollection( - validateAndBuildLogsConfiguration({ ...initConfiguration, forwardErrorsToLogs: true })!, - lifeCycle + validateAndBuildLogsConfiguration({ ...initConfiguration, forwardConsoleLogs: ['error'] })!, + lifeCycle, + bufferedDataObservable )) - /* eslint-disable-next-line no-console */ - console.error('foo', 'bar') + notifyConsole({ + api: ConsoleApiName.error, + message: 'foo bar', + handlingStack: '', + error: makeRawError(), + }) expect(rawLogsEvents[0].rawLogsEvent.error).toEqual({ stack: undefined, @@ -78,89 +125,22 @@ describe('console collection', () => { }) }) - it('should retrieve fingerprint from console error', () => { + it('should use error context as message context', () => { ;({ stop: stopConsoleCollection } = startConsoleCollection( - validateAndBuildLogsConfiguration({ ...initConfiguration, forwardErrorsToLogs: true })!, - lifeCycle + validateAndBuildLogsConfiguration({ ...initConfiguration, forwardConsoleLogs: ['error'] })!, + lifeCycle, + bufferedDataObservable )) - interface DatadogError extends Error { - dd_fingerprint?: string - } - const error = new Error('foo') - ;(error as DatadogError).dd_fingerprint = 'my-fingerprint' - - // eslint-disable-next-line no-console - console.error(error) - expect(rawLogsEvents[0].rawLogsEvent.error).toEqual({ - stack: jasmine.any(String), - fingerprint: 'my-fingerprint', - causes: undefined, - handling: ErrorHandling.HANDLED, - kind: 'Error', - message: undefined, + notifyConsole({ + api: ConsoleApiName.error, + message: 'Error: foo', + handlingStack: '', + error: makeRawError({ context: { foo: 'bar' } }), }) - }) - - it('should retrieve dd_context from console', () => { - ;({ stop: stopConsoleCollection } = startConsoleCollection( - validateAndBuildLogsConfiguration({ ...initConfiguration, forwardErrorsToLogs: true })!, - lifeCycle - )) - interface DatadogError extends Error { - dd_context?: Context - } - const error = new Error('foo') - ;(error as DatadogError).dd_context = { foo: 'bar' } - - // eslint-disable-next-line no-console - console.error(error) expect(rawLogsEvents[0].messageContext).toEqual({ foo: 'bar' }) }) - - it('should retrieve causes from console error', () => { - ;({ stop: stopConsoleCollection } = startConsoleCollection( - validateAndBuildLogsConfiguration({ ...initConfiguration, forwardErrorsToLogs: true })!, - lifeCycle - )) - const error = new Error('High level error') as ErrorWithCause - error.stack = 'Error: High level error' - - const nestedError = new Error('Mid level error') as ErrorWithCause - nestedError.stack = 'Error: Mid level error' - - const deepNestedError = new TypeError('Low level error') as ErrorWithCause - deepNestedError.stack = 'TypeError: Low level error' - - nestedError.cause = deepNestedError - error.cause = nestedError - - // eslint-disable-next-line no-console - console.error(error) - - expect(rawLogsEvents[0].rawLogsEvent.error).toEqual({ - stack: jasmine.any(String), - handling: ErrorHandling.HANDLED, - causes: [ - { - source: ErrorSource.CONSOLE, - type: 'Error', - stack: jasmine.any(String), - message: 'Mid level error', - }, - { - source: ErrorSource.CONSOLE, - type: 'TypeError', - stack: jasmine.any(String), - message: 'Low level error', - }, - ], - fingerprint: undefined, - kind: 'Error', - message: undefined, - }) - }) }) function whatever() { diff --git a/packages/logs/src/domain/console/consoleCollection.ts b/packages/logs/src/domain/console/consoleCollection.ts index 3eb2ccb25d..a1a792f363 100644 --- a/packages/logs/src/domain/console/consoleCollection.ts +++ b/packages/logs/src/domain/console/consoleCollection.ts @@ -1,5 +1,5 @@ -import type { Context, ClocksState, ConsoleLog } from '@datadog/browser-core' -import { timeStampNow, ConsoleApiName, ErrorSource, initConsoleObservable } from '@datadog/browser-core' +import type { Context, ClocksState, Observable, BufferedData } from '@datadog/browser-core' +import { BufferedDataType, timeStampNow, ConsoleApiName, ErrorSource } from '@datadog/browser-core' import type { LogsConfiguration } from '../configuration' import type { LifeCycle, RawLogsEventCollectedData } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' @@ -21,8 +21,16 @@ export const LogStatusForApi = { [ConsoleApiName.warn]: StatusType.warn, [ConsoleApiName.error]: StatusType.error, } -export function startConsoleCollection(configuration: LogsConfiguration, lifeCycle: LifeCycle) { - const consoleSubscription = initConsoleObservable(configuration.forwardConsoleLogs).subscribe((log: ConsoleLog) => { + +export function startConsoleCollection( + configuration: LogsConfiguration, + lifeCycle: LifeCycle, + bufferedDataObservable: Observable +) { + const subscription = bufferedDataObservable.subscribe(({ data: log, type }) => { + if (type !== BufferedDataType.CONSOLE || !configuration.forwardConsoleLogs.includes(log.api)) { + return + } const collectedData: RawLogsEventCollectedData = { rawLogsEvent: { date: timeStampNow(), @@ -40,9 +48,5 @@ export function startConsoleCollection(configuration: LogsConfiguration, lifeCyc lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, collectedData) }) - return { - stop: () => { - consoleSubscription.unsubscribe() - }, - } + return { stop: () => subscription.unsubscribe() } } diff --git a/packages/logs/src/domain/contexts/internalContext.spec.ts b/packages/logs/src/domain/contexts/internalContext.spec.ts index 936a237ffc..5080da7a31 100644 --- a/packages/logs/src/domain/contexts/internalContext.spec.ts +++ b/packages/logs/src/domain/contexts/internalContext.spec.ts @@ -1,14 +1,14 @@ -import { createLogsSessionManagerMock } from '../../../test/mockLogsSessionManager' +import { createSessionManagerMock } from '@datadog/browser-core/test' import { startInternalContext } from './internalContext' describe('internal context', () => { it('should return undefined if session is not tracked', () => { - const sessionManagerMock = createLogsSessionManagerMock().setNotTracked() + const sessionManagerMock = createSessionManagerMock().setNotTracked() expect(startInternalContext(sessionManagerMock).get()).toEqual(undefined) }) it('should return internal context corresponding to startTime', () => { - const sessionManagerMock = createLogsSessionManagerMock().setTracked() + const sessionManagerMock = createSessionManagerMock().setTracked() expect(startInternalContext(sessionManagerMock).get()).toEqual({ session_id: jasmine.any(String), }) diff --git a/packages/logs/src/domain/contexts/internalContext.ts b/packages/logs/src/domain/contexts/internalContext.ts index a60f18c7b0..ad3257a072 100644 --- a/packages/logs/src/domain/contexts/internalContext.ts +++ b/packages/logs/src/domain/contexts/internalContext.ts @@ -1,11 +1,10 @@ -import type { RelativeTime } from '@datadog/browser-core' -import type { LogsSessionManager } from '../logsSessionManager' +import type { RelativeTime, SessionManager } from '@datadog/browser-core' export interface InternalContext { session_id: string | undefined } -export function startInternalContext(sessionManager: LogsSessionManager) { +export function startInternalContext(sessionManager: SessionManager) { return { get: (startTime?: number): InternalContext | undefined => { const trackedSession = sessionManager.findTrackedSession(startTime as RelativeTime) diff --git a/packages/logs/src/domain/contexts/sessionContext.spec.ts b/packages/logs/src/domain/contexts/sessionContext.spec.ts index a5115fe3ac..98a40cfb17 100644 --- a/packages/logs/src/domain/contexts/sessionContext.spec.ts +++ b/packages/logs/src/domain/contexts/sessionContext.spec.ts @@ -1,20 +1,19 @@ -import type { RelativeTime } from '@datadog/browser-core' +import type { RelativeTime, SessionManager } from '@datadog/browser-core' import { DISCARDED, HookNames } from '@datadog/browser-core' -import type { LogsSessionManager } from '../logsSessionManager' +import { createSessionManagerMock } from '@datadog/browser-core/test' import type { DefaultLogsEventAttributes, Hooks } from '../hooks' import { createHooks } from '../hooks' import type { LogsConfiguration } from '../configuration' -import { createLogsSessionManagerMock } from '../../../test/mockLogsSessionManager' import { startSessionContext } from './sessionContext' describe('session context', () => { let hooks: Hooks - let sessionManager: LogsSessionManager + let sessionManager: SessionManager const configuration = { service: 'foo' } as LogsConfiguration beforeEach(() => { hooks = createHooks() - sessionManager = createLogsSessionManagerMock().setTracked() + sessionManager = createSessionManagerMock().setTracked() }) describe('assemble hook', () => { @@ -29,7 +28,7 @@ describe('session context', () => { }) it('should discard logs if session is not tracked', () => { - startSessionContext(hooks, configuration, createLogsSessionManagerMock().setNotTracked()) + startSessionContext(hooks, configuration, createSessionManagerMock().setNotTracked()) const defaultLogAttributes = hooks.triggerHook(HookNames.Assemble, { startTime: 0 as RelativeTime, @@ -39,7 +38,7 @@ describe('session context', () => { }) it('should set session id if session is active', () => { - startSessionContext(hooks, configuration, createLogsSessionManagerMock().setTracked()) + startSessionContext(hooks, configuration, createSessionManagerMock().setTracked()) const defaultLogAttributes = hooks.triggerHook(HookNames.Assemble, { startTime: 0 as RelativeTime, @@ -53,7 +52,9 @@ describe('session context', () => { }) it('should no set session id if session has expired', () => { - startSessionContext(hooks, configuration, createLogsSessionManagerMock().expire()) + const sessionManagerMock = createSessionManagerMock() + sessionManagerMock.expire() + startSessionContext(hooks, configuration, sessionManagerMock) const defaultLogAttributes = hooks.triggerHook(HookNames.Assemble, { startTime: 0 as RelativeTime, @@ -66,28 +67,4 @@ describe('session context', () => { }) }) }) - - describe('assemble telemetry hook', () => { - it('should set the session id', () => { - startSessionContext(hooks, configuration, createLogsSessionManagerMock()) - - const defaultRumEventAttributes = hooks.triggerHook(HookNames.AssembleTelemetry, { - startTime: 0 as RelativeTime, - }) - - expect(defaultRumEventAttributes).toEqual({ - session: { id: jasmine.any(String) }, - }) - }) - - it('should not set the session id if session is not tracked', () => { - startSessionContext(hooks, configuration, createLogsSessionManagerMock().setNotTracked()) - - const defaultRumEventAttributes = hooks.triggerHook(HookNames.AssembleTelemetry, { - startTime: 0 as RelativeTime, - }) - - expect(defaultRumEventAttributes).toBeUndefined() - }) - }) }) diff --git a/packages/logs/src/domain/contexts/sessionContext.ts b/packages/logs/src/domain/contexts/sessionContext.ts index 6f2bd875d8..31acd602c2 100644 --- a/packages/logs/src/domain/contexts/sessionContext.ts +++ b/packages/logs/src/domain/contexts/sessionContext.ts @@ -1,17 +1,15 @@ -import { DISCARDED, HookNames, SKIPPED } from '@datadog/browser-core' +import type { SessionManager } from '@datadog/browser-core' +import { DISCARDED, HookNames } from '@datadog/browser-core' import type { LogsConfiguration } from '../configuration' -import type { LogsSessionManager } from '../logsSessionManager' import type { Hooks } from '../hooks' -export function startSessionContext( - hooks: Hooks, - configuration: LogsConfiguration, - sessionManager: LogsSessionManager -) { +export function startSessionContext(hooks: Hooks, configuration: LogsConfiguration, sessionManager: SessionManager) { hooks.register(HookNames.Assemble, ({ startTime }) => { const session = sessionManager.findTrackedSession(startTime) - const isSessionTracked = sessionManager.findTrackedSession(startTime, { returnInactive: true }) + const isSessionTracked = sessionManager.findTrackedSession(startTime, { + returnInactive: true, + }) if (!isSessionTracked) { return DISCARDED @@ -23,16 +21,4 @@ export function startSessionContext( session: session ? { id: session.id } : undefined, } }) - - hooks.register(HookNames.AssembleTelemetry, ({ startTime }) => { - const session = sessionManager.findTrackedSession(startTime) - - if (!session || !session.id) { - return SKIPPED - } - - return { - session: { id: session.id }, - } - }) } diff --git a/packages/logs/src/domain/createErrorFieldFromRawError.spec.ts b/packages/logs/src/domain/createErrorFieldFromRawError.spec.ts index d7df73733a..39a8dfc1e6 100644 --- a/packages/logs/src/domain/createErrorFieldFromRawError.spec.ts +++ b/packages/logs/src/domain/createErrorFieldFromRawError.spec.ts @@ -15,7 +15,20 @@ describe('createErrorFieldFromRawError', () => { type: 'qux', message: 'quux', stack: 'quuz', - causes: [], + causes: [ + { + source: ErrorSource.CONSOLE, + type: 'Error', + stack: 'Error: Mid level error', + message: 'Mid level error', + }, + { + source: ErrorSource.CONSOLE, + type: 'TypeError', + stack: 'TypeError: Low level error', + message: 'Low level error', + }, + ], fingerprint: 'corge', csp: { disposition: 'enforce', @@ -30,7 +43,20 @@ describe('createErrorFieldFromRawError', () => { message: undefined, kind: 'qux', stack: 'quuz', - causes: [], + causes: [ + { + source: ErrorSource.CONSOLE, + type: 'Error', + stack: 'Error: Mid level error', + message: 'Mid level error', + }, + { + source: ErrorSource.CONSOLE, + type: 'TypeError', + stack: 'TypeError: Low level error', + message: 'Low level error', + }, + ], fingerprint: 'corge', handling: ErrorHandling.HANDLED, }) diff --git a/packages/logs/src/domain/logger.spec.ts b/packages/logs/src/domain/logger.spec.ts index 9ffcfbc7e7..a316f1d0a9 100644 --- a/packages/logs/src/domain/logger.spec.ts +++ b/packages/logs/src/domain/logger.spec.ts @@ -1,10 +1,5 @@ import type { ErrorWithCause } from '@datadog/browser-core' -import { - display, - ErrorHandling, - NO_ERROR_STACK_PRESENT_MESSAGE, - supportUnicodePropertyEscapes, -} from '@datadog/browser-core' +import { display, ErrorHandling, NO_ERROR_STACK_PRESENT_MESSAGE } from '@datadog/browser-core' import type { LogsMessage } from './logger' import { HandlerType, Logger, STATUSES } from './logger' import { StatusType } from './logger/isAuthorized' @@ -195,11 +190,9 @@ describe('Logger', () => { describe('tags', () => { let displaySpy: jasmine.Spy function expectWarning() { - if (supportUnicodePropertyEscapes()) { - expect(displaySpy).toHaveBeenCalledOnceWith( - jasmine.stringMatching("Tag .* doesn't meet tag requirements and will be sanitized") - ) - } + expect(displaySpy).toHaveBeenCalledOnceWith( + jasmine.stringMatching("Tag .* doesn't meet tag requirements and will be sanitized") + ) } beforeEach(() => { diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts b/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts index df6ed73b88..0964c1b7d2 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts @@ -1,6 +1,6 @@ -import { ErrorSource } from '@datadog/browser-core' -import type { MockFetch, MockFetchManager } from '@datadog/browser-core/test' -import { SPEC_ENDPOINTS, mockFetch, registerCleanupTask } from '@datadog/browser-core/test' +import type { BufferedData, FetchResolveContext } from '@datadog/browser-core' +import { BufferedDataType, ErrorSource, Observable, clocksNow } from '@datadog/browser-core' +import { SPEC_ENDPOINTS, registerCleanupTask } from '@datadog/browser-core/test' import type { RawNetworkLogsEvent } from '../../rawLogsEvent.types' import type { LogsConfiguration } from '../configuration' import type { RawLogsEventCollectedData } from '../lifeCycle' @@ -14,137 +14,124 @@ const CONFIGURATION = { ...SPEC_ENDPOINTS, } as LogsConfiguration +const FAKE_URL = 'http://fake.com/' + +const DEFAULT_FETCH_CONTEXT: FetchResolveContext = { + state: 'resolve', + method: 'GET', + url: FAKE_URL, + status: 503, + responseBody: 'Server error', + isAborted: false, + handlingStack: '', + startClocks: clocksNow(), + input: FAKE_URL, + isAbortedOnStart: false, +} + describe('network error collection', () => { - let fetch: MockFetch - let mockFetchManager: MockFetchManager let lifeCycle: LifeCycle let rawLogsEvents: Array> - const FAKE_URL = 'http://fake.com/' - const DEFAULT_REQUEST = { - duration: 10, - method: 'GET', - responseText: 'Server error', - startTime: 0, - status: 503, - url: FAKE_URL, - } + let bufferedDataObservable: Observable function startCollection(forwardErrorsToLogs = true) { - mockFetchManager = mockFetch() - const { stop } = startNetworkErrorCollection({ ...CONFIGURATION, forwardErrorsToLogs }, lifeCycle) + const { stop } = startNetworkErrorCollection( + { ...CONFIGURATION, forwardErrorsToLogs }, + lifeCycle, + bufferedDataObservable + ) registerCleanupTask(() => { stop() }) - fetch = window.fetch as MockFetch + } + + function notifyFetch(context: Partial = {}) { + bufferedDataObservable.notify({ + type: BufferedDataType.FETCH, + data: { ...DEFAULT_FETCH_CONTEXT, ...context }, + }) } beforeEach(() => { rawLogsEvents = [] lifeCycle = new LifeCycle() + bufferedDataObservable = new Observable() lifeCycle.subscribe(LifeCycleEventType.RAW_LOG_COLLECTED, (rawLogsEvent) => rawLogsEvents.push(rawLogsEvent as RawLogsEventCollectedData) ) }) - it('should track server error', (done) => { + it('should track server error', () => { startCollection() - fetch(FAKE_URL).resolveWith(DEFAULT_REQUEST) - - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents[0].rawLogsEvent).toEqual({ - message: 'Fetch error GET http://fake.com/', - date: jasmine.any(Number), - status: StatusType.error, - origin: ErrorSource.NETWORK, - error: { - stack: 'Server error', - handling: undefined, - }, - http: { - method: 'GET', - status_code: 503, - url: 'http://fake.com/', - }, - }) - done() + notifyFetch() + + expect(rawLogsEvents[0].rawLogsEvent).toEqual({ + message: 'Fetch error GET http://fake.com/', + date: jasmine.any(Number), + status: StatusType.error, + origin: ErrorSource.NETWORK, + error: { + stack: 'Server error', + handling: undefined, + }, + http: { + method: 'GET', + status_code: 503, + url: 'http://fake.com/', + }, }) }) - it('should not track network error when forwardErrorsToLogs is false', (done) => { + it('should not track network error when forwardErrorsToLogs is false', () => { startCollection(false) - fetch(FAKE_URL).resolveWith(DEFAULT_REQUEST) + notifyFetch() - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(0) - done() - }) + expect(rawLogsEvents.length).toEqual(0) }) - it('should not track intake error', (done) => { + it('should not track intake error', () => { startCollection() - fetch( - 'https://logs-intake.com/v1/input/send?ddsource=browser&dd-api-key=xxxx&dd-request-id=1234567890' - ).resolveWith(DEFAULT_REQUEST) - - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(0) - done() + notifyFetch({ + url: 'https://logs-intake.com/v1/input/send?ddsource=browser&dd-api-key=xxxx&dd-request-id=1234567890', }) + + expect(rawLogsEvents.length).toEqual(0) }) - it('should track aborted requests', (done) => { + it('should not track aborted requests', () => { startCollection() - fetch(FAKE_URL).abort() - - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(1) - expect(rawLogsEvents[0].domainContext).toEqual({ - isAborted: true, - handlingStack: jasmine.any(String), - }) - done() - }) + notifyFetch({ isAborted: true, status: 0 }) + + expect(rawLogsEvents.length).toEqual(0) }) - it('should track refused request', (done) => { + it('should track refused request', () => { startCollection() - fetch(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 0 }) + notifyFetch({ status: 0 }) - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(1) - done() - }) + expect(rawLogsEvents.length).toEqual(1) }) - it('should not track client error', (done) => { + it('should not track client error', () => { startCollection() - fetch(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 400 }) + notifyFetch({ status: 400 }) - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(0) - done() - }) + expect(rawLogsEvents.length).toEqual(0) }) - it('should not track successful request', (done) => { + it('should not track successful request', () => { startCollection() - fetch(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 200 }) + notifyFetch({ status: 200 }) - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(0) - done() - }) + expect(rawLogsEvents.length).toEqual(0) }) - it('uses a fallback when the response text is empty', (done) => { + it('uses a fallback when the response text is empty', () => { startCollection() - fetch(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: '' }) + notifyFetch({ responseBody: '' }) - mockFetchManager.whenAllComplete(() => { - expect(rawLogsEvents.length).toEqual(1) - expect(rawLogsEvents[0].rawLogsEvent.error.stack).toEqual('Failed to load') - done() - }) + expect(rawLogsEvents.length).toEqual(1) + expect(rawLogsEvents[0].rawLogsEvent.error.stack).toEqual('Failed to load') }) describe('response body handling', () => { @@ -152,74 +139,36 @@ describe('network error collection', () => { startCollection() }) - it('should use responseBody for fetch server errors', (done) => { + it('should use responseBody for fetch server errors', () => { const responseBody = 'Internal Server Error Details' - fetch('http://fake-url/').resolveWith({ - status: 500, - responseText: responseBody, - }) - - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack).toBe(responseBody) - done() - }) - }) - - it('should use error stack trace for fetch rejections', (done) => { - fetch('http://fake-url/').rejectWith(new Error('Network failure')) + notifyFetch({ url: 'http://fake-url/', status: 500, responseBody }) - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack).toContain('Error: Network failure') - done() - }) + const log = rawLogsEvents[0].rawLogsEvent + expect(log.error.stack).toBe(responseBody) }) - it('should truncate responseBody according to limit', (done) => { - const longResponse = 'a'.repeat(100) + it('should use error stack trace for fetch rejections', () => { + const error = new Error('Network failure') + notifyFetch({ url: 'http://fake-url/', status: 0, error, responseBody: undefined }) - fetch('http://fake-url/').resolveWith({ - status: 500, - responseText: longResponse, - }) - - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack?.length).toBe(67) // 64 chars + '...' - expect(log.error.stack).toMatch(/^a{64}\.\.\.$/) - done() - }) + const log = rawLogsEvents[0].rawLogsEvent + expect(log.error.stack).toContain('Error: Network failure') }) - it('should use fallback message when no responseBody available', (done) => { - fetch('http://fake-url/').resolveWith({ status: 500 }) - - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack).toBe('Failed to load') - done() - }) - }) - - it('should use fallback message when response body is already used', (done) => { - fetch('http://fake-url/').resolveWith({ status: 500, bodyUsed: true }) + it('should truncate responseBody according to limit', () => { + const longResponse = 'a'.repeat(100) + notifyFetch({ url: 'http://fake-url/', status: 500, responseBody: longResponse }) - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack).toBe('Failed to load') - done() - }) + const log = rawLogsEvents[0].rawLogsEvent + expect(log.error.stack?.length).toBe(67) // 64 chars + '...' + expect(log.error.stack).toMatch(/^a{64}\.\.\.$/) }) - it('should use fallback message when response body is disturbed', (done) => { - fetch('http://fake-url/').resolveWith({ status: 500, bodyDisturbed: true }) + it('should use fallback message when no responseBody available', () => { + notifyFetch({ url: 'http://fake-url/', status: 500, responseBody: undefined }) - mockFetchManager.whenAllComplete(() => { - const log = rawLogsEvents[0].rawLogsEvent - expect(log.error.stack).toBe('Failed to load') - done() - }) + const log = rawLogsEvents[0].rawLogsEvent + expect(log.error.stack).toBe('Failed to load') }) }) }) diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.ts b/packages/logs/src/domain/networkError/networkErrorCollection.ts index 3f7a8ac4d1..b2f2145144 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.ts @@ -1,9 +1,7 @@ -import type { FetchResolveContext, XhrCompleteContext } from '@datadog/browser-core' +import type { FetchResolveContext, XhrCompleteContext, Observable, BufferedData } from '@datadog/browser-core' import { - isWorkerEnvironment, - Observable, + BufferedDataType, ErrorSource, - initXhrObservable, RequestType, initFetchObservable, computeStackTrace, @@ -20,30 +18,30 @@ import type { LogsEventDomainContext } from '../../domainContext.types' import { LifeCycleEventType } from '../lifeCycle' import { StatusType } from '../logger/isAuthorized' -export function startNetworkErrorCollection(configuration: LogsConfiguration, lifeCycle: LifeCycle) { +export function startNetworkErrorCollection( + configuration: LogsConfiguration, + lifeCycle: LifeCycle, + bufferedDataObservable: Observable +) { if (!configuration.forwardErrorsToLogs) { return { stop: noop } } - // XHR is not available in web workers, so we use an empty observable that never emits - const xhrSubscription = ( - isWorkerEnvironment ? new Observable() : initXhrObservable(configuration) - ).subscribe((context) => { - if (context.state === 'complete') { - handleResponse(RequestType.XHR, context) - } + // Register responseBodyAction getter (no subscription needed) + initFetchObservable({ + responseBodyAction: (context) => (isNetworkError(context) ? ResponseBodyAction.COLLECT : ResponseBodyAction.IGNORE), }) - const fetchSubscription = initFetchObservable({ - responseBodyAction: (context) => (isNetworkError(context) ? ResponseBodyAction.COLLECT : ResponseBodyAction.IGNORE), - }).subscribe((context) => { - if (context.state === 'resolve') { - handleResponse(RequestType.FETCH, context) + const subscription = bufferedDataObservable.subscribe(({ data, type }) => { + if (type === BufferedDataType.FETCH && data.state === 'resolve') { + handleResponse(RequestType.FETCH, data) + } else if (type === BufferedDataType.XHR && data.state === 'complete') { + handleResponse(RequestType.XHR, data) } }) function isNetworkError(request: XhrCompleteContext | FetchResolveContext) { - return !isIntakeUrl(request.url) && (isRejected(request) || isServerError(request.status)) + return !isIntakeUrl(request.url) && !request.isAborted && (isRejected(request) || isServerError(request.status)) } function handleResponse(type: RequestType, request: XhrCompleteContext | FetchResolveContext) { @@ -57,7 +55,6 @@ export function startNetworkErrorCollection(configuration: LogsConfiguration, li : request.responseBody || 'Failed to load' const domainContext: LogsEventDomainContext = { - isAborted: request.isAborted, handlingStack: request.handlingStack, } @@ -82,12 +79,7 @@ export function startNetworkErrorCollection(configuration: LogsConfiguration, li }) } - return { - stop: () => { - xhrSubscription.unsubscribe() - fetchSubscription.unsubscribe() - }, - } + return { stop: () => subscription.unsubscribe() } } function isRejected(request: { status: number; responseType?: string }) { diff --git a/packages/logs/src/domain/runtimeError/runtimeErrorCollection.spec.ts b/packages/logs/src/domain/runtimeError/runtimeErrorCollection.spec.ts index 80dceee947..57c29e23df 100644 --- a/packages/logs/src/domain/runtimeError/runtimeErrorCollection.spec.ts +++ b/packages/logs/src/domain/runtimeError/runtimeErrorCollection.spec.ts @@ -57,7 +57,7 @@ describe('runtime error collection', () => { bufferedDataObservable.notify({ type: BufferedDataType.RUNTIME_ERROR, - error: RAW_ERROR, + data: RAW_ERROR, }) expect(rawLogsEvents[0].rawLogsEvent).toEqual({ @@ -81,7 +81,7 @@ describe('runtime error collection', () => { bufferedDataObservable.notify({ type: BufferedDataType.RUNTIME_ERROR, - error: { + data: { ...RAW_ERROR, message: 'High level error', causes: [ @@ -137,7 +137,7 @@ describe('runtime error collection', () => { bufferedDataObservable.notify({ type: BufferedDataType.RUNTIME_ERROR, - error: RAW_ERROR, + data: RAW_ERROR, }) expect(rawLogsEvents.length).toEqual(0) @@ -148,7 +148,7 @@ describe('runtime error collection', () => { bufferedDataObservable.notify({ type: BufferedDataType.RUNTIME_ERROR, - error: { + data: { ...RAW_ERROR, context: { foo: 'bar' }, }, diff --git a/packages/logs/src/domain/runtimeError/runtimeErrorCollection.ts b/packages/logs/src/domain/runtimeError/runtimeErrorCollection.ts index b0fe777966..2bb2f0f38f 100644 --- a/packages/logs/src/domain/runtimeError/runtimeErrorCollection.ts +++ b/packages/logs/src/domain/runtimeError/runtimeErrorCollection.ts @@ -24,7 +24,7 @@ export function startRuntimeErrorCollection( const rawErrorSubscription = bufferedDataObservable.subscribe((bufferedData) => { if (bufferedData.type === BufferedDataType.RUNTIME_ERROR) { - const error = bufferedData.error + const error = bufferedData.data lifeCycle.notify(LifeCycleEventType.RAW_LOG_COLLECTED, { rawLogsEvent: { message: error.message, diff --git a/packages/logs/src/domainContext.types.ts b/packages/logs/src/domainContext.types.ts index 80af93288a..2e9dad54b3 100644 --- a/packages/logs/src/domainContext.types.ts +++ b/packages/logs/src/domainContext.types.ts @@ -9,7 +9,6 @@ export type LogsEventDomainContext = T extends type : undefined export interface NetworkLogsEventDomainContext { - isAborted: boolean handlingStack?: string } diff --git a/packages/logs/src/transport/startLogsBatch.ts b/packages/logs/src/transport/startLogsBatch.ts index c0674a8521..9a9dbb0756 100644 --- a/packages/logs/src/transport/startLogsBatch.ts +++ b/packages/logs/src/transport/startLogsBatch.ts @@ -1,17 +1,16 @@ -import type { Context, Observable, PageMayExitEvent, RawError } from '@datadog/browser-core' +import type { Context, Observable, PageMayExitEvent, RawError, SessionManager } from '@datadog/browser-core' import { createBatch, createFlushController, createHttpRequest, createIdentityEncoder } from '@datadog/browser-core' import type { LogsConfiguration } from '../domain/configuration' import type { LifeCycle } from '../domain/lifeCycle' import { LifeCycleEventType } from '../domain/lifeCycle' import type { LogsEvent } from '../logsEvent.types' -import type { LogsSessionManager } from '../domain/logsSessionManager' export function startLogsBatch( configuration: LogsConfiguration, lifeCycle: LifeCycle, reportError: (error: RawError) => void, pageMayExitObservable: Observable, - session: LogsSessionManager + sessionManager: SessionManager ) { const endpoints = [configuration.logsEndpointBuilder] if (configuration.replica) { @@ -23,7 +22,7 @@ export function startLogsBatch( request: createHttpRequest(endpoints, reportError), flushController: createFlushController({ pageMayExitObservable, - sessionExpireObservable: session.expireObservable, + sessionExpireObservable: sessionManager.expireObservable, }), })