Skip to content

Commit

Permalink
👌 Rename configureTracingUrls to allowedTracingUrls + Updates followi…
Browse files Browse the repository at this point in the history 
…ng review
  • Loading branch information
@yannickadam
yannickadam committed Dec 9, 2022
1 parent 95c77f0 commit 0074234
Show file tree
Hide file tree
Showing 5 changed files with 228 additions and 189 deletions.
81 changes: 47 additions & 34 deletions packages/rum-core/src/domain/configuration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,22 +173,23 @@ describe('validateAndBuildRumConfiguration', () => {
...DEFAULT_INIT_CONFIGURATION,
allowedTracingOrigins: ['foo'],
service: 'bar',
})!.configureTracingUrls
).toEqual([{ match: 'foo', headerTypes: ['dd'] }])
})!.allowedTracingUrls
).toEqual([{ match: 'foo', headersTypes: ['dd'] }])
})

it('accepts functions', () => {
const customOriginFunction = (origin: string): boolean => origin === 'https://my.origin.com'
const originMatchSpy = jasmine.createSpy<(origin: string) => boolean>()

const func = validateAndBuildRumConfiguration({
const tracingUrlOptionMatch = validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
allowedTracingOrigins: [customOriginFunction],
allowedTracingOrigins: [originMatchSpy],
service: 'bar',
})!.configureTracingUrls[0].match as (url: string) => boolean
})!.allowedTracingUrls[0].match as (url: string) => boolean

expect(typeof func).toBe('function')
expect(typeof tracingUrlOptionMatch).toBe('function')
// Replicating behavior from allowedTracingOrigins, new function will treat the origin part of the URL
expect(func('https://my.origin.com/api')).toEqual(customOriginFunction('https://my.origin.com'))
tracingUrlOptionMatch('https://my.origin.com/api')
expect(originMatchSpy).toHaveBeenCalledWith('https://my.origin.com')
})

it('does not validate the configuration if a value is provided and service is undefined', () => {
Expand All @@ -206,19 +207,19 @@ describe('validateAndBuildRumConfiguration', () => {
})
})

describe('configureTracingUrls', () => {
describe('allowedTracingUrls', () => {
it('defaults to an empty array', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.configureTracingUrls).toEqual([])
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.allowedTracingUrls).toEqual([])
})

it('is set to provided value', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: ['foo'],
allowedTracingUrls: ['foo'],
service: 'bar',
})!.configureTracingUrls
).toEqual([{ match: 'foo', headerTypes: ['dd'] }])
})!.allowedTracingUrls
).toEqual([{ match: 'foo', headersTypes: ['dd'] }])
})

it('accepts functions', () => {
Expand All @@ -227,44 +228,56 @@ describe('validateAndBuildRumConfiguration', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: [customOriginFunction],
allowedTracingUrls: [customOriginFunction],
service: 'bar',
})!.configureTracingUrls
).toEqual([{ match: customOriginFunction, headerTypes: ['dd'] }])
})!.allowedTracingUrls
).toEqual([{ match: customOriginFunction, headersTypes: ['dd'] }])
})

it('accepts RegExp', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: [/az/i],
allowedTracingUrls: [/az/i],
service: 'bar',
})!.configureTracingUrls
).toEqual([{ match: /az/i, headerTypes: ['dd'] }])
})!.allowedTracingUrls
).toEqual([{ match: /az/i, headersTypes: ['dd'] }])
})

it('keeps headers', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: [{ match: 'simple', headerTypes: ['b3m', 'w3c'] }],
allowedTracingUrls: [{ match: 'simple', headersTypes: ['b3m', 'w3c'] }],
service: 'bar',
})!.configureTracingUrls
).toEqual([{ match: 'simple', headerTypes: ['b3m', 'w3c'] }])
})!.allowedTracingUrls
).toEqual([{ match: 'simple', headersTypes: ['b3m', 'w3c'] }])
})

it('should filter out unexpected parameter types', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
service: 'bar',
allowedTracingUrls: [42 as any, undefined, { match: 42 as any, headersTypes: ['dd'] }, { match: 'toto' }],
})!.allowedTracingUrls
).toEqual([])

expect(displayWarnSpy).toHaveBeenCalledTimes(4)
})

it('does not validate the configuration if a value is provided and service is undefined', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, configureTracingUrls: ['foo'] })
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, allowedTracingUrls: ['foo'] })
).toBeUndefined()
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Service needs to be configured when tracing is enabled')
})

it('does not validate the configuration if an incorrect value is provided', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, configureTracingUrls: 'foo' as any })
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, allowedTracingUrls: 'foo' as any })
).toBeUndefined()
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Configure Tracing URLs should be an array')
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Allowed Tracing URLs should be an array')
})
})

Expand Down Expand Up @@ -444,29 +457,29 @@ describe('validateAndBuildRumConfiguration', () => {
})
})

describe('Serialize RUM configuration', () => {
describe('getSelectedTracingHeaders', () => {
describe('serializeRumConfiguration', () => {
describe('selected tracing headers serialization', () => {
it('should not return any header type', () => {
expect(serializeRumConfiguration(DEFAULT_INIT_CONFIGURATION).selected_tracing_headers).toEqual([])
})

it('should return Datadog header type', () => {
const simpleTracingConfig: RumInitConfiguration = {
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: ['foo'],
allowedTracingUrls: ['foo'],
}
expect(serializeRumConfiguration(simpleTracingConfig).selected_tracing_headers).toEqual(['dd'])
})

it('should return Datadog all header types', () => {
it('should return all header types', () => {
const complexTracingConfig: RumInitConfiguration = {
...DEFAULT_INIT_CONFIGURATION,
configureTracingUrls: [
allowedTracingUrls: [
'foo',
{ match: 'first', headerTypes: ['dd'] },
{ match: 'test', headerTypes: ['w3c'] },
{ match: 'other', headerTypes: ['b3'] },
{ match: 'final', headerTypes: ['b3m'] },
{ match: 'first', headersTypes: ['dd'] },
{ match: 'test', headersTypes: ['w3c'] },
{ match: 'other', headersTypes: ['b3'] },
{ match: 'final', headersTypes: ['b3m'] },
],
}
expect(serializeRumConfiguration(complexTracingConfig).selected_tracing_headers).toEqual(
Expand Down
98 changes: 52 additions & 46 deletions packages/rum-core/src/domain/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
} from '@datadog/browser-core'
import type { RumEventDomainContext } from '../domainContext.types'
import type { RumEvent } from '../rumEvent.types'
import type { ConfigureTracingOption, TracingHeadersType } from './tracing/tracer.types'
import { isTracingOption } from './tracing/tracer'
import type { TracingOption, TracingHeadersType } from './tracing/tracer.types'

export interface RumInitConfiguration extends InitConfiguration {
// global options
Expand All @@ -27,10 +28,10 @@ export interface RumInitConfiguration extends InitConfiguration {

// tracing options
/**
* @deprecated use configureTracingUrls instead
* @deprecated use allowedTracingUrls instead
*/
allowedTracingOrigins?: MatchOption[] | undefined
configureTracingUrls?: Array<MatchOption | ConfigureTracingOption> | undefined
allowedTracingUrls?: Array<MatchOption | TracingOption> | undefined
tracingSampleRate?: number | undefined

// replay options
Expand Down Expand Up @@ -59,7 +60,7 @@ export interface RumConfiguration extends Configuration {
// Built from init configuration
actionNameAttribute: string | undefined
tracingSampleRate: number | undefined
configureTracingUrls: ConfigureTracingOption[]
allowedTracingUrls: TracingOption[]
excludedActivityUrls: MatchOption[]
applicationId: string
defaultPrivacyLevel: DefaultPrivacyLevel
Expand Down Expand Up @@ -111,8 +112,8 @@ export function validateAndBuildRumConfiguration(
return
}

const configureTracingUrls = handleTracingParameters(initConfiguration)
if (configureTracingUrls === undefined) {
const allowedTracingUrls = validateAndBuildTracingOptions(initConfiguration)
if (!allowedTracingUrls) {
return
}

Expand All @@ -131,7 +132,7 @@ export function validateAndBuildRumConfiguration(
sessionReplaySampleRate: initConfiguration.sessionReplaySampleRate ?? premiumSampleRate ?? 100,
oldPlansBehavior: initConfiguration.sessionReplaySampleRate === undefined,
tracingSampleRate: initConfiguration.tracingSampleRate,
configureTracingUrls,
allowedTracingUrls,
excludedActivityUrls: initConfiguration.excludedActivityUrls ?? [],
trackInteractions: !!initConfiguration.trackInteractions || trackFrustrations,
trackFrustrations,
Expand All @@ -147,38 +148,44 @@ export function validateAndBuildRumConfiguration(
}

/**
* Handles configureTracingUrls and processes legacy allowedTracingOrigins
* Handles allowedTracingUrls and processes legacy allowedTracingOrigins
*/
function handleTracingParameters(initConfiguration: RumInitConfiguration): ConfigureTracingOption[] | undefined {
function validateAndBuildTracingOptions(initConfiguration: RumInitConfiguration): TracingOption[] | undefined {
// Advise about parameters precedence.
if (initConfiguration.configureTracingUrls !== undefined && initConfiguration.allowedTracingOrigins !== undefined) {
if (initConfiguration.allowedTracingUrls !== undefined && initConfiguration.allowedTracingOrigins !== undefined) {
display.warn(
'Both configureTracingUrls and allowedTracingOrigins (deprecated) have been defined. The parameter configureTracingUrls will override allowedTracingOrigins.'
'Both allowedTracingUrls and allowedTracingOrigins (deprecated) have been defined. The parameter allowedTracingUrls will override allowedTracingOrigins.'
)
}

// Handle configureTracingUrls first
if (initConfiguration.configureTracingUrls !== undefined) {
if (!Array.isArray(initConfiguration.configureTracingUrls)) {
display.error('Configure Tracing URLs should be an array')
// Handle allowedTracingUrls first
if (initConfiguration.allowedTracingUrls !== undefined) {
if (!Array.isArray(initConfiguration.allowedTracingUrls)) {
display.error('Allowed Tracing URLs should be an array')
return
}
if (initConfiguration.configureTracingUrls.length !== 0 && initConfiguration.service === undefined) {
if (initConfiguration.allowedTracingUrls.length !== 0 && initConfiguration.service === undefined) {
display.error('Service needs to be configured when tracing is enabled')
return
}
// Convert from (MatchOption | TracingOption) to TracingOption, remove unknown properties
const tracingOptions: TracingOption[] = []
initConfiguration.allowedTracingUrls.forEach((option) => {
if (isMatchOption(option)) {
tracingOptions.push({ match: option, headersTypes: ['dd'] })
} else if (isTracingOption(option)) {
tracingOptions.push(option)
} else {
display.warn(
'Allowed Tracing Urls parameters should be a string, RegExp, function, or an object. Ignoring parameter',
option
)
}
})

// Convert from MatchOption | TracingOption to TracingOption
const configureTracingUrls: ConfigureTracingOption[] = []
for (const option of initConfiguration.configureTracingUrls) {
configureTracingUrls.push(
isMatchOption(option) ? ({ match: option, headerTypes: ['dd'] } as ConfigureTracingOption) : option
)
}
return configureTracingUrls
return tracingOptions
}

// Handle conversion of allowedTracingOrigins to configureTracingUrls
// Handle conversion of allowedTracingOrigins to allowedTracingUrls
if (initConfiguration.allowedTracingOrigins !== undefined) {
if (!Array.isArray(initConfiguration.allowedTracingOrigins)) {
display.error('Allowed Tracing Origins should be an array')
Expand All @@ -189,31 +196,30 @@ function handleTracingParameters(initConfiguration: RumInitConfiguration): Confi
return
}

return initConfiguration.allowedTracingOrigins.reduce((configArray, item) => {
const option = convertLegacyMatchOptionToTracingOption(item)
if (option) {
configArray.push(option)
const tracingOptions: TracingOption[] = []
initConfiguration.allowedTracingOrigins.forEach((legacyMatchOption) => {
const tracingOption = convertLegacyMatchOptionToTracingOption(legacyMatchOption)
if (tracingOption) {
tracingOptions.push(tracingOption)
}
return configArray
}, [] as ConfigureTracingOption[])
})
return tracingOptions
}

return []
}

/**
* Converts parameters from the deprecated allowedTracingOrigins
* to configureTracingUrls. Handles the change from origin to full URLs.
* to allowedTracingUrls. Handles the change from origin to full URLs.
*/
function convertLegacyMatchOptionToTracingOption(item: MatchOption) {
function convertLegacyMatchOptionToTracingOption(item: MatchOption): TracingOption | undefined {
let match: MatchOption | undefined
if (typeof item === 'string') {
match = item
}
if (item instanceof RegExp) {
} else if (item instanceof RegExp) {
match = (url) => item.test(getOrigin(url))
}
if (typeof item === 'function') {
} else if (typeof item === 'function') {
match = (url) => item(getOrigin(url))
}

Expand All @@ -222,22 +228,22 @@ function convertLegacyMatchOptionToTracingOption(item: MatchOption) {
return undefined
}

return { match, headerTypes: ['dd'] } as ConfigureTracingOption
return { match, headersTypes: ['dd'] }
}

/**
* Combines the selected tracing headers from the different options in configureTracingUrls,
* Combines the selected tracing headers from the different options in allowedTracingUrls,
* and assumes 'dd' has been selected when using allowedTracingOrigins
*/
function getSelectedTracingHeaders(configuration: RumInitConfiguration): TracingHeadersType[] {
const usedTracingHeaders = new Set<TracingHeadersType>()

if (Array.isArray(configuration.configureTracingUrls) && configuration.configureTracingUrls.length > 0) {
configuration.configureTracingUrls.forEach((config) => {
if (Array.isArray(configuration.allowedTracingUrls) && configuration.allowedTracingUrls.length > 0) {
configuration.allowedTracingUrls.forEach((config) => {
if (isMatchOption(config)) {
usedTracingHeaders.add('dd')
} else if (Array.isArray(config.headerTypes)) {
config.headerTypes.forEach((headerType) => usedTracingHeaders.add(headerType))
} else {
config.headersTypes.forEach((headerType) => usedTracingHeaders.add(headerType))
}
})
}
Expand All @@ -261,8 +267,8 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration):
action_name_attribute: configuration.actionNameAttribute,
use_allowed_tracing_origins:
Array.isArray(configuration.allowedTracingOrigins) && configuration.allowedTracingOrigins.length > 0,
use_configure_tracing_urls:
Array.isArray(configuration.configureTracingUrls) && configuration.configureTracingUrls.length > 0,
use_allowed_tracing_urls:
Array.isArray(configuration.allowedTracingUrls) && configuration.allowedTracingUrls.length > 0,
selected_tracing_headers: getSelectedTracingHeaders(configuration),
default_privacy_level: configuration.defaultPrivacyLevel,
use_excluded_activity_urls:
Expand Down
Loading

0 comments on commit 0074234

Please sign in to comment.