From 413e7d1790793956aeb1b3dbd33e595d1bf7cbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 16 Jan 2024 15:00:53 +0100 Subject: [PATCH 1/9] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUM-2445]=20factorize?= =?UTF-8?q?=20displayAlreadyInitializedError?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../displayAlreadyInitializedError.spec.ts | 18 ++++++++++++++++++ .../src/boot/displayAlreadyInitializedError.ts | 8 ++++++++ packages/core/src/index.ts | 1 + packages/logs/src/boot/logsPublicApi.ts | 5 ++--- packages/rum-core/src/boot/rumPublicApi.ts | 5 ++--- 5 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 packages/core/src/boot/displayAlreadyInitializedError.spec.ts create mode 100644 packages/core/src/boot/displayAlreadyInitializedError.ts diff --git a/packages/core/src/boot/displayAlreadyInitializedError.spec.ts b/packages/core/src/boot/displayAlreadyInitializedError.spec.ts new file mode 100644 index 0000000000..c2a884cf4e --- /dev/null +++ b/packages/core/src/boot/displayAlreadyInitializedError.spec.ts @@ -0,0 +1,18 @@ +import type { InitConfiguration } from '../domain/configuration' +import { display } from '../tools/display' +import { displayAlreadyInitializedError } from './displayAlreadyInitializedError' + +describe('displayAlreadyInitializedError', () => { + it('should display an error', () => { + const displayErrorSpy = spyOn(display, 'error') + displayAlreadyInitializedError('DD_RUM', {} as InitConfiguration) + expect(displayErrorSpy).toHaveBeenCalledTimes(1) + expect(displayErrorSpy).toHaveBeenCalledWith('DD_RUM is already initialized.') + }) + + it('should not display an error if the "silentMultipleInit" option is used', () => { + const displayErrorSpy = spyOn(display, 'error') + displayAlreadyInitializedError('DD_RUM', { silentMultipleInit: true } as InitConfiguration) + expect(displayErrorSpy).not.toHaveBeenCalled() + }) +}) diff --git a/packages/core/src/boot/displayAlreadyInitializedError.ts b/packages/core/src/boot/displayAlreadyInitializedError.ts new file mode 100644 index 0000000000..cff7d6a286 --- /dev/null +++ b/packages/core/src/boot/displayAlreadyInitializedError.ts @@ -0,0 +1,8 @@ +import type { InitConfiguration } from '../domain/configuration' +import { display } from '../tools/display' + +export function displayAlreadyInitializedError(sdkName: 'DD_RUM' | 'DD_LOGS', initConfiguration: InitConfiguration) { + if (!initConfiguration.silentMultipleInit) { + display.error(`${sdkName} is already initialized.`) + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fc9af50cdb..b318d99b5e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -20,6 +20,7 @@ export { export { trackRuntimeError } from './domain/error/trackRuntimeError' export { computeStackTrace, StackTrace } from './domain/error/computeStackTrace' export { defineGlobal, makePublicApi } from './boot/init' +export { displayAlreadyInitializedError } from './boot/displayAlreadyInitializedError' export { initReportObservable, RawReport, RawReportType } from './domain/report/reportObservable' export { startTelemetry, diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index cae28923be..4854948883 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -15,6 +15,7 @@ import { sanitize, createCustomerDataTrackerManager, storeContextManager, + displayAlreadyInitializedError, } from '@datadog/browser-core' import type { LogsInitConfiguration } from '../domain/configuration' import { validateAndBuildLogsConfiguration } from '../domain/configuration' @@ -163,9 +164,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { function canInitLogs(initConfiguration: LogsInitConfiguration) { if (isAlreadyInitialized) { - if (!initConfiguration.silentMultipleInit) { - display.error('DD_LOGS is already initialized.') - } + displayAlreadyInitializedError('DD_LOGS', initConfiguration) return false } return true diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 04fba460fa..8049a3b53b 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -31,6 +31,7 @@ import { CustomerDataCompressionStatus, createCustomerDataTrackerManager, storeContextManager, + displayAlreadyInitializedError, } from '@datadog/browser-core' import type { LifeCycle } from '../domain/lifeCycle' import type { ViewContexts } from '../domain/contexts/viewContexts' @@ -333,9 +334,7 @@ export function makeRumPublicApi( function canInitRum(initConfiguration: RumInitConfiguration) { if (isAlreadyInitialized) { - if (!initConfiguration.silentMultipleInit) { - display.error('DD_RUM is already initialized.') - } + displayAlreadyInitializedError('DD_RUM', initConfiguration) return false } return true From 5beefd745016d58819f88ffd855001365a40f34d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 10 Jan 2024 10:25:02 +0100 Subject: [PATCH 2/9] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUM-2445]=20split=20r?= =?UTF-8?q?umPublicApi?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/tools/boundedBuffer.ts | 10 +- packages/rum-core/src/boot/preStartRum.ts | 169 +++++++++++++ .../rum-core/src/boot/rumPublicApi.spec.ts | 3 +- packages/rum-core/src/boot/rumPublicApi.ts | 238 +++++------------- packages/rum-core/src/boot/startRum.ts | 3 + packages/rum-core/src/index.ts | 3 +- 6 files changed, 248 insertions(+), 178 deletions(-) create mode 100644 packages/rum-core/src/boot/preStartRum.ts diff --git a/packages/core/src/tools/boundedBuffer.ts b/packages/core/src/tools/boundedBuffer.ts index 60a6212a72..9e10f605d1 100644 --- a/packages/core/src/tools/boundedBuffer.ts +++ b/packages/core/src/tools/boundedBuffer.ts @@ -1,17 +1,17 @@ const BUFFER_LIMIT = 500 -export class BoundedBuffer { - private buffer: Array<() => void> = [] +export class BoundedBuffer { + private buffer: Array<(arg: T) => void> = [] - add(callback: () => void) { + add(callback: (arg: T) => void) { const length = this.buffer.push(callback) if (length > BUFFER_LIMIT) { this.buffer.splice(0, 1) } } - drain() { - this.buffer.forEach((callback) => callback()) + drain(arg: T) { + this.buffer.forEach((callback) => callback(arg)) this.buffer.length = 0 } } diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts new file mode 100644 index 0000000000..375a50c4d3 --- /dev/null +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -0,0 +1,169 @@ +import { + BoundedBuffer, + display, + type DeflateWorker, + canUseEventBridge, + displayAlreadyInitializedError, + willSyntheticsInjectRum, + noop, + timeStampNow, + clocksNow, + assign, +} from '@datadog/browser-core' +import { + validateAndBuildRumConfiguration, + type RumConfiguration, + type RumInitConfiguration, +} from '../domain/configuration' +import type { CommonContext } from '../domain/contexts/commonContext' +import type { ViewOptions } from '../domain/view/trackViews' +import type { RumPublicApiOptions, Strategy } from './rumPublicApi' +import type { StartRumResult } from './startRum' + +export function createPreStartStrategy( + { ignoreInitIfSyntheticsWillInjectRum, startDeflateWorker }: RumPublicApiOptions, + getCommonContext: () => CommonContext, + doStartRum: ( + initConfiguration: RumInitConfiguration, + configuration: RumConfiguration, + deflateWorker: DeflateWorker | undefined, + initialViewOptions?: ViewOptions + ) => StartRumResult +): Strategy { + const bufferApiCalls = new BoundedBuffer() + let firstStartView: { options: ViewOptions | undefined; ignore(): void } | undefined + let deflateWorker: DeflateWorker | undefined + + let cachedInitConfiguration: RumInitConfiguration | undefined + let cachedConfiguration: RumConfiguration | undefined + + function tryStartRum() { + if ( + !cachedInitConfiguration || + !cachedConfiguration || + (cachedConfiguration.trackViewsManually && !firstStartView) + ) { + return + } + + let initialViewOptions: ViewOptions | undefined + + if (cachedConfiguration.trackViewsManually) { + firstStartView!.ignore() + initialViewOptions = firstStartView!.options + } + + const startRumResult = doStartRum(cachedInitConfiguration, cachedConfiguration, deflateWorker, initialViewOptions) + + bufferApiCalls.drain(startRumResult) + } + + return { + init(initConfiguration) { + if (!initConfiguration) { + display.error('Missing configuration') + return + } + + const eventBridgeAvailable = canUseEventBridge() + if (eventBridgeAvailable) { + initConfiguration = overrideInitConfigurationForBridge(initConfiguration) + } + + // Expose the initial configuration regardless of initialization success. + cachedInitConfiguration = initConfiguration + + if (cachedConfiguration) { + displayAlreadyInitializedError('DD_RUM', initConfiguration) + return + } + + // If we are in a Synthetics test configured to automatically inject a RUM instance, we want to + // completely discard the customer application RUM instance by ignoring their init() call. But, + // we should not ignore the init() call from the Synthetics-injected RUM instance, so the + // internal `ignoreInitIfSyntheticsWillInjectRum` option is here to bypass this condition. + if (ignoreInitIfSyntheticsWillInjectRum && willSyntheticsInjectRum()) { + return + } + + const configuration = validateAndBuildRumConfiguration(initConfiguration) + if (!configuration) { + return + } + + if (!eventBridgeAvailable && !configuration.sessionStoreStrategyType) { + display.warn('No storage available for session. We will not send any data.') + return + } + + if (configuration.compressIntakeRequests && !eventBridgeAvailable && startDeflateWorker) { + deflateWorker = startDeflateWorker( + configuration, + 'Datadog RUM', + // Worker initialization can fail asynchronously, especially in Firefox where even CSP + // issues are reported asynchronously. For now, the SDK will continue its execution even if + // data won't be sent to Datadog. We could improve this behavior in the future. + noop + ) + if (!deflateWorker) { + // `startDeflateWorker` should have logged an error message explaining the issue + return + } + } + + cachedConfiguration = configuration + tryStartRum() + }, + + get initConfiguration() { + return cachedInitConfiguration + }, + + getInternalContext: noop as () => undefined, + + stopSession: noop, + + addTiming(name, time = timeStampNow()) { + bufferApiCalls.add((startRumResult) => startRumResult.addTiming(name, time)) + }, + + startView(options, startClocks = clocksNow()) { + let ignore = false + if (!firstStartView) { + firstStartView = { + options, + ignore() { + ignore = true + }, + } + tryStartRum() + } + + bufferApiCalls.add((startRumResult) => { + if (!ignore) { + startRumResult.startView(options, startClocks) + } + }) + }, + + addAction(action, commonContext = getCommonContext()) { + bufferApiCalls.add((startRumResult) => startRumResult.addAction(action, commonContext)) + }, + + addError(providedError, commonContext = getCommonContext()) { + bufferApiCalls.add((startRumResult) => startRumResult.addError(providedError, commonContext)) + }, + + addFeatureFlagEvaluation(key, value) { + bufferApiCalls.add((startRumResult) => startRumResult.addFeatureFlagEvaluation(key, value)) + }, + } +} + +function overrideInitConfigurationForBridge(initConfiguration: RumInitConfiguration): RumInitConfiguration { + return assign({}, initConfiguration, { + applicationId: '00000000-aaaa-0000-aaaa-000000000000', + clientToken: 'empty', + sessionSampleRate: 100, + }) +} diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index 95316eb56a..cfdefbbf6e 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -29,8 +29,9 @@ import { setup, noopRecorderApi } from '../../test' import type { HybridInitConfiguration, RumInitConfiguration } from '../domain/configuration' import { ActionType } from '../rawRumEvent.types' import type { ViewOptions } from '../domain/view/trackViews' -import type { RumPublicApi, StartRum, RecorderApi } from './rumPublicApi' +import type { RumPublicApi, RecorderApi } from './rumPublicApi' import { makeRumPublicApi } from './rumPublicApi' +import type { StartRum } from './startRum' const noopStartRum = (): ReturnType => ({ addAction: () => undefined, diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 8049a3b53b..b0c4932cd2 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -1,6 +1,5 @@ import type { Context, - InitConfiguration, TimeStamp, RelativeTime, User, @@ -9,21 +8,15 @@ import type { DeflateEncoder, } from '@datadog/browser-core' import { - noop, CustomerDataType, - willSyntheticsInjectRum, assign, - BoundedBuffer, createContextManager, deepClone, makePublicApi, monitor, clocksNow, - timeStampNow, - display, callMonitored, createHandlingStack, - canUseEventBridge, checkUser, sanitizeUser, sanitize, @@ -39,17 +32,13 @@ import type { RumSessionManager } from '../domain/rumSessionManager' import type { ReplayStats } from '../rawRumEvent.types' import { ActionType } from '../rawRumEvent.types' import type { RumConfiguration, RumInitConfiguration } from '../domain/configuration' -import { validateAndBuildRumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/view/trackViews' import { buildCommonContext } from '../domain/contexts/commonContext' -import type { startRum } from './startRum' +import { createPreStartStrategy } from './preStartRum' +import type { StartRum, StartRumResult } from './startRum' export type RumPublicApi = ReturnType -export type StartRum = typeof startRum - -type StartRumResult = ReturnType - export interface RecorderApi { start: () => void stop: () => void @@ -64,7 +53,8 @@ export interface RecorderApi { getReplayStats: (viewId: string) => ReplayStats | undefined getSessionReplayLink: () => string | undefined } -interface RumPublicApiOptions { + +export interface RumPublicApiOptions { ignoreInitIfSyntheticsWillInjectRum?: boolean startDeflateWorker?: ( configuration: RumConfiguration, @@ -80,13 +70,19 @@ interface RumPublicApiOptions { const RUM_STORAGE_KEY = 'rum' -export function makeRumPublicApi( - startRumImpl: StartRum, - recorderApi: RecorderApi, - { ignoreInitIfSyntheticsWillInjectRum = true, startDeflateWorker, createDeflateEncoder }: RumPublicApiOptions = {} -) { - let isAlreadyInitialized = false +export interface Strategy { + init: (initConfiguration: RumInitConfiguration) => void + initConfiguration: RumInitConfiguration | undefined + getInternalContext: StartRumResult['getInternalContext'] + stopSession: StartRumResult['stopSession'] + addTiming: StartRumResult['addTiming'] + startView: StartRumResult['startView'] + addAction: StartRumResult['addAction'] + addError: StartRumResult['addError'] + addFeatureFlagEvaluation: StartRumResult['addFeatureFlagEvaluation'] +} +export function makeRumPublicApi(startRumImpl: StartRum, recorderApi: RecorderApi, options: RumPublicApiOptions = {}) { const customerDataTrackerManager = createCustomerDataTrackerManager(CustomerDataCompressionStatus.Unknown) const globalContextManager = createContextManager( customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext) @@ -97,153 +93,56 @@ export function makeRumPublicApi( return buildCommonContext(globalContextManager, userContextManager, recorderApi) } - let getInternalContextStrategy: StartRumResult['getInternalContext'] = () => undefined - let getInitConfigurationStrategy = (): InitConfiguration | undefined => undefined - let stopSessionStrategy: () => void = noop - - let bufferApiCalls = new BoundedBuffer() - let addTimingStrategy: StartRumResult['addTiming'] = (name, time = timeStampNow()) => { - bufferApiCalls.add(() => addTimingStrategy(name, time)) - } - let startViewStrategy: StartRumResult['startView'] = (options, startClocks = clocksNow()) => { - bufferApiCalls.add(() => startViewStrategy(options, startClocks)) - } - let addActionStrategy: StartRumResult['addAction'] = (action, commonContext = getCommonContext()) => { - bufferApiCalls.add(() => addActionStrategy(action, commonContext)) - } - let addErrorStrategy: StartRumResult['addError'] = (providedError, commonContext = getCommonContext()) => { - bufferApiCalls.add(() => addErrorStrategy(providedError, commonContext)) - } - - let addFeatureFlagEvaluationStrategy: StartRumResult['addFeatureFlagEvaluation'] = (key: string, value: any) => { - bufferApiCalls.add(() => addFeatureFlagEvaluationStrategy(key, value)) - } - - let deflateWorker: DeflateWorker | undefined + let strategy = createPreStartStrategy( + options, + getCommonContext, - function initRum(initConfiguration: RumInitConfiguration) { - if (!initConfiguration) { - display.error('Missing configuration') - return - } - // This function should be available, regardless of initialization success. - getInitConfigurationStrategy = () => deepClone(initConfiguration) - - // If we are in a Synthetics test configured to automatically inject a RUM instance, we want to - // completely discard the customer application RUM instance by ignoring their init() call. But, - // we should not ignore the init() call from the Synthetics-injected RUM instance, so the - // internal `ignoreInitIfSyntheticsWillInjectRum` option is here to bypass this condition. - if (ignoreInitIfSyntheticsWillInjectRum && willSyntheticsInjectRum()) { - return - } - - const eventBridgeAvailable = canUseEventBridge() - if (eventBridgeAvailable) { - initConfiguration = overrideInitConfigurationForBridge(initConfiguration) - } - - if (!canInitRum(initConfiguration)) { - return - } - - const configuration = validateAndBuildRumConfiguration(initConfiguration) - if (!configuration) { - return - } + (initConfiguration, configuration, deflateWorker, initialViewOptions) => { + if (initConfiguration.storeContextsAcrossPages) { + storeContextManager(configuration, globalContextManager, RUM_STORAGE_KEY, CustomerDataType.GlobalContext) + storeContextManager(configuration, userContextManager, RUM_STORAGE_KEY, CustomerDataType.User) + } - if (!eventBridgeAvailable && !configuration.sessionStoreStrategyType) { - display.warn('No storage available for session. We will not send any data.') - return - } + customerDataTrackerManager.setCompressionStatus( + deflateWorker ? CustomerDataCompressionStatus.Enabled : CustomerDataCompressionStatus.Disabled + ) - if (configuration.compressIntakeRequests && !eventBridgeAvailable && startDeflateWorker) { - deflateWorker = startDeflateWorker( + const startRumResult = startRumImpl( + initConfiguration, configuration, - 'Datadog RUM', - // Worker initialization can fail asynchronously, especially in Firefox where even CSP - // issues are reported asynchronously. For now, the SDK will continue its execution even if - // data won't be sent to Datadog. We could improve this behavior in the future. - noop + recorderApi, + customerDataTrackerManager, + getCommonContext, + initialViewOptions, + deflateWorker && options.createDeflateEncoder + ? (streamId) => options.createDeflateEncoder!(configuration, deflateWorker, streamId) + : createIdentityEncoder ) - if (!deflateWorker) { - // `startDeflateWorker` should have logged an error message explaining the issue - return - } - } - if (!configuration.trackViewsManually) { - doStartRum(initConfiguration, configuration) - } else { - // drain beforeInitCalls by buffering them until we start RUM - // if we get a startView, drain re-buffered calls before continuing to drain beforeInitCalls - // in order to ensure that calls are processed in order - const beforeInitCalls = bufferApiCalls - bufferApiCalls = new BoundedBuffer() - - startViewStrategy = (options) => { - doStartRum(initConfiguration, configuration, options) - } - beforeInitCalls.drain() - } + recorderApi.onRumStart( + startRumResult.lifeCycle, + configuration, + startRumResult.session, + startRumResult.viewContexts, + deflateWorker + ) - isAlreadyInitialized = true - } + strategy = createPostStartStrategy(initConfiguration, startRumResult) - function doStartRum( - initConfiguration: RumInitConfiguration, - configuration: RumConfiguration, - initialViewOptions?: ViewOptions - ) { - if (initConfiguration.storeContextsAcrossPages) { - storeContextManager(configuration, globalContextManager, RUM_STORAGE_KEY, CustomerDataType.GlobalContext) - storeContextManager(configuration, userContextManager, RUM_STORAGE_KEY, CustomerDataType.User) + return startRumResult } - - customerDataTrackerManager.setCompressionStatus( - deflateWorker ? CustomerDataCompressionStatus.Enabled : CustomerDataCompressionStatus.Disabled - ) - - const startRumResults = startRumImpl( - initConfiguration, - configuration, - recorderApi, - customerDataTrackerManager, - getCommonContext, - initialViewOptions, - deflateWorker && createDeflateEncoder - ? (streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId) - : createIdentityEncoder - ) - ;({ - startView: startViewStrategy, - addAction: addActionStrategy, - addError: addErrorStrategy, - addTiming: addTimingStrategy, - addFeatureFlagEvaluation: addFeatureFlagEvaluationStrategy, - getInternalContext: getInternalContextStrategy, - stopSession: stopSessionStrategy, - } = startRumResults) - - recorderApi.onRumStart( - startRumResults.lifeCycle, - configuration, - startRumResults.session, - startRumResults.viewContexts, - deflateWorker - ) - bufferApiCalls.drain() - } + ) const startView: { (name?: string): void (options: ViewOptions): void } = monitor((options?: string | ViewOptions) => { const sanitizedOptions = typeof options === 'object' ? options : { name: options } - startViewStrategy(sanitizedOptions) + strategy.startView(sanitizedOptions) }) const rumPublicApi = makePublicApi({ - init: monitor(initRum), + init: monitor((initConfiguration: RumInitConfiguration) => strategy.init(initConfiguration)), setGlobalContextProperty: monitor((key, value) => globalContextManager.setContextProperty(key, value)), @@ -255,11 +154,12 @@ export function makeRumPublicApi( clearGlobalContext: monitor(() => globalContextManager.clearContext()), - getInternalContext: monitor((startTime?: number) => getInternalContextStrategy(startTime)), - getInitConfiguration: monitor(() => getInitConfigurationStrategy()), + getInternalContext: monitor((startTime?: number) => strategy.getInternalContext(startTime)), + + getInitConfiguration: monitor(() => deepClone(strategy.initConfiguration)), addAction: monitor((name: string, context?: object) => { - addActionStrategy({ + strategy.addAction({ name: sanitize(name)!, context: sanitize(context) as Context, startClocks: clocksNow(), @@ -270,7 +170,7 @@ export function makeRumPublicApi( addError: (error: unknown, context?: object) => { const handlingStack = createHandlingStack() callMonitored(() => { - addErrorStrategy({ + strategy.addError({ error, // Do not sanitize error here, it is needed unserialized by computeRawError() handlingStack, context: sanitize(context) as Context, @@ -292,7 +192,7 @@ export function makeRumPublicApi( */ addTiming: monitor((name: string, time?: number) => { // TODO: next major decide to drop relative time support or update its behaviour - addTimingStrategy(sanitize(name)!, time as RelativeTime | TimeStamp | undefined) + strategy.addTiming(sanitize(name)!, time as RelativeTime | TimeStamp | undefined) }), setUser: monitor((newUser: User) => { @@ -315,14 +215,14 @@ export function makeRumPublicApi( startView, stopSession: monitor(() => { - stopSessionStrategy() + strategy.stopSession() }), /** * This feature is currently in beta. For more information see the full [feature flag tracking guide](https://docs.datadoghq.com/real_user_monitoring/feature_flag_tracking/). */ addFeatureFlagEvaluation: monitor((key: string, value: any) => { - addFeatureFlagEvaluationStrategy(sanitize(key)!, sanitize(value)) + strategy.addFeatureFlagEvaluation(sanitize(key)!, sanitize(value)) }), getSessionReplayLink: monitor(() => recorderApi.getSessionReplayLink()), @@ -331,20 +231,16 @@ export function makeRumPublicApi( }) return rumPublicApi +} - function canInitRum(initConfiguration: RumInitConfiguration) { - if (isAlreadyInitialized) { - displayAlreadyInitializedError('DD_RUM', initConfiguration) - return false - } - return true - } - - function overrideInitConfigurationForBridge(initConfiguration: C): C { - return assign({}, initConfiguration, { - applicationId: '00000000-aaaa-0000-aaaa-000000000000', - clientToken: 'empty', - sessionSampleRate: 100, - }) - } +function createPostStartStrategy(initConfiguration: RumInitConfiguration, startRumResult: StartRumResult): Strategy { + return assign( + { + init: (initConfiguration: RumInitConfiguration) => { + displayAlreadyInitializedError('DD_RUM', initConfiguration) + }, + initConfiguration, + }, + startRumResult + ) } diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index af8d4d1454..a32bb19da5 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -46,6 +46,9 @@ import type { CommonContext } from '../domain/contexts/commonContext' import { startDisplayContext } from '../domain/contexts/displayContext' import type { RecorderApi } from './rumPublicApi' +export type StartRum = typeof startRum +export type StartRumResult = ReturnType + export function startRum( initConfiguration: RumInitConfiguration, configuration: RumConfiguration, diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index 3850af5a80..8085d1c139 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -1,4 +1,5 @@ -export { RumPublicApi, makeRumPublicApi, RecorderApi, StartRum } from './boot/rumPublicApi' +export { RumPublicApi, makeRumPublicApi, RecorderApi } from './boot/rumPublicApi' +export { StartRum } from './boot/startRum' export { RumEvent, RumActionEvent, From 57d2bcda4c9b6f4262da5bc2ef85b19b7d447a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 16 Jan 2024 17:36:41 +0100 Subject: [PATCH 3/9] =?UTF-8?q?=E2=9C=85=E2=99=BB=EF=B8=8F=20[RUM-2445]=20?= =?UTF-8?q?move=20RUM=20public=20API=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum-core/src/boot/preStartRum.spec.ts | 490 ++++++++++++++++++ .../rum-core/src/boot/rumPublicApi.spec.ts | 438 ++-------------- 2 files changed, 520 insertions(+), 408 deletions(-) create mode 100644 packages/rum-core/src/boot/preStartRum.spec.ts diff --git a/packages/rum-core/src/boot/preStartRum.spec.ts b/packages/rum-core/src/boot/preStartRum.spec.ts new file mode 100644 index 0000000000..c85db81074 --- /dev/null +++ b/packages/rum-core/src/boot/preStartRum.spec.ts @@ -0,0 +1,490 @@ +import type { DeflateWorker, RelativeTime, TimeStamp } from '@datadog/browser-core' +import { display, getTimeStamp, noop, relativeToClocks, clocksNow } from '@datadog/browser-core' +import type { Clock } from '@datadog/browser-core/test' +import { + cleanupSyntheticsWorkerValues, + deleteEventBridgeStub, + initEventBridgeStub, + mockClock, + mockSyntheticsWorkerValues, +} from '@datadog/browser-core/test' +import type { HybridInitConfiguration, RumConfiguration, RumInitConfiguration } from '../domain/configuration' +import type { CommonContext } from '../domain/contexts/commonContext' +import type { ViewOptions } from '../domain/view/trackViews' +import { ActionType } from '../rawRumEvent.types' +import type { CustomAction } from '../domain/action/actionCollection' +import type { Strategy } from './rumPublicApi' +import type { StartRumResult } from './startRum' +import { createPreStartStrategy } from './preStartRum' + +const DEFAULT_INIT_CONFIGURATION = { applicationId: 'xxx', clientToken: 'xxx' } +const INVALID_INIT_CONFIGURATION = { clientToken: 'yes' } as RumInitConfiguration +const FAKE_WORKER = {} as DeflateWorker + +describe('preStartRum', () => { + let doStartRumSpy: jasmine.Spy< + ( + initConfiguration: RumInitConfiguration, + configuration: RumConfiguration, + deflateWorker: DeflateWorker | undefined, + initialViewOptions?: ViewOptions + ) => StartRumResult + > + let getCommonContextSpy: jasmine.Spy<() => CommonContext> + + beforeEach(() => { + doStartRumSpy = jasmine.createSpy() + getCommonContextSpy = jasmine.createSpy() + }) + + describe('configuration validation', () => { + let strategy: Strategy + let displaySpy: jasmine.Spy + + beforeEach(() => { + displaySpy = spyOn(display, 'error') + strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + }) + + it('should start when the configuration is valid', () => { + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).not.toHaveBeenCalled() + expect(doStartRumSpy).toHaveBeenCalled() + }) + + it('should not start when the configuration is missing', () => { + ;(strategy.init as () => void)() + expect(displaySpy).toHaveBeenCalled() + expect(doStartRumSpy).not.toHaveBeenCalled() + }) + + it('should not start when the configuration is invalid', () => { + strategy.init(INVALID_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalled() + expect(doStartRumSpy).not.toHaveBeenCalled() + }) + + describe('multiple init', () => { + it('should log an error if init is called several times', () => { + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalledTimes(0) + + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalledTimes(1) + }) + + it('should not log an error if init is called several times and silentMultipleInit is true', () => { + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + silentMultipleInit: true, + }) + expect(displaySpy).toHaveBeenCalledTimes(0) + + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + silentMultipleInit: true, + }) + expect(displaySpy).toHaveBeenCalledTimes(0) + }) + }) + + describe('if event bridge present', () => { + beforeEach(() => { + initEventBridgeStub() + }) + + afterEach(() => { + deleteEventBridgeStub() + }) + + it('init should accept empty application id and client token', () => { + const hybridInitConfiguration: HybridInitConfiguration = {} + strategy.init(hybridInitConfiguration as RumInitConfiguration) + expect(display.error).not.toHaveBeenCalled() + }) + + it('init should force session sample rate to 100', () => { + const invalidConfiguration: HybridInitConfiguration = { sessionSampleRate: 50 } + strategy.init(invalidConfiguration as RumInitConfiguration) + expect(strategy.initConfiguration?.sessionSampleRate).toEqual(100) + }) + + it('should initialize even if session cannot be handled', () => { + spyOnProperty(document, 'cookie', 'get').and.returnValue('') + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(doStartRumSpy).toHaveBeenCalled() + }) + }) + }) + + describe('init', () => { + afterEach(() => { + cleanupSyntheticsWorkerValues() + }) + + it('should not initialize if session cannot be handled and bridge is not present', () => { + spyOnProperty(document, 'cookie', 'get').and.returnValue('') + const displaySpy = spyOn(display, 'warn') + const strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(doStartRumSpy).not.toHaveBeenCalled() + expect(displaySpy).toHaveBeenCalled() + }) + + describe('skipInitIfSyntheticsWillInjectRum option', () => { + it('when true, ignores init() call if Synthetics will inject its own instance of RUM', () => { + mockSyntheticsWorkerValues({ injectsRum: true }) + + const strategy = createPreStartStrategy( + { + ignoreInitIfSyntheticsWillInjectRum: true, + }, + getCommonContextSpy, + doStartRumSpy + ) + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(doStartRumSpy).not.toHaveBeenCalled() + }) + + it('when false, does not ignore init() call even if Synthetics will inject its own instance of RUM', () => { + mockSyntheticsWorkerValues({ injectsRum: true }) + + const strategy = createPreStartStrategy( + { + ignoreInitIfSyntheticsWillInjectRum: false, + }, + getCommonContextSpy, + doStartRumSpy + ) + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(doStartRumSpy).toHaveBeenCalled() + }) + }) + + describe('deflate worker', () => { + let strategy: Strategy + let startDeflateWorkerSpy: jasmine.Spy + + beforeEach(() => { + startDeflateWorkerSpy = jasmine.createSpy().and.returnValue(FAKE_WORKER) + + strategy = createPreStartStrategy( + { + startDeflateWorker: startDeflateWorkerSpy, + createDeflateEncoder: noop as any, + }, + getCommonContextSpy, + doStartRumSpy + ) + }) + + afterEach(() => { + deleteEventBridgeStub() + }) + + describe('with compressIntakeRequests: false', () => { + it('does not create a deflate worker', () => { + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(startDeflateWorkerSpy).not.toHaveBeenCalled() + const worker: DeflateWorker | undefined = doStartRumSpy.calls.mostRecent().args[2] + expect(worker).toBeUndefined() + }) + }) + + describe('with compressIntakeRequests: true', () => { + it('creates a deflate worker instance', () => { + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + compressIntakeRequests: true, + }) + + expect(startDeflateWorkerSpy).toHaveBeenCalledTimes(1) + const worker: DeflateWorker | undefined = doStartRumSpy.calls.mostRecent().args[2] + expect(worker).toBeDefined() + }) + + it('aborts the initialization if it fails to create a deflate worker', () => { + startDeflateWorkerSpy.and.returnValue(undefined) + + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + compressIntakeRequests: true, + }) + + expect(doStartRumSpy).not.toHaveBeenCalled() + }) + + it('if message bridge is present, does not create a deflate worker instance', () => { + initEventBridgeStub() + + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + compressIntakeRequests: true, + }) + + expect(startDeflateWorkerSpy).not.toHaveBeenCalled() + expect(doStartRumSpy).toHaveBeenCalledTimes(1) + }) + }) + }) + + describe('trackViews mode', () => { + const AUTO_CONFIGURATION = { ...DEFAULT_INIT_CONFIGURATION } + const MANUAL_CONFIGURATION = { ...AUTO_CONFIGURATION, trackViewsManually: true } + + let clock: Clock | undefined + let strategy: Strategy + let startViewSpy: jasmine.Spy + let addTimingSpy: jasmine.Spy + + beforeEach(() => { + startViewSpy = jasmine.createSpy('startView') + addTimingSpy = jasmine.createSpy('addTiming') + doStartRumSpy.and.returnValue({ + startView: startViewSpy, + addTiming: addTimingSpy, + } as unknown as StartRumResult) + strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + }) + + afterEach(() => { + if (clock) { + clock.cleanup() + } + }) + + describe('when auto', () => { + it('should start rum at init', () => { + strategy.init(AUTO_CONFIGURATION) + + expect(doStartRumSpy).toHaveBeenCalled() + }) + + it('before init startView should be handled after init', () => { + clock = mockClock() + + clock.tick(10) + strategy.startView({ name: 'foo' }) + + expect(startViewSpy).not.toHaveBeenCalled() + + clock.tick(20) + strategy.init(AUTO_CONFIGURATION) + + expect(startViewSpy).toHaveBeenCalled() + expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'foo' }) + expect(startViewSpy.calls.argsFor(0)[1]).toEqual({ + relative: 10 as RelativeTime, + timeStamp: jasmine.any(Number) as unknown as TimeStamp, + }) + }) + }) + + describe('when views are tracked manually', () => { + it('should not start rum at init', () => { + strategy.init(MANUAL_CONFIGURATION) + + expect(doStartRumSpy).not.toHaveBeenCalled() + }) + + it('calling startView then init should start rum', () => { + strategy.startView({ name: 'foo' }) + expect(doStartRumSpy).not.toHaveBeenCalled() + expect(startViewSpy).not.toHaveBeenCalled() + + strategy.init(MANUAL_CONFIGURATION) + expect(doStartRumSpy).toHaveBeenCalled() + const initialViewOptions: ViewOptions | undefined = doStartRumSpy.calls.argsFor(0)[3] + expect(initialViewOptions).toEqual({ name: 'foo' }) + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('calling startView twice before init should start rum and create a new view', () => { + clock = mockClock() + clock.tick(10) + strategy.startView({ name: 'foo' }) + + clock.tick(10) + strategy.startView({ name: 'bar' }) + + clock.tick(10) + strategy.init(MANUAL_CONFIGURATION) + + expect(doStartRumSpy).toHaveBeenCalled() + const initialViewOptions: ViewOptions | undefined = doStartRumSpy.calls.argsFor(0)[3] + expect(initialViewOptions).toEqual({ name: 'foo' }) + expect(startViewSpy).toHaveBeenCalledOnceWith({ name: 'bar' }, relativeToClocks(20 as RelativeTime)) + }) + + it('calling init then startView should start rum', () => { + strategy.init(MANUAL_CONFIGURATION) + expect(doStartRumSpy).not.toHaveBeenCalled() + expect(startViewSpy).not.toHaveBeenCalled() + + strategy.startView({ name: 'foo' }) + expect(doStartRumSpy).toHaveBeenCalled() + const initialViewOptions: ViewOptions | undefined = doStartRumSpy.calls.argsFor(0)[3] + expect(initialViewOptions).toEqual({ name: 'foo' }) + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('API calls should be handled in order', () => { + clock = mockClock() + + clock.tick(10) + strategy.addTiming('first') + + clock.tick(10) + strategy.startView({ name: 'foo' }) + + clock.tick(10) + strategy.addTiming('second') + + clock.tick(10) + strategy.init(MANUAL_CONFIGURATION) + + expect(addTimingSpy).toHaveBeenCalledTimes(2) + + expect(addTimingSpy.calls.argsFor(0)[0]).toEqual('first') + expect(addTimingSpy.calls.argsFor(0)[1]).toEqual(getTimeStamp(10 as RelativeTime)) + + expect(addTimingSpy.calls.argsFor(1)[0]).toEqual('second') + expect(addTimingSpy.calls.argsFor(1)[1]).toEqual(getTimeStamp(30 as RelativeTime)) + }) + }) + }) + }) + + describe('getInternalContext', () => { + it('returns undefined', () => { + const strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + expect(strategy.getInternalContext()).toBe(undefined) + }) + }) + + describe('stopSession', () => { + it('does not buffer the call before starting RUM', () => { + const strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + const stopSessionSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ stopSession: stopSessionSpy } as unknown as StartRumResult) + + strategy.stopSession() + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(stopSessionSpy).not.toHaveBeenCalled() + }) + }) + + describe('initConfiguration', () => { + let strategy: Strategy + let initConfiguration: RumInitConfiguration + + beforeEach(() => { + strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + initConfiguration = { ...DEFAULT_INIT_CONFIGURATION, service: 'my-service', version: '1.4.2', env: 'dev' } + }) + + afterEach(() => { + cleanupSyntheticsWorkerValues() + }) + + it('is undefined before init', () => { + expect(strategy.initConfiguration).toBe(undefined) + }) + + it('returns the user configuration after init', () => { + strategy.init(initConfiguration) + + expect(strategy.initConfiguration).toEqual(initConfiguration) + }) + + it('returns the user configuration even if skipInitIfSyntheticsWillInjectRum is true', () => { + mockSyntheticsWorkerValues({ injectsRum: true }) + + const strategy = createPreStartStrategy( + { + ignoreInitIfSyntheticsWillInjectRum: true, + }, + getCommonContextSpy, + doStartRumSpy + ) + strategy.init(initConfiguration) + + expect(strategy.initConfiguration).toEqual(initConfiguration) + }) + }) + + describe('buffers API calls before starting RUM', () => { + let strategy: Strategy + + beforeEach(() => { + strategy = createPreStartStrategy({}, getCommonContextSpy, doStartRumSpy) + }) + + it('addAction', () => { + const addActionSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ addAction: addActionSpy } as unknown as StartRumResult) + + const customAction: CustomAction = { + name: 'foo', + type: ActionType.CUSTOM, + startClocks: clocksNow(), + } + strategy.addAction(customAction) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(addActionSpy).toHaveBeenCalledOnceWith(customAction, undefined) + }) + + it('addError', () => { + const addErrorSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ addError: addErrorSpy } as unknown as StartRumResult) + + const error = { + error: new Error('foo'), + handlingStack: '', + context: {}, + startClocks: clocksNow(), + } + strategy.addError(error) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(addErrorSpy).toHaveBeenCalledOnceWith(error, undefined) + }) + + it('startView', () => { + const startViewSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ startView: startViewSpy } as unknown as StartRumResult) + + const options = { name: 'foo' } + const clockState = clocksNow() + strategy.startView(options, clockState) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(startViewSpy).toHaveBeenCalledOnceWith(options, clockState) + }) + + it('addTiming', () => { + const addTimingSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ addTiming: addTimingSpy } as unknown as StartRumResult) + + const name = 'foo' + const time = 123 as TimeStamp + strategy.addTiming(name, time) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(addTimingSpy).toHaveBeenCalledOnceWith(name, time) + }) + + it('addFeatureFlagEvaluation', () => { + const addFeatureFlagEvaluationSpy = jasmine.createSpy() + doStartRumSpy.and.returnValue({ + addFeatureFlagEvaluation: addFeatureFlagEvaluationSpy, + } as unknown as StartRumResult) + + const key = 'foo' + const value = 'bar' + strategy.addFeatureFlagEvaluation(key, value) + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(addFeatureFlagEvaluationSpy).toHaveBeenCalledOnceWith(key, value) + }) + }) +}) diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index cfdefbbf6e..77396190ff 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -1,34 +1,15 @@ -import type { - RelativeTime, - TimeStamp, - Context, - DeflateWorker, - CustomerDataTrackerManager, - DeflateEncoderStreamId, - Encoder, -} from '@datadog/browser-core' +import type { RelativeTime, Context, DeflateWorker, CustomerDataTrackerManager } from '@datadog/browser-core' import { ONE_SECOND, - getTimeStamp, display, DefaultPrivacyLevel, removeStorageListeners, - noop, - resetExperimentalFeatures, - createIdentityEncoder, CustomerDataCompressionStatus, } from '@datadog/browser-core' -import { - initEventBridgeStub, - deleteEventBridgeStub, - cleanupSyntheticsWorkerValues, - mockSyntheticsWorkerValues, -} from '@datadog/browser-core/test' +import { cleanupSyntheticsWorkerValues } from '@datadog/browser-core/test' import type { TestSetupBuilder } from '../../test' import { setup, noopRecorderApi } from '../../test' -import type { HybridInitConfiguration, RumInitConfiguration } from '../domain/configuration' import { ActionType } from '../rawRumEvent.types' -import type { ViewOptions } from '../domain/view/trackViews' import type { RumPublicApi, RecorderApi } from './rumPublicApi' import { makeRumPublicApi } from './rumPublicApi' import type { StartRum } from './startRum' @@ -47,93 +28,9 @@ const noopStartRum = (): ReturnType => ({ stop: () => undefined, }) const DEFAULT_INIT_CONFIGURATION = { applicationId: 'xxx', clientToken: 'xxx' } -const INVALID_INIT_CONFIGURATION = { clientToken: 'yes' } as RumInitConfiguration const FAKE_WORKER = {} as DeflateWorker describe('rum public api', () => { - describe('configuration validation', () => { - let rumPublicApi: RumPublicApi - let displaySpy: jasmine.Spy - let startRumSpy: jasmine.Spy - - beforeEach(() => { - displaySpy = spyOn(display, 'error') - startRumSpy = jasmine.createSpy().and.callFake(noopStartRum) - rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi) - }) - - it('should start when the configuration is valid', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).not.toHaveBeenCalled() - expect(startRumSpy).toHaveBeenCalled() - }) - - it('should not start when the configuration is missing', () => { - ;(rumPublicApi.init as () => void)() - expect(displaySpy).toHaveBeenCalled() - expect(startRumSpy).not.toHaveBeenCalled() - }) - - it('should not start when the configuration is invalid', () => { - rumPublicApi.init(INVALID_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalled() - expect(startRumSpy).not.toHaveBeenCalled() - }) - - describe('multiple init', () => { - it('should log an error if init is called several times', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalledTimes(0) - - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalledTimes(1) - }) - - it('should not log an error if init is called several times and silentMultipleInit is true', () => { - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - silentMultipleInit: true, - }) - expect(displaySpy).toHaveBeenCalledTimes(0) - - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - silentMultipleInit: true, - }) - expect(displaySpy).toHaveBeenCalledTimes(0) - }) - }) - - describe('if event bridge present', () => { - beforeEach(() => { - initEventBridgeStub() - }) - - afterEach(() => { - deleteEventBridgeStub() - }) - - it('init should accept empty application id and client token', () => { - const hybridInitConfiguration: HybridInitConfiguration = {} - rumPublicApi.init(hybridInitConfiguration as RumInitConfiguration) - expect(display.error).not.toHaveBeenCalled() - }) - - it('init should force session sample rate to 100', () => { - const invalidConfiguration: HybridInitConfiguration = { sessionSampleRate: 50 } - rumPublicApi.init(invalidConfiguration as RumInitConfiguration) - expect(rumPublicApi.getInitConfiguration()?.sessionSampleRate).toEqual(100) - }) - - it('should initialize even if session cannot be handled', () => { - spyOnProperty(document, 'cookie', 'get').and.returnValue('') - const rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi, {}) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(startRumSpy).toHaveBeenCalled() - }) - }) - }) - describe('init', () => { let startRumSpy: jasmine.Spy @@ -145,46 +42,11 @@ describe('rum public api', () => { cleanupSyntheticsWorkerValues() }) - it('should not initialize if session cannot be handled and bridge is not present', () => { - spyOnProperty(document, 'cookie', 'get').and.returnValue('') - const displaySpy = spyOn(display, 'warn') - const rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi, {}) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(startRumSpy).not.toHaveBeenCalled() - expect(displaySpy).toHaveBeenCalled() - }) - - describe('skipInitIfSyntheticsWillInjectRum option', () => { - it('when true, ignores init() call if Synthetics will inject its own instance of RUM', () => { - mockSyntheticsWorkerValues({ injectsRum: true }) - - const rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi, { - ignoreInitIfSyntheticsWillInjectRum: true, - }) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - - expect(startRumSpy).not.toHaveBeenCalled() - }) - - it('when false, does not ignore init() call even if Synthetics will inject its own instance of RUM', () => { - mockSyntheticsWorkerValues({ injectsRum: true }) - - const rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi, { - ignoreInitIfSyntheticsWillInjectRum: false, - }) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - - expect(startRumSpy).toHaveBeenCalled() - }) - }) - describe('deflate worker', () => { let rumPublicApi: RumPublicApi - let startDeflateWorkerSpy: jasmine.Spy let recorderApiOnRumStartSpy: jasmine.Spy beforeEach(() => { - startDeflateWorkerSpy = jasmine.createSpy().and.returnValue(FAKE_WORKER) recorderApiOnRumStartSpy = jasmine.createSpy() rumPublicApi = makeRumPublicApi( @@ -194,69 +56,17 @@ describe('rum public api', () => { onRumStart: recorderApiOnRumStartSpy, }, { - startDeflateWorker: startDeflateWorkerSpy, - createDeflateEncoder: noop as any, + startDeflateWorker: () => FAKE_WORKER, } ) }) - afterEach(() => { - resetExperimentalFeatures() - deleteEventBridgeStub() - }) - - describe('with compressIntakeRequests: false', () => { - it('does not create a deflate worker', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - - expect(startDeflateWorkerSpy).not.toHaveBeenCalled() - const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[6] - expect(createEncoder).toBe(createIdentityEncoder) - }) - }) - - describe('with compressIntakeRequests: true', () => { - it('creates a deflate worker instance', () => { - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - compressIntakeRequests: true, - }) - - expect(startDeflateWorkerSpy).toHaveBeenCalledTimes(1) - const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[6] - expect(createEncoder).not.toBe(createIdentityEncoder) - }) - - it('aborts the initialization if it fails to create a deflate worker', () => { - startDeflateWorkerSpy.and.returnValue(undefined) - - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - compressIntakeRequests: true, - }) - - expect(startRumSpy).not.toHaveBeenCalled() - }) - - it('if message bridge is present, does not create a deflate worker instance', () => { - initEventBridgeStub() - - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - compressIntakeRequests: true, - }) - - expect(startDeflateWorkerSpy).not.toHaveBeenCalled() - expect(startRumSpy).toHaveBeenCalledTimes(1) - }) - - it('pass the worker to the recorder API', () => { - rumPublicApi.init({ - ...DEFAULT_INIT_CONFIGURATION, - compressIntakeRequests: true, - }) - expect(recorderApiOnRumStartSpy.calls.mostRecent().args[4]).toBe(FAKE_WORKER) + it('pass the worker to the recorder API', () => { + rumPublicApi.init({ + ...DEFAULT_INIT_CONFIGURATION, + compressIntakeRequests: true, }) + expect(recorderApiOnRumStartSpy.calls.mostRecent().args[4]).toBe(FAKE_WORKER) }) }) @@ -309,11 +119,6 @@ describe('rum public api', () => { ) }) - it('returns undefined before init', () => { - expect(rumPublicApi.getInternalContext()).toBe(undefined) - expect(getInternalContextSpy).not.toHaveBeenCalled() - }) - it('returns the internal context after init', () => { rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) @@ -331,38 +136,13 @@ describe('rum public api', () => { }) describe('getInitConfiguration', () => { - let rumPublicApi: RumPublicApi - let initConfiguration: RumInitConfiguration + it('clones the init configuration', () => { + const rumPublicApi = makeRumPublicApi(noopStartRum, noopRecorderApi) - beforeEach(() => { - rumPublicApi = makeRumPublicApi(noopStartRum, noopRecorderApi) - initConfiguration = { ...DEFAULT_INIT_CONFIGURATION, service: 'my-service', version: '1.4.2', env: 'dev' } - }) - afterEach(() => { - cleanupSyntheticsWorkerValues() - }) - - it('returns undefined before init', () => { - expect(rumPublicApi.getInitConfiguration()).toBe(undefined) - }) - - it('returns the user configuration after init', () => { - rumPublicApi.init(initConfiguration) - - expect(rumPublicApi.getInitConfiguration()).toEqual(initConfiguration) - expect(rumPublicApi.getInitConfiguration()).not.toBe(initConfiguration) - }) - - it('returns the user configuration even if skipInitIfSyntheticsWillInjectRum is true', () => { - mockSyntheticsWorkerValues({ injectsRum: true }) - - const rumPublicApi = makeRumPublicApi(noopStartRum, noopRecorderApi, { - ignoreInitIfSyntheticsWillInjectRum: true, - }) - rumPublicApi.init(initConfiguration) + rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(rumPublicApi.getInitConfiguration()).toEqual(initConfiguration) - expect(rumPublicApi.getInitConfiguration()).not.toBe(initConfiguration) + expect(rumPublicApi.getInitConfiguration()).toEqual(DEFAULT_INIT_CONFIGURATION) + expect(rumPublicApi.getInitConfiguration()).not.toBe(DEFAULT_INIT_CONFIGURATION) }) }) @@ -744,21 +524,6 @@ describe('rum public api', () => { setupBuilder.cleanup() }) - it('should allow to add custom timing before init', () => { - const { clock } = setupBuilder.withFakeClock().build() - - clock.tick(10) - rumPublicApi.addTiming('foo') - - expect(addTimingSpy).not.toHaveBeenCalled() - - clock.tick(20) - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - - expect(addTimingSpy.calls.argsFor(0)[0]).toEqual('foo') - expect(addTimingSpy.calls.argsFor(0)[1]).toEqual(getTimeStamp(10 as RelativeTime)) - }) - it('should add custom timings', () => { rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) @@ -813,174 +578,31 @@ describe('rum public api', () => { }) }) - describe('trackViews mode', () => { - const AUTO_CONFIGURATION = { ...DEFAULT_INIT_CONFIGURATION } - const MANUAL_CONFIGURATION = { ...AUTO_CONFIGURATION, trackViewsManually: true } - - let startRumSpy: jasmine.Spy - let startViewSpy: jasmine.Spy['startView']> - let addTimingSpy: jasmine.Spy['addTiming']> - let displaySpy: jasmine.Spy<() => void> - let recorderApiOnRumStartSpy: jasmine.Spy<() => void> - let rumPublicApi: RumPublicApi - let setupBuilder: TestSetupBuilder - - beforeEach(() => { - startViewSpy = jasmine.createSpy('startView') - addTimingSpy = jasmine.createSpy('addTiming') - displaySpy = spyOn(display, 'error') - startRumSpy = jasmine.createSpy('startRum').and.returnValue({ - ...noopStartRum(), - addTiming: addTimingSpy, - startView: startViewSpy, - }) - recorderApiOnRumStartSpy = jasmine.createSpy('recorderApiOnRumStart') - rumPublicApi = makeRumPublicApi(startRumSpy, { ...noopRecorderApi, onRumStart: recorderApiOnRumStartSpy }) - setupBuilder = setup() - }) - - afterEach(() => { - setupBuilder.cleanup() - }) - - describe('when auto', () => { - it('should start rum at init', () => { - rumPublicApi.init(AUTO_CONFIGURATION) - - expect(startRumSpy).toHaveBeenCalled() - expect(recorderApiOnRumStartSpy).toHaveBeenCalled() - }) - - it('before init startView should be handled after init', () => { - const { clock } = setupBuilder.withFakeClock().build() - - clock.tick(10) - rumPublicApi.startView('foo') - - expect(startViewSpy).not.toHaveBeenCalled() - - clock.tick(20) - rumPublicApi.init(AUTO_CONFIGURATION) - - expect(startViewSpy).toHaveBeenCalled() - expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'foo' }) - expect(startViewSpy.calls.argsFor(0)[1]).toEqual({ - relative: 10 as RelativeTime, - timeStamp: jasmine.any(Number) as unknown as TimeStamp, - }) - }) - - it('after init startView should be handle immediately', () => { - rumPublicApi.init(AUTO_CONFIGURATION) - - rumPublicApi.startView('foo') - - expect(startViewSpy).toHaveBeenCalled() - expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'foo' }) - expect(startViewSpy.calls.argsFor(0)[1]).toBeUndefined() - expect(displaySpy).not.toHaveBeenCalled() - }) - }) - - describe('when views are tracked manually', () => { - it('should not start rum at init', () => { - rumPublicApi.init(MANUAL_CONFIGURATION) - - expect(startRumSpy).not.toHaveBeenCalled() - expect(recorderApiOnRumStartSpy).not.toHaveBeenCalled() - }) - - it('before init startView should start rum', () => { - rumPublicApi.startView('foo') - expect(startRumSpy).not.toHaveBeenCalled() - expect(startViewSpy).not.toHaveBeenCalled() - - rumPublicApi.init(MANUAL_CONFIGURATION) - expect(startRumSpy).toHaveBeenCalled() - const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5] - expect(initialViewOptions).toEqual({ name: 'foo' }) - expect(recorderApiOnRumStartSpy).toHaveBeenCalled() - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('after init startView should start rum', () => { - rumPublicApi.init(MANUAL_CONFIGURATION) - expect(startRumSpy).not.toHaveBeenCalled() - expect(startViewSpy).not.toHaveBeenCalled() - - rumPublicApi.startView('foo') - expect(startRumSpy).toHaveBeenCalled() - const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5] - expect(initialViewOptions).toEqual({ name: 'foo' }) - expect(recorderApiOnRumStartSpy).toHaveBeenCalled() - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('after start rum startView should start view', () => { - rumPublicApi.init(MANUAL_CONFIGURATION) - rumPublicApi.startView('foo') - rumPublicApi.startView('bar') - - expect(startRumSpy).toHaveBeenCalled() - const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5] - expect(initialViewOptions).toEqual({ name: 'foo' }) - expect(recorderApiOnRumStartSpy).toHaveBeenCalled() - expect(startViewSpy).toHaveBeenCalled() - expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'bar' }) - expect(startViewSpy.calls.argsFor(0)[1]).toBeUndefined() - }) - - it('API calls should be handled in order', () => { - const { clock } = setupBuilder.withFakeClock().build() - - clock.tick(10) - rumPublicApi.addTiming('first') - - clock.tick(10) - rumPublicApi.startView('foo') - - clock.tick(10) - rumPublicApi.addTiming('second') - - clock.tick(10) - rumPublicApi.init(MANUAL_CONFIGURATION) - - clock.tick(10) - rumPublicApi.addTiming('third') - - expect(addTimingSpy).toHaveBeenCalledTimes(3) - - expect(addTimingSpy.calls.argsFor(0)[0]).toEqual('first') - expect(addTimingSpy.calls.argsFor(0)[1]).toEqual(getTimeStamp(10 as RelativeTime)) - - expect(addTimingSpy.calls.argsFor(1)[0]).toEqual('second') - expect(addTimingSpy.calls.argsFor(1)[1]).toEqual(getTimeStamp(30 as RelativeTime)) - - expect(addTimingSpy.calls.argsFor(2)[0]).toEqual('third') - expect(addTimingSpy.calls.argsFor(2)[1]).toBeUndefined() // no time saved when started - }) - }) - }) - describe('stopSession', () => { - let rumPublicApi: RumPublicApi - let stopSessionSpy: jasmine.Spy - - beforeEach(() => { - stopSessionSpy = jasmine.createSpy() - rumPublicApi = makeRumPublicApi(() => ({ ...noopStartRum(), stopSession: stopSessionSpy }), noopRecorderApi) - }) - it('calls stopSession on the startRum result', () => { + const stopSessionSpy = jasmine.createSpy() + const rumPublicApi = makeRumPublicApi(() => ({ ...noopStartRum(), stopSession: stopSessionSpy }), noopRecorderApi) rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) rumPublicApi.stopSession() expect(stopSessionSpy).toHaveBeenCalled() }) + }) - it('does nothing when called before init', () => { - rumPublicApi.stopSession() + describe('startView', () => { + it('should call RUM results startView with the view name', () => { + const startViewSpy = jasmine.createSpy() + const rumPublicApi = makeRumPublicApi(() => ({ ...noopStartRum(), startView: startViewSpy }), noopRecorderApi) + rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) + rumPublicApi.startView('foo') + expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'foo' }) + }) + + it('should call RUM results startView with the view options', () => { + const startViewSpy = jasmine.createSpy() + const rumPublicApi = makeRumPublicApi(() => ({ ...noopStartRum(), startView: startViewSpy }), noopRecorderApi) rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - expect(stopSessionSpy).not.toHaveBeenCalled() + rumPublicApi.startView({ name: 'foo', service: 'bar', version: 'baz' }) + expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'foo', service: 'bar', version: 'baz' }) }) }) From c6b50a32be5a8ab4eab061ae13a35951eb4baeff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 10 Jan 2024 16:03:59 +0100 Subject: [PATCH 4/9] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[RUM-2445]=20split=20l?= =?UTF-8?q?ogsPublicApi?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/logsPublicApi.spec.ts | 3 +- packages/logs/src/boot/logsPublicApi.ts | 125 +++++++------------ packages/logs/src/boot/preStartLogs.ts | 79 ++++++++++++ packages/logs/src/boot/startLogs.ts | 3 + 4 files changed, 127 insertions(+), 83 deletions(-) create mode 100644 packages/logs/src/boot/preStartLogs.ts diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index 4670a1781e..816ad121eb 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -6,8 +6,9 @@ import type { HybridInitConfiguration, LogsInitConfiguration } from '../domain/c import type { Logger, LogsMessage } from '../domain/logger' import { HandlerType, StatusType } from '../domain/logger' import type { CommonContext } from '../rawLogsEvent.types' -import type { LogsPublicApi, StartLogs } from './logsPublicApi' +import type { LogsPublicApi } from './logsPublicApi' import { makeLogsPublicApi } from './logsPublicApi' +import type { StartLogs } from './startLogs' const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' } const INVALID_INIT_CONFIGURATION = {} as LogsInitConfiguration diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index 4854948883..d9d8d842ac 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -1,28 +1,24 @@ -import type { Context, InitConfiguration, User } from '@datadog/browser-core' +import type { Context, User } from '@datadog/browser-core' import { CustomerDataType, assign, - BoundedBuffer, createContextManager, makePublicApi, monitor, - display, - deepClone, - canUseEventBridge, - timeStampNow, checkUser, sanitizeUser, sanitize, createCustomerDataTrackerManager, storeContextManager, displayAlreadyInitializedError, + deepClone, } from '@datadog/browser-core' import type { LogsInitConfiguration } from '../domain/configuration' -import { validateAndBuildLogsConfiguration } from '../domain/configuration' -import type { HandlerType, StatusType, LogsMessage } from '../domain/logger' +import type { HandlerType, StatusType } from '../domain/logger' import { Logger } from '../domain/logger' import { buildCommonContext } from '../domain/contexts/commonContext' -import type { startLogs } from './startLogs' +import type { StartLogs, StartLogsResult } from './startLogs' +import { createPreStartStrategy } from './preStartLogs' export interface LoggerConfiguration { level?: StatusType @@ -32,84 +28,49 @@ export interface LoggerConfiguration { export type LogsPublicApi = ReturnType -export type StartLogs = typeof startLogs - -type StartLogsResult = ReturnType - const LOGS_STORAGE_KEY = 'logs' -export function makeLogsPublicApi(startLogsImpl: StartLogs) { - let isAlreadyInitialized = false +export interface Strategy { + init: (initConfiguration: LogsInitConfiguration) => void + initConfiguration: LogsInitConfiguration | undefined + getInternalContext: StartLogsResult['getInternalContext'] + handleLog: StartLogsResult['handleLog'] +} +export function makeLogsPublicApi(startLogsImpl: StartLogs) { const customerDataTrackerManager = createCustomerDataTrackerManager() const globalContextManager = createContextManager( customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext) ) const userContextManager = createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User)) - const customLoggers: { [name: string]: Logger | undefined } = {} - let getInternalContextStrategy: StartLogsResult['getInternalContext'] = () => undefined - - const beforeInitLoggerLog = new BoundedBuffer() - - let handleLogStrategy: StartLogsResult['handleLog'] = ( - logsMessage: LogsMessage, - logger: Logger, - savedCommonContext = getCommonContext(), - date = timeStampNow() - ) => { - beforeInitLoggerLog.add(() => handleLogStrategy(logsMessage, logger, savedCommonContext, date)) - } - - let getInitConfigurationStrategy = (): InitConfiguration | undefined => undefined - const mainLogger = new Logger( - (...params) => handleLogStrategy(...params), - customerDataTrackerManager.createDetachedTracker() - ) - function getCommonContext() { return buildCommonContext(globalContextManager, userContextManager) } - return makePublicApi({ - logger: mainLogger, - - init: monitor((initConfiguration: LogsInitConfiguration) => { - if (!initConfiguration) { - display.error('Missing configuration') - return - } - // This function should be available, regardless of initialization success. - getInitConfigurationStrategy = () => deepClone(initConfiguration) - - if (canUseEventBridge()) { - initConfiguration = overrideInitConfigurationForBridge(initConfiguration) - } + let strategy = createPreStartStrategy(getCommonContext, (initConfiguration, configuration) => { + if (initConfiguration.storeContextsAcrossPages) { + storeContextManager(configuration, globalContextManager, LOGS_STORAGE_KEY, CustomerDataType.GlobalContext) + storeContextManager(configuration, userContextManager, LOGS_STORAGE_KEY, CustomerDataType.User) + } - if (!canInitLogs(initConfiguration)) { - return - } + const startLogsResult = startLogsImpl(initConfiguration, configuration, getCommonContext) - const configuration = validateAndBuildLogsConfiguration(initConfiguration) - if (!configuration) { - return - } + strategy = createActualStrategy(initConfiguration, startLogsResult) + return startLogsResult + }) - if (initConfiguration.storeContextsAcrossPages) { - storeContextManager(configuration, globalContextManager, LOGS_STORAGE_KEY, CustomerDataType.GlobalContext) - storeContextManager(configuration, userContextManager, LOGS_STORAGE_KEY, CustomerDataType.User) - } + const customLoggers: { [name: string]: Logger | undefined } = {} - ;({ handleLog: handleLogStrategy, getInternalContext: getInternalContextStrategy } = startLogsImpl( - initConfiguration, - configuration, - getCommonContext - )) + const mainLogger = new Logger( + (...params) => strategy.handleLog(...params), + customerDataTrackerManager.createDetachedTracker() + ) - beforeInitLoggerLog.drain() + return makePublicApi({ + logger: mainLogger, - isAlreadyInitialized = true - }), + init: monitor((initConfiguration: LogsInitConfiguration) => strategy.init(initConfiguration)), getGlobalContext: monitor(() => globalContextManager.getContext()), @@ -123,7 +84,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { createLogger: monitor((name: string, conf: LoggerConfiguration = {}) => { customLoggers[name] = new Logger( - (...params) => handleLogStrategy(...params), + (...params) => strategy.handleLog(...params), customerDataTrackerManager.createDetachedTracker(), sanitize(name), conf.handler, @@ -136,9 +97,9 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { getLogger: monitor((name: string) => customLoggers[name]), - getInitConfiguration: monitor(() => getInitConfigurationStrategy()), + getInitConfiguration: monitor(() => deepClone(strategy.initConfiguration)), - getInternalContext: monitor((startTime?: number | undefined) => getInternalContextStrategy(startTime)), + getInternalContext: monitor((startTime?: number | undefined) => strategy.getInternalContext(startTime)), setUser: monitor((newUser: User) => { if (checkUser(newUser)) { @@ -157,16 +118,16 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { clearUser: monitor(() => userContextManager.clearContext()), }) +} - function overrideInitConfigurationForBridge(initConfiguration: C): C { - return assign({}, initConfiguration, { clientToken: 'empty' }) - } - - function canInitLogs(initConfiguration: LogsInitConfiguration) { - if (isAlreadyInitialized) { - displayAlreadyInitializedError('DD_LOGS', initConfiguration) - return false - } - return true - } +function createActualStrategy(initConfiguration: LogsInitConfiguration, startLogsResult: StartLogsResult): Strategy { + return assign( + { + init: (initConfiguration: LogsInitConfiguration) => { + displayAlreadyInitializedError('DD_LOGS', initConfiguration) + }, + initConfiguration, + }, + startLogsResult + ) } diff --git a/packages/logs/src/boot/preStartLogs.ts b/packages/logs/src/boot/preStartLogs.ts new file mode 100644 index 0000000000..ee112277a2 --- /dev/null +++ b/packages/logs/src/boot/preStartLogs.ts @@ -0,0 +1,79 @@ +import { + BoundedBuffer, + assign, + canUseEventBridge, + display, + displayAlreadyInitializedError, + noop, + timeStampNow, +} from '@datadog/browser-core' +import { + validateAndBuildLogsConfiguration, + type LogsConfiguration, + type LogsInitConfiguration, +} from '../domain/configuration' +import type { CommonContext } from '../rawLogsEvent.types' +import type { Strategy } from './logsPublicApi' +import type { StartLogsResult } from './startLogs' + +export function createPreStartStrategy( + getCommonContext: () => CommonContext, + doStartLogs: (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult +): Strategy { + const bufferApiCalls = new BoundedBuffer() + let cachedInitConfiguration: LogsInitConfiguration | undefined + let cachedConfiguration: LogsConfiguration | undefined + + function tryStartLogs() { + if (!cachedConfiguration || !cachedInitConfiguration) { + return + } + + const startLogsResult = doStartLogs(cachedInitConfiguration, cachedConfiguration) + + bufferApiCalls.drain(startLogsResult) + } + + return { + init(initConfiguration) { + if (!initConfiguration) { + display.error('Missing configuration') + return + } + + if (canUseEventBridge()) { + initConfiguration = overrideInitConfigurationForBridge(initConfiguration) + } + + // Expose the initial configuration regardless of initialization success. + cachedInitConfiguration = initConfiguration + + if (cachedConfiguration) { + displayAlreadyInitializedError('DD_LOGS', initConfiguration) + return + } + + const configuration = validateAndBuildLogsConfiguration(initConfiguration) + if (!configuration) { + return + } + + cachedConfiguration = configuration + tryStartLogs() + }, + + get initConfiguration() { + return cachedInitConfiguration + }, + + getInternalContext: noop as () => undefined, + + handleLog(message, statusType, context = getCommonContext(), date = timeStampNow()) { + bufferApiCalls.add((startLogsResult) => startLogsResult.handleLog(message, statusType, context, date)) + }, + } +} + +function overrideInitConfigurationForBridge(initConfiguration: LogsInitConfiguration): LogsInitConfiguration { + return assign({}, initConfiguration, { clientToken: 'empty' }) +} diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index 6f1954cad6..0135b5b8a5 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -20,6 +20,9 @@ import { startReportError } from '../domain/reportError' import { startLogsTelemetry } from '../domain/logsTelemetry' import type { CommonContext } from '../rawLogsEvent.types' +export type StartLogs = typeof startLogs +export type StartLogsResult = ReturnType + export function startLogs( initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration, From b8a9a4ec8430a4efda0f3fbd84d66da8dbcc90a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 16 Jan 2024 18:43:48 +0100 Subject: [PATCH 5/9] =?UTF-8?q?=E2=9C=85=E2=99=BB=EF=B8=8F=20[RUM-2445]=20?= =?UTF-8?q?move=20Logs=20public=20API=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/logsPublicApi.spec.ts | 209 ++----------------- packages/logs/src/boot/preStartLogs.spec.ts | 204 ++++++++++++++++++ 2 files changed, 227 insertions(+), 186 deletions(-) create mode 100644 packages/logs/src/boot/preStartLogs.spec.ts diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index 816ad121eb..4be9fea3dc 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -1,8 +1,5 @@ import type { TimeStamp } from '@datadog/browser-core' -import { monitor, ONE_SECOND, display, removeStorageListeners } from '@datadog/browser-core' -import type { Clock } from '@datadog/browser-core/test' -import { deleteEventBridgeStub, initEventBridgeStub, mockClock } from '@datadog/browser-core/test' -import type { HybridInitConfiguration, LogsInitConfiguration } from '../domain/configuration' +import { monitor, display, removeStorageListeners } from '@datadog/browser-core' import type { Logger, LogsMessage } from '../domain/logger' import { HandlerType, StatusType } from '../domain/logger' import type { CommonContext } from '../rawLogsEvent.types' @@ -11,7 +8,6 @@ import { makeLogsPublicApi } from './logsPublicApi' import type { StartLogs } from './startLogs' const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' } -const INVALID_INIT_CONFIGURATION = {} as LogsInitConfiguration const mockSessionId = 'some-session-id' const getInternalContext = () => ({ session_id: mockSessionId }) @@ -37,6 +33,26 @@ describe('logs entry', () => { startLogs = jasmine.createSpy().and.callFake(() => ({ handleLog: handleLogSpy, getInternalContext })) }) + it('should add a `_setDebug` that works', () => { + const displaySpy = spyOn(display, 'error') + const LOGS = makeLogsPublicApi(startLogs) + const setDebug: (debug: boolean) => void = (LOGS as any)._setDebug + expect(!!setDebug).toEqual(true) + + monitor(() => { + throw new Error() + })() + expect(displaySpy).toHaveBeenCalledTimes(0) + + setDebug(true) + monitor(() => { + throw new Error() + })() + expect(displaySpy).toHaveBeenCalledTimes(1) + + setDebug(false) + }) + it('should define the public API with init', () => { const LOGS = makeLogsPublicApi(startLogs) expect(!!LOGS).toEqual(true) @@ -48,99 +64,6 @@ describe('logs entry', () => { expect(LOGS.version).toBe('test') }) - describe('configuration validation', () => { - let LOGS: LogsPublicApi - let displaySpy: jasmine.Spy - - beforeEach(() => { - displaySpy = spyOn(display, 'error') - LOGS = makeLogsPublicApi(startLogs) - }) - - it('should start when the configuration is valid', () => { - LOGS.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).not.toHaveBeenCalled() - expect(startLogs).toHaveBeenCalled() - }) - - it('should not start when the configuration is missing', () => { - ;(LOGS.init as () => void)() - expect(displaySpy).toHaveBeenCalled() - expect(startLogs).not.toHaveBeenCalled() - }) - - it('should not start when the configuration is invalid', () => { - LOGS.init(INVALID_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalled() - expect(startLogs).not.toHaveBeenCalled() - }) - - it("should return init configuration even if it's invalid", () => { - LOGS.init(INVALID_INIT_CONFIGURATION) - expect(LOGS.getInitConfiguration()).toEqual(INVALID_INIT_CONFIGURATION) - }) - - it('should add a `_setDebug` that works', () => { - const setDebug: (debug: boolean) => void = (LOGS as any)._setDebug - expect(!!setDebug).toEqual(true) - - monitor(() => { - throw new Error() - })() - expect(displaySpy).toHaveBeenCalledTimes(0) - - setDebug(true) - monitor(() => { - throw new Error() - })() - expect(displaySpy).toHaveBeenCalledTimes(1) - - setDebug(false) - }) - - describe('multiple init', () => { - it('should log an error if init is called several times', () => { - LOGS.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalledTimes(0) - - LOGS.init(DEFAULT_INIT_CONFIGURATION) - expect(displaySpy).toHaveBeenCalledTimes(1) - }) - - it('should not log an error if init is called several times and silentMultipleInit is true', () => { - LOGS.init({ - ...DEFAULT_INIT_CONFIGURATION, - silentMultipleInit: true, - }) - expect(displaySpy).toHaveBeenCalledTimes(0) - - LOGS.init({ - ...DEFAULT_INIT_CONFIGURATION, - silentMultipleInit: true, - }) - expect(displaySpy).toHaveBeenCalledTimes(0) - }) - }) - - describe('if event bridge present', () => { - beforeEach(() => { - initEventBridgeStub() - }) - - afterEach(() => { - deleteEventBridgeStub() - }) - - it('init should accept empty client token', () => { - const hybridInitConfiguration: HybridInitConfiguration = {} - LOGS.init(hybridInitConfiguration as LogsInitConfiguration) - - expect(displaySpy).not.toHaveBeenCalled() - expect(startLogs).toHaveBeenCalled() - }) - }) - }) - describe('common context', () => { let LOGS: LogsPublicApi @@ -164,84 +87,7 @@ describe('logs entry', () => { }) }) - describe('pre-init API usages', () => { - let LOGS: LogsPublicApi - let clock: Clock - - beforeEach(() => { - LOGS = makeLogsPublicApi(startLogs) - clock = mockClock() - }) - - afterEach(() => { - clock.cleanup() - }) - - it('allows sending logs', () => { - LOGS.logger.log('message') - - expect(handleLogSpy).not.toHaveBeenCalled() - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(handleLogSpy.calls.all().length).toBe(1) - expect(getLoggedMessage(0).message.message).toBe('message') - }) - - it('allows creating logger', () => { - const logger = LOGS.createLogger('foo') - logger.error('message') - - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(getLoggedMessage(0).logger.getContext().logger).toEqual({ name: 'foo' }) - expect(getLoggedMessage(0).message.message).toEqual('message') - }) - - it('returns undefined initial configuration', () => { - expect(LOGS.getInitConfiguration()).toBeUndefined() - }) - - describe('save context when submitting a log', () => { - it('saves the date', () => { - LOGS.logger.log('message') - clock.tick(ONE_SECOND) - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(getLoggedMessage(0).savedDate).toEqual((Date.now() - ONE_SECOND) as TimeStamp) - }) - - it('saves the URL', () => { - const initialLocation = window.location.href - LOGS.logger.log('message') - location.href = `#tata${Math.random()}` - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(getLoggedMessage(0).savedCommonContext!.view.url).toEqual(initialLocation) - }) - - it('stores a deep copy of the global context', () => { - LOGS.setGlobalContextProperty('foo', 'bar') - LOGS.logger.log('message') - LOGS.setGlobalContextProperty('foo', 'baz') - - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(getLoggedMessage(0).savedCommonContext!.context.foo).toEqual('bar') - }) - - it('stores a deep copy of the log context', () => { - const context = { foo: 'bar' } - LOGS.logger.log('message', context) - context.foo = 'baz' - - LOGS.init(DEFAULT_INIT_CONFIGURATION) - - expect(getLoggedMessage(0).message.context!.foo).toEqual('bar') - }) - }) - }) - - describe('post-init API usages', () => { + describe('post start API usages', () => { let LOGS: LogsPublicApi beforeEach(() => { @@ -323,17 +169,8 @@ describe('logs entry', () => { }) describe('internal context', () => { - let LOGS: LogsPublicApi - - beforeEach(() => { - LOGS = makeLogsPublicApi(startLogs) - }) - - it('should return undefined if not initialized', () => { - expect(LOGS.getInternalContext()).toBeUndefined() - }) - it('should get the internal context', () => { + const LOGS = makeLogsPublicApi(startLogs) LOGS.init(DEFAULT_INIT_CONFIGURATION) expect(LOGS.getInternalContext()?.session_id).toEqual(mockSessionId) }) diff --git a/packages/logs/src/boot/preStartLogs.spec.ts b/packages/logs/src/boot/preStartLogs.spec.ts new file mode 100644 index 0000000000..efcdf88e8b --- /dev/null +++ b/packages/logs/src/boot/preStartLogs.spec.ts @@ -0,0 +1,204 @@ +import { mockClock, type Clock, deleteEventBridgeStub, initEventBridgeStub } from '@datadog/browser-core/test' +import type { TimeStamp } from '@datadog/browser-core' +import { ONE_SECOND, display } from '@datadog/browser-core' +import type { CommonContext } from '../rawLogsEvent.types' +import type { HybridInitConfiguration, LogsConfiguration, LogsInitConfiguration } from '../domain/configuration' +import { StatusType, type Logger } from '../domain/logger' +import type { Strategy } from './logsPublicApi' +import { createPreStartStrategy } from './preStartLogs' +import type { StartLogsResult } from './startLogs' + +const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' } +const INVALID_INIT_CONFIGURATION = {} as LogsInitConfiguration + +describe('preStartLogs', () => { + let doStartLogsSpy: jasmine.Spy< + (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult + > + let handleLogSpy: jasmine.Spy + let getCommonContextSpy: jasmine.Spy<() => CommonContext> + let strategy: Strategy + let clock: Clock + + function getLoggedMessage(index: number) { + const [message, logger, savedCommonContext, savedDate] = handleLogSpy.calls.argsFor(index) + return { message, logger, savedCommonContext, savedDate } + } + + beforeEach(() => { + handleLogSpy = jasmine.createSpy() + doStartLogsSpy = jasmine.createSpy().and.returnValue({ + handleLog: handleLogSpy, + } as unknown as StartLogsResult) + getCommonContextSpy = jasmine.createSpy() + strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy) + clock = mockClock() + }) + + afterEach(() => { + clock.cleanup() + }) + + describe('configuration validation', () => { + let displaySpy: jasmine.Spy + + beforeEach(() => { + displaySpy = spyOn(display, 'error') + }) + + it('should start when the configuration is valid', () => { + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).not.toHaveBeenCalled() + expect(doStartLogsSpy).toHaveBeenCalled() + }) + + it('should not start when the configuration is missing', () => { + ;(strategy.init as () => void)() + expect(displaySpy).toHaveBeenCalled() + expect(doStartLogsSpy).not.toHaveBeenCalled() + }) + + it('should not start when the configuration is invalid', () => { + strategy.init(INVALID_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalled() + expect(doStartLogsSpy).not.toHaveBeenCalled() + }) + + it("should return init configuration even if it's invalid", () => { + strategy.init(INVALID_INIT_CONFIGURATION) + expect(strategy.initConfiguration).toEqual(INVALID_INIT_CONFIGURATION) + }) + + describe('multiple init', () => { + it('should log an error if init is called several times', () => { + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalledTimes(0) + + strategy.init(DEFAULT_INIT_CONFIGURATION) + expect(displaySpy).toHaveBeenCalledTimes(1) + }) + + it('should not log an error if init is called several times and silentMultipleInit is true', () => { + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + silentMultipleInit: true, + }) + expect(displaySpy).toHaveBeenCalledTimes(0) + + strategy.init({ + ...DEFAULT_INIT_CONFIGURATION, + silentMultipleInit: true, + }) + expect(displaySpy).toHaveBeenCalledTimes(0) + }) + }) + + describe('if event bridge present', () => { + beforeEach(() => { + initEventBridgeStub() + }) + + afterEach(() => { + deleteEventBridgeStub() + }) + + it('init should accept empty client token', () => { + const hybridInitConfiguration: HybridInitConfiguration = {} + strategy.init(hybridInitConfiguration as LogsInitConfiguration) + + expect(displaySpy).not.toHaveBeenCalled() + expect(doStartLogsSpy).toHaveBeenCalled() + }) + }) + }) + + it('allows sending logs', () => { + strategy.handleLog( + { + status: StatusType.info, + message: 'message', + }, + {} as Logger + ) + + expect(handleLogSpy).not.toHaveBeenCalled() + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(handleLogSpy.calls.all().length).toBe(1) + expect(getLoggedMessage(0).message.message).toBe('message') + }) + + it('returns undefined initial configuration', () => { + expect(strategy.initConfiguration).toBeUndefined() + }) + + describe('save context when submitting a log', () => { + it('saves the date', () => { + strategy.handleLog( + { + status: StatusType.info, + message: 'message', + }, + {} as Logger + ) + clock.tick(ONE_SECOND) + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(getLoggedMessage(0).savedDate).toEqual((Date.now() - ONE_SECOND) as TimeStamp) + }) + + it('saves the URL', () => { + getCommonContextSpy.and.returnValue({ view: { url: 'url' } } as unknown as CommonContext) + strategy.handleLog( + { + status: StatusType.info, + message: 'message', + }, + {} as Logger + ) + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(getLoggedMessage(0).savedCommonContext!.view.url).toEqual('url') + }) + + it('saves the common context', () => { + getCommonContextSpy.and.returnValue({ context: { foo: 'bar' } } as unknown as CommonContext) + strategy.handleLog( + { + status: StatusType.info, + message: 'message', + }, + {} as Logger + ) + getCommonContextSpy.and.returnValue({ context: { foo: 'baz' } } as unknown as CommonContext) + + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(getLoggedMessage(0).savedCommonContext!.context.foo).toEqual('bar') + }) + + it('saves the log context', () => { + const context = { foo: 'bar' } + strategy.handleLog( + { + status: StatusType.info, + message: 'message', + context: { foo: 'bar' }, + }, + {} as Logger + ) + context.foo = 'baz' + + strategy.init(DEFAULT_INIT_CONFIGURATION) + + expect(getLoggedMessage(0).message.context!.foo).toEqual('bar') + }) + }) + + describe('internal context', () => { + it('should return undefined if not initialized', () => { + const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy) + expect(strategy.getInternalContext()).toBeUndefined() + }) + }) +}) From aa8fbde1d8343102930793bc6c722de5f166656f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 24 Jan 2024 16:21:13 +0100 Subject: [PATCH 6/9] =?UTF-8?q?=F0=9F=91=8C=20rename=20createActualStrateg?= =?UTF-8?q?y=20in=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/logs/src/boot/logsPublicApi.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index d9d8d842ac..eaca555ecd 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -56,7 +56,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { const startLogsResult = startLogsImpl(initConfiguration, configuration, getCommonContext) - strategy = createActualStrategy(initConfiguration, startLogsResult) + strategy = createPostStartStrategy(initConfiguration, startLogsResult) return startLogsResult }) @@ -120,7 +120,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { }) } -function createActualStrategy(initConfiguration: LogsInitConfiguration, startLogsResult: StartLogsResult): Strategy { +function createPostStartStrategy(initConfiguration: LogsInitConfiguration, startLogsResult: StartLogsResult): Strategy { return assign( { init: (initConfiguration: LogsInitConfiguration) => { From 0300c627a999e72970999bc287fb6ad33fd04a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 24 Jan 2024 16:41:19 +0100 Subject: [PATCH 7/9] =?UTF-8?q?=F0=9F=91=8C=20change=20first=20startView?= =?UTF-8?q?=20call=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/tools/boundedBuffer.spec.ts | 10 +++++ packages/core/src/tools/boundedBuffer.ts | 7 +++ packages/rum-core/src/boot/preStartRum.ts | 43 +++++++++---------- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/core/src/tools/boundedBuffer.spec.ts b/packages/core/src/tools/boundedBuffer.spec.ts index 31accb164b..efd5046ca7 100644 --- a/packages/core/src/tools/boundedBuffer.spec.ts +++ b/packages/core/src/tools/boundedBuffer.spec.ts @@ -27,4 +27,14 @@ describe('BoundedBuffer', () => { buffered.drain() expect(spy.calls.count()).toEqual(limit) }) + + it('removes a callback', () => { + const spy = jasmine.createSpy<() => void>() + const buffered = new BoundedBuffer() + + buffered.add(spy) + buffered.remove(spy) + buffered.drain() + expect(spy).not.toHaveBeenCalled() + }) }) diff --git a/packages/core/src/tools/boundedBuffer.ts b/packages/core/src/tools/boundedBuffer.ts index 9e10f605d1..7e212e1ccd 100644 --- a/packages/core/src/tools/boundedBuffer.ts +++ b/packages/core/src/tools/boundedBuffer.ts @@ -10,6 +10,13 @@ export class BoundedBuffer { } } + remove(callback: (arg: T) => void) { + const index = this.buffer.indexOf(callback) + if (index !== -1) { + this.buffer.splice(index, 1) + } + } + drain(arg: T) { this.buffer.forEach((callback) => callback(arg)) this.buffer.length = 0 diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index 375a50c4d3..657b2eb9bf 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -31,26 +31,32 @@ export function createPreStartStrategy( ) => StartRumResult ): Strategy { const bufferApiCalls = new BoundedBuffer() - let firstStartView: { options: ViewOptions | undefined; ignore(): void } | undefined + let firstStartViewCall: + | { options: ViewOptions | undefined; callback: (startRumResult: StartRumResult) => void } + | undefined let deflateWorker: DeflateWorker | undefined let cachedInitConfiguration: RumInitConfiguration | undefined let cachedConfiguration: RumConfiguration | undefined function tryStartRum() { - if ( - !cachedInitConfiguration || - !cachedConfiguration || - (cachedConfiguration.trackViewsManually && !firstStartView) - ) { + if (!cachedInitConfiguration || !cachedConfiguration) { return } let initialViewOptions: ViewOptions | undefined if (cachedConfiguration.trackViewsManually) { - firstStartView!.ignore() - initialViewOptions = firstStartView!.options + if (!firstStartViewCall) { + return + } + // An initial view is always created when starting RUM. + // When tracking views automatically, any startView call before RUM start creates an extra + // view. + // When tracking views manually, we use the ViewOptions from the first startView call as the + // initial view options, and we remove the actual startView call so we don't create + bufferApiCalls.remove(firstStartViewCall.callback) + initialViewOptions = firstStartViewCall.options } const startRumResult = doStartRum(cachedInitConfiguration, cachedConfiguration, deflateWorker, initialViewOptions) @@ -128,22 +134,15 @@ export function createPreStartStrategy( }, startView(options, startClocks = clocksNow()) { - let ignore = false - if (!firstStartView) { - firstStartView = { - options, - ignore() { - ignore = true - }, - } - tryStartRum() + const callback = (startRumResult: StartRumResult) => { + startRumResult.startView(options, startClocks) } + bufferApiCalls.add(callback) - bufferApiCalls.add((startRumResult) => { - if (!ignore) { - startRumResult.startView(options, startClocks) - } - }) + if (!firstStartViewCall) { + firstStartViewCall = { options, callback } + tryStartRum() + } }, addAction(action, commonContext = getCommonContext()) { From 3e5576bb3bae052eb3aae1a93274c48bcdff0a29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Thu, 1 Feb 2024 10:26:57 +0100 Subject: [PATCH 8/9] =?UTF-8?q?=F0=9F=91=8C=20refactor=20logic=20to=20remo?= =?UTF-8?q?ve=20an=20item=20from=20an=20array?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/tools/boundedBuffer.ts | 7 +++---- packages/core/src/tools/utils/arrayUtils.ts | 7 +++++++ packages/core/src/tools/valueHistory.ts | 6 ++---- packages/rum/test/mockWorker.ts | 18 ++++++------------ 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/packages/core/src/tools/boundedBuffer.ts b/packages/core/src/tools/boundedBuffer.ts index 7e212e1ccd..2b1d3b7c7f 100644 --- a/packages/core/src/tools/boundedBuffer.ts +++ b/packages/core/src/tools/boundedBuffer.ts @@ -1,3 +1,5 @@ +import { removeItem } from './utils/arrayUtils' + const BUFFER_LIMIT = 500 export class BoundedBuffer { @@ -11,10 +13,7 @@ export class BoundedBuffer { } remove(callback: (arg: T) => void) { - const index = this.buffer.indexOf(callback) - if (index !== -1) { - this.buffer.splice(index, 1) - } + removeItem(this.buffer, callback) } drain(arg: T) { diff --git a/packages/core/src/tools/utils/arrayUtils.ts b/packages/core/src/tools/utils/arrayUtils.ts index 913adc8884..40503fb2dd 100644 --- a/packages/core/src/tools/utils/arrayUtils.ts +++ b/packages/core/src/tools/utils/arrayUtils.ts @@ -5,3 +5,10 @@ export function removeDuplicates(array: T[]) { array.forEach((item) => set.add(item)) return arrayFrom(set) } + +export function removeItem(array: T[], item: T) { + const index = array.indexOf(item) + if (index >= 0) { + array.splice(index, 1) + } +} diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index 6107fa9c21..709058c4a9 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -1,5 +1,6 @@ import { setInterval, clearInterval } from './timer' import type { TimeoutId } from './timer' +import { removeItem } from './utils/arrayUtils' import type { Duration, RelativeTime } from './utils/timeUtils' import { addDuration, relativeNow, ONE_MINUTE } from './utils/timeUtils' @@ -40,10 +41,7 @@ export class ValueHistory { startTime, endTime: END_OF_TIMES, remove: () => { - const index = this.entries.indexOf(entry) - if (index >= 0) { - this.entries.splice(index, 1) - } + removeItem(this.entries, entry) }, close: (endTime: RelativeTime) => { entry.endTime = endTime diff --git a/packages/rum/test/mockWorker.ts b/packages/rum/test/mockWorker.ts index 4a8d41a190..9c94d94b09 100644 --- a/packages/rum/test/mockWorker.ts +++ b/packages/rum/test/mockWorker.ts @@ -13,22 +13,16 @@ export class MockWorker implements DeflateWorker { private streams = new Map() private listeners: { - message: DeflateWorkerListener[] - error: Array<(error: unknown) => void> - } = { message: [], error: [] } + message: Set + error: Set<(error: unknown) => void> + } = { message: new Set(), error: new Set() } addEventListener(eventName: 'message' | 'error', listener: any): void { - const index = this.listeners[eventName].indexOf(listener) - if (index < 0) { - this.listeners[eventName].push(listener) - } + this.listeners[eventName].add(listener) } removeEventListener(eventName: 'message' | 'error', listener: any): void { - const index = this.listeners[eventName].indexOf(listener) - if (index >= 0) { - this.listeners[eventName].splice(index, 1) - } + this.listeners[eventName].delete(listener) } dispatchEvent(): boolean { @@ -49,7 +43,7 @@ export class MockWorker implements DeflateWorker { } get messageListenersCount() { - return this.listeners.message.length + return this.listeners.message.size } processAllMessages(): void { From d4e47b236d3c7d59fab4e1b6cdccd229b2968c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Thu, 1 Feb 2024 10:28:15 +0100 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=91=8C=20fix=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/boot/preStartRum.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index 657b2eb9bf..959bf46a0a 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -54,7 +54,8 @@ export function createPreStartStrategy( // When tracking views automatically, any startView call before RUM start creates an extra // view. // When tracking views manually, we use the ViewOptions from the first startView call as the - // initial view options, and we remove the actual startView call so we don't create + // initial view options, and we remove the actual startView call so we don't create an extra + // view. bufferApiCalls.remove(firstStartViewCall.callback) initialViewOptions = firstStartViewCall.options } @@ -84,9 +85,9 @@ export function createPreStartStrategy( return } - // If we are in a Synthetics test configured to automatically inject a RUM instance, we want to - // completely discard the customer application RUM instance by ignoring their init() call. But, - // we should not ignore the init() call from the Synthetics-injected RUM instance, so the + // If we are in a Synthetics test configured to automatically inject a RUM instance, we want + // to completely discard the customer application RUM instance by ignoring their init() call. + // But, we should not ignore the init() call from the Synthetics-injected RUM instance, so the // internal `ignoreInitIfSyntheticsWillInjectRum` option is here to bypass this condition. if (ignoreInitIfSyntheticsWillInjectRum && willSyntheticsInjectRum()) { return