From 4d666f7d40d7e81b2a3a94989694f0a2145a690b Mon Sep 17 00:00:00 2001 From: Gitlab staging reset job Date: Fri, 25 Feb 2022 12:00:22 +0100 Subject: [PATCH] Add new forwardReports config option --- packages/core/test/specHelper.ts | 20 +++-- packages/logs/src/boot/startLogs.spec.ts | 2 +- packages/logs/src/boot/startLogs.ts | 2 +- .../logs/src/domain/configuration.spec.ts | 86 ++++++++----------- packages/logs/src/domain/configuration.ts | 63 ++++++++------ 5 files changed, 93 insertions(+), 80 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 06c2742960..83fecdec90 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -1,5 +1,5 @@ import type { EndpointBuilder } from '../src/domain/configuration' -import type { ReportType, BrowserWindow, Report } from '../src/domain/report/browser.types' +import type { ReportType, BrowserWindow, Report, ReportingObserverOption } from '../src/domain/report/browser.types' import { instrumentMethod } from '../src/tools/instrumentMethod' import { resetNavigationStart } from '../src/tools/timeUtils' import { buildUrl } from '../src/tools/urlPolyfill' @@ -415,10 +415,18 @@ export function stubCookie() { export function stubReportingObserver() { const originalReportingObserver = (window as BrowserWindow).ReportingObserver - let callbacks: Array<(reports: Report[]) => void> = [] + let callbacks: { [k in ReportType]?: Array<(reports: Report[]) => void> } = {} + + ;(window as BrowserWindow).ReportingObserver = function ( + callback: (reports: Report[]) => void, + { types }: ReportingObserverOption + ) { + types.forEach((type) => { + if (!callbacks[type]) callbacks[type] = [] + + callbacks[type]?.push(callback) + }) - ;(window as BrowserWindow).ReportingObserver = function (callback: (reports: Report[]) => void) { - callbacks.push(callback) return { disconnect() { noop() @@ -434,11 +442,11 @@ export function stubReportingObserver() { return { raiseReport(type: ReportType) { - callbacks.forEach((callback) => callback([{ ...FAKE_REPORT, type }])) + if (callbacks[type]) callbacks[type]!.forEach((callback) => callback([{ ...FAKE_REPORT, type }])) }, reset() { ;(window as BrowserWindow).ReportingObserver = originalReportingObserver - callbacks = [] + callbacks = {} }, } } diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index ea3a890ff5..40e7d2bc9a 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -216,7 +216,7 @@ describe('logs', () => { const logErrorSpy = spyOn(logger, 'log') const reportingObserverStub = stubReportingObserver() updateExperimentalFeatures(['forward-reports']) - originalStartLogs({ ...baseConfiguration }, logger) + originalStartLogs({ ...baseConfiguration, forwardReports: ['intervention'] }, logger) reportingObserverStub.raiseReport('intervention') diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index 3fcc167714..3e4d637052 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -56,7 +56,7 @@ export function startLogs(configuration: LogsConfiguration, logger: Logger) { trackNetworkError(configuration, rawErrorObservable) } const consoleObservable = initConsoleObservable(configuration.forwardConsoleLogs) - const reportObservable = initReportObservable(['intervention', 'deprecation', 'csp_violation']) + const reportObservable = initReportObservable(configuration.forwardReports) const session = areCookiesAuthorized(configuration.cookieOptions) && !canUseEventBridge() diff --git a/packages/logs/src/domain/configuration.spec.ts b/packages/logs/src/domain/configuration.spec.ts index 9bbb4bb310..8eba15b902 100644 --- a/packages/logs/src/domain/configuration.spec.ts +++ b/packages/logs/src/domain/configuration.spec.ts @@ -1,5 +1,5 @@ -import { ConsoleApiName, display, resetExperimentalFeatures, updateExperimentalFeatures } from '@datadog/browser-core' -import { validateAndBuildLogsConfiguration } from './configuration' +import { display, resetExperimentalFeatures, updateExperimentalFeatures } from '@datadog/browser-core' +import { validateAndBuildForwardOption, validateAndBuildLogsConfiguration } from './configuration' const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' } @@ -24,14 +24,37 @@ describe('validateAndBuildLogsConfiguration', () => { }) }) - describe('forwardConsoleLogs if ff forward-logs enabled', () => { - let displaySpy: jasmine.Spy - const errorMessage = - 'Forward Console Logs should be "all" or an array with allowed values "log", "debug", "info", "warn", "error"' + describe('forwardConsoleLogs', () => { + it('contains "error" when forwardErrorsToLogs is enabled', () => { + expect( + validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: true })! + .forwardConsoleLogs + ).toEqual(['error']) + }) + it('contains "error" once when both forwardErrorsToLogs and forwardConsoleLogs are enabled', () => { + expect( + validateAndBuildLogsConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + forwardConsoleLogs: ['error'], + forwardErrorsToLogs: true, + })!.forwardConsoleLogs + ).toEqual(['error']) + }) + }) +}) + +describe('validateAndBuildForwardOption', () => { + let displaySpy: jasmine.Spy + const allowedValues = ['foo', 'bar'] + const label = 'Label' + const ff = 'flag' + const errorMessage = 'Label should be "all" or an array with allowed values "foo", "bar"' + + describe(`if ff enabled`, () => { beforeEach(() => { displaySpy = spyOn(display, 'error') - updateExperimentalFeatures(['forward-logs']) + updateExperimentalFeatures([ff]) }) afterEach(() => { @@ -39,66 +62,33 @@ describe('validateAndBuildLogsConfiguration', () => { }) it('does not validate the configuration if an incorrect string is provided', () => { - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: 'foo' as any }) + validateAndBuildForwardOption('foo' as any, allowedValues, label, ff) expect(displaySpy).toHaveBeenCalledOnceWith(errorMessage) }) it('does not validate the configuration if an incorrect api is provided', () => { - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: ['dir'] as any }) + validateAndBuildForwardOption(['dir'], allowedValues, label, ff) expect(displaySpy).toHaveBeenCalledOnceWith(errorMessage) }) it('defaults to an empty array', () => { - expect(validateAndBuildLogsConfiguration(DEFAULT_INIT_CONFIGURATION)!.forwardConsoleLogs).toEqual([]) + expect(validateAndBuildForwardOption(undefined, allowedValues, label, ff)).toEqual([]) }) it('is set to provided value', () => { - expect( - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: ['log'] })! - .forwardConsoleLogs - ).toEqual(['log']) + expect(validateAndBuildForwardOption(['log'], allowedValues, label, ff)).toEqual(['log']) }) - it('contains "error" when forwardErrorsToLogs is enabled', () => { - expect( - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: true })! - .forwardConsoleLogs - ).toEqual(['error']) - }) - - it('contains all apis when forwardConsoleLogs is set to "all"', () => { - expect( - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: 'all' })! - .forwardConsoleLogs - ).toEqual(Object.keys(ConsoleApiName) as ConsoleApiName[]) - }) - - it('contains "error" once when both forwardErrorsToLogs and forwardConsoleLogs are enabled', () => { - expect( - validateAndBuildLogsConfiguration({ - ...DEFAULT_INIT_CONFIGURATION, - forwardConsoleLogs: ['error'], - forwardErrorsToLogs: true, - })!.forwardConsoleLogs - ).toEqual(['error']) + it('contains all options when "all" is provided', () => { + expect(validateAndBuildForwardOption('all', allowedValues, label, ff)).toEqual(allowedValues) }) }) - describe('forwardConsoleLogs if ff forward-logs disabled', () => { + describe('if ff disabled', () => { it('should be set to empty array', () => { - expect( - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardConsoleLogs: ['log'] })! - .forwardConsoleLogs - ).toEqual([]) - }) - - it('contains "error" when forwardErrorsToLogs is enabled', () => { - expect( - validateAndBuildLogsConfiguration({ ...DEFAULT_INIT_CONFIGURATION, forwardErrorsToLogs: true })! - .forwardConsoleLogs - ).toEqual(['error']) + expect(validateAndBuildForwardOption(['log'], allowedValues, label, ff)).toEqual([]) }) }) }) diff --git a/packages/logs/src/domain/configuration.ts b/packages/logs/src/domain/configuration.ts index 5e3fec389d..f216e3537e 100644 --- a/packages/logs/src/domain/configuration.ts +++ b/packages/logs/src/domain/configuration.ts @@ -7,15 +7,17 @@ import { removeDuplicates, ConsoleApiName, objectHasValue, + CustomReportType, + includes, } from '@datadog/browser-core' import { buildEnv } from '../boot/buildEnv' import type { LogsEvent } from '../logsEvent.types' -import type { StatusType } from './logger' export interface LogsInitConfiguration extends InitConfiguration { beforeSend?: ((event: LogsEvent) => void | boolean) | undefined forwardErrorsToLogs?: boolean | undefined forwardConsoleLogs?: readonly ConsoleApiName[] | 'all' | undefined + forwardReports?: readonly CustomReportType[] | 'all' | undefined } export type HybridInitConfiguration = Omit @@ -23,6 +25,7 @@ export type HybridInitConfiguration = Omit export interface LogsConfiguration extends Configuration { forwardErrorsToLogs: boolean forwardConsoleLogs: ConsoleApiName[] + forwardReports: CustomReportType[] requestErrorResponseLengthLimit: number } @@ -35,40 +38,52 @@ export function validateAndBuildLogsConfiguration( initConfiguration: LogsInitConfiguration ): LogsConfiguration | undefined { const baseConfiguration = validateAndBuildConfiguration(initConfiguration, buildEnv) - if (!baseConfiguration) { - return - } - let forwardConsoleLogs: StatusType[] = [] - if (isExperimentalFeatureEnabled('forward-logs') && initConfiguration.forwardConsoleLogs !== undefined) { - const allowedConsoleApis = Object.keys(ConsoleApiName) as ConsoleApiName[] + const forwardConsoleLogs = validateAndBuildForwardOption( + initConfiguration.forwardConsoleLogs, + Object.keys(ConsoleApiName) as ConsoleApiName[], + 'Forward Console Logs', + 'forward-logs' + ) - if ( - !( - initConfiguration.forwardConsoleLogs === 'all' || - (Array.isArray(initConfiguration.forwardConsoleLogs) && - initConfiguration.forwardConsoleLogs.every((api) => objectHasValue(ConsoleApiName, api))) - ) - ) { - display.error( - `Forward Console Logs should be "all" or an array with allowed values "${allowedConsoleApis.join('", "')}"` - ) - return - } + const forwardReports = validateAndBuildForwardOption( + initConfiguration.forwardReports, + Object.keys(CustomReportType) as CustomReportType[], + 'Forward Reports', + 'forward-reports' + ) - forwardConsoleLogs = - initConfiguration.forwardConsoleLogs === 'all' ? allowedConsoleApis : initConfiguration.forwardConsoleLogs + if (!baseConfiguration || !forwardConsoleLogs || !forwardReports) { + return } - if (initConfiguration.forwardErrorsToLogs) { + if (initConfiguration.forwardErrorsToLogs && !includes(forwardConsoleLogs, ConsoleApiName.error)) { forwardConsoleLogs.push(ConsoleApiName.error) } return { ...baseConfiguration, - forwardErrorsToLogs: !!initConfiguration.forwardErrorsToLogs, - forwardConsoleLogs: removeDuplicates(forwardConsoleLogs), + forwardConsoleLogs, + forwardReports, requestErrorResponseLengthLimit: DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT, } } + +export function validateAndBuildForwardOption( + option: readonly T[] | 'all' | undefined, + allowedValues: T[], + label: string, + featureFlag: string +): T[] | undefined { + if (!isExperimentalFeatureEnabled(featureFlag) || option === undefined) { + return [] + } + + if (!(option === 'all' || (Array.isArray(option) && option.every((api) => objectHasValue(ConsoleApiName, api))))) { + display.error(`${label} should be "all" or an array with allowed values "${allowedValues.join('", "')}"`) + return + } + + return option === 'all' ? allowedValues : removeDuplicates(option) +}