Skip to content

Commit

Permalink
Add new forwardReports config option
Browse files Browse the repository at this point in the history
  • Loading branch information
amortemousque committed Feb 25, 2022
1 parent 37582bb commit 4d666f7
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 80 deletions.
20 changes: 14 additions & 6 deletions packages/core/test/specHelper.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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()
Expand All @@ -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 = {}
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/logs/src/boot/startLogs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
2 changes: 1 addition & 1 deletion packages/logs/src/boot/startLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
86 changes: 38 additions & 48 deletions packages/logs/src/domain/configuration.spec.ts
Original file line number Diff line number Diff line change
@@ -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' }

Expand All @@ -24,81 +24,71 @@ describe('validateAndBuildLogsConfiguration', () => {
})
})

describe('forwardConsoleLogs if ff forward-logs enabled', () => {
let displaySpy: jasmine.Spy<typeof display.error>
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<typeof display.error>
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(() => {
resetExperimentalFeatures()
})

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([])
})
})
})
63 changes: 39 additions & 24 deletions packages/logs/src/domain/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,25 @@ 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<LogsInitConfiguration, 'clientToken'>

export interface LogsConfiguration extends Configuration {
forwardErrorsToLogs: boolean
forwardConsoleLogs: ConsoleApiName[]
forwardReports: CustomReportType[]
requestErrorResponseLengthLimit: number
}

Expand All @@ -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<ConsoleApiName>(
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<CustomReportType>(
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<T>(
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<T>(option)
}

0 comments on commit 4d666f7

Please sign in to comment.