Skip to content

Commit

Permalink
πŸ’₯[RUMF-1554] Drop some deprecated config parameters (#2238)
Browse files Browse the repository at this point in the history
* πŸ’₯ drop proxyUrl

* πŸ’₯ drop sampleRate

* πŸ’₯ drop allowedTracingOrigins

* πŸ’₯ drop tracingSampleRate

* πŸ’₯ drop trackInteractions

* πŸ‘Œ typo

Co-authored-by: Yannick Adam <[email protected]>

* πŸ‘Œremove outdated comment

---------

Co-authored-by: Yannick Adam <[email protected]>
  • Loading branch information
bcaudan and yannickadam authored May 12, 2023
1 parent 356499a commit 503751f
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 292 deletions.
17 changes: 0 additions & 17 deletions packages/core/src/domain/configuration/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,6 @@ describe('validateAndBuildConfiguration', () => {
expect(displaySpy).not.toHaveBeenCalled()
})

it('requires deprecated sampleRate to be a percentage', () => {
expect(
validateAndBuildConfiguration({ clientToken, sampleRate: 'foo' } as unknown as InitConfiguration)
).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Session Sample Rate should be a number between 0 and 100')

displaySpy.calls.reset()
expect(
validateAndBuildConfiguration({ clientToken, sampleRate: 200 } as unknown as InitConfiguration)
).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Session Sample Rate should be a number between 0 and 100')

displaySpy.calls.reset()
validateAndBuildConfiguration({ clientToken: 'yes', sampleRate: 1 })
expect(displaySpy).not.toHaveBeenCalled()
})

it('requires sessionSampleRate to be a percentage', () => {
expect(
validateAndBuildConfiguration({ clientToken, sessionSampleRate: 'foo' } as unknown as InitConfiguration)
Expand Down
18 changes: 4 additions & 14 deletions packages/core/src/domain/configuration/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ export interface InitConfiguration {
// global options
clientToken: string
beforeSend?: GenericBeforeSendCallback | undefined
/**
* @deprecated use sessionSampleRate instead
*/
sampleRate?: number | undefined
sessionSampleRate?: number | undefined
telemetrySampleRate?: number | undefined
silentMultipleInit?: boolean | undefined
Expand All @@ -36,10 +32,6 @@ export interface InitConfiguration {

// transport options
proxy?: string | undefined
/**
* @deprecated use `proxy` instead
*/
proxyUrl?: string | undefined
site?: string | undefined

// tag and context options
Expand Down Expand Up @@ -97,8 +89,7 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
return
}

const sessionSampleRate = initConfiguration.sessionSampleRate ?? initConfiguration.sampleRate
if (sessionSampleRate !== undefined && !isPercentage(sessionSampleRate)) {
if (initConfiguration.sessionSampleRate !== undefined && !isPercentage(initConfiguration.sessionSampleRate)) {
display.error('Session Sample Rate should be a number between 0 and 100')
return
}
Expand Down Expand Up @@ -130,7 +121,7 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
beforeSend:
initConfiguration.beforeSend && catchUserErrors(initConfiguration.beforeSend, 'beforeSend threw an error:'),
cookieOptions: buildCookieOptions(initConfiguration),
sessionSampleRate: sessionSampleRate ?? 100,
sessionSampleRate: initConfiguration.sessionSampleRate ?? 100,
telemetrySampleRate: initConfiguration.telemetrySampleRate ?? 20,
telemetryConfigurationSampleRate: initConfiguration.telemetryConfigurationSampleRate ?? 5,
service: initConfiguration.service,
Expand Down Expand Up @@ -179,15 +170,14 @@ function mustUseSecureCookie(initConfiguration: InitConfiguration) {
}

export function serializeConfiguration(configuration: InitConfiguration): Partial<RawTelemetryConfiguration> {
const proxy = configuration.proxy ?? configuration.proxyUrl
return {
session_sample_rate: configuration.sessionSampleRate ?? configuration.sampleRate,
session_sample_rate: configuration.sessionSampleRate,
telemetry_sample_rate: configuration.telemetrySampleRate,
telemetry_configuration_sample_rate: configuration.telemetryConfigurationSampleRate,
use_before_send: !!configuration.beforeSend,
use_cross_site_session_cookie: configuration.useCrossSiteSessionCookie,
use_secure_session_cookie: configuration.useSecureSessionCookie,
use_proxy: proxy !== undefined ? !!proxy : undefined,
use_proxy: !!configuration.proxy,
silent_multiple_init: configuration.silentMultipleInit,
track_session_across_subdomains: configuration.trackSessionAcrossSubdomains,
track_resources: configuration.trackResources,
Expand Down
40 changes: 0 additions & 40 deletions packages/core/src/domain/configuration/endpointBuilder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,46 +63,6 @@ describe('endpointBuilder', () => {
)
).toBeTrue()
})

it('uses `proxy` over `proxyUrl`', () => {
expect(
createEndpointBuilder(
{ ...initConfiguration, proxy: 'https://proxy.io/path', proxyUrl: 'https://legacy-proxy.io/path' },
'rum',
[]
).build('xhr')
).toMatch(/^https:\/\/proxy.io\/path\?/)

expect(
createEndpointBuilder(
{ ...initConfiguration, proxy: false as any, proxyUrl: 'https://legacy-proxy.io/path' },
'rum',
[]
).build('xhr')
).toMatch(/^https:\/\/rum.browser-intake-datadoghq.com\//)
})
})

describe('deprecated proxyUrl configuration', () => {
it('should replace the full intake endpoint by the proxyUrl and set it in the attribute ddforward', () => {
expect(
createEndpointBuilder({ ...initConfiguration, proxyUrl: 'https://proxy.io/path' }, 'rum', []).build('xhr')
).toMatch(
`https://proxy.io/path\\?ddforward=${encodeURIComponent(
`https://rum.browser-intake-datadoghq.com/api/v2/rum?ddsource=(.*)&ddtags=(.*)&dd-api-key=${clientToken}` +
'&dd-evp-origin-version=(.*)&dd-evp-origin=browser&dd-request-id=(.*)&batch_time=(.*)'
)}`
)
})

it('normalizes the proxy url', () => {
expect(
startsWith(
createEndpointBuilder({ ...initConfiguration, proxyUrl: '/path' }, 'rum', []).build('xhr'),
`${location.origin}/path?ddforward`
)
).toBeTrue()
})
})

describe('tags', () => {
Expand Down
12 changes: 1 addition & 11 deletions packages/core/src/domain/configuration/endpointBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,12 @@ function createEndpointUrlWithParametersBuilder(
endpointType: EndpointType
): (parameters: string) => string {
const path = `/api/v2/${INTAKE_TRACKS[endpointType]}`

const { proxy, proxyUrl } = initConfiguration
const proxy = initConfiguration.proxy
if (proxy) {
const normalizedProxyUrl = normalizeUrl(proxy)
return (parameters) => `${normalizedProxyUrl}?ddforward=${encodeURIComponent(`${path}?${parameters}`)}`
}

const host = buildEndpointHost(initConfiguration, endpointType)

if (proxy === undefined && proxyUrl) {
// TODO: remove this in a future major.
const normalizedProxyUrl = normalizeUrl(proxyUrl)
return (parameters) =>
`${normalizedProxyUrl}?ddforward=${encodeURIComponent(`https://${host}${path}?${parameters}`)}`
}

return (parameters) => `https://${host}${path}?${parameters}`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,44 +97,34 @@ describe('transportConfiguration', () => {
const configuration = computeTransportConfiguration({ clientToken })
expect(configuration.isIntakeUrl('https://www.foo.com')).toBe(false)
})
;[
{
proxyConfigurationName: 'proxy' as const,
intakeUrl: '/api/v2/rum',
},
{
proxyConfigurationName: 'proxyUrl' as const,
intakeUrl: 'https://rum.browser-intake-datadoghq.com/api/v2/rum',
},
].forEach(({ proxyConfigurationName, intakeUrl }) => {
describe(`${proxyConfigurationName} configuration`, () => {
it('should detect proxy intake request', () => {
let configuration = computeTransportConfiguration({
clientToken,
[proxyConfigurationName]: 'https://www.proxy.com',
})
expect(
configuration.isIntakeUrl(`https://www.proxy.com/?ddforward=${encodeURIComponent(`${intakeUrl}?foo=bar`)}`)
).toBe(true)

configuration = computeTransportConfiguration({
clientToken,
[proxyConfigurationName]: 'https://www.proxy.com/custom/path',
})
expect(
configuration.isIntakeUrl(
`https://www.proxy.com/custom/path?ddforward=${encodeURIComponent(`${intakeUrl}?foo=bar`)}`
)
).toBe(true)

describe('proxy configuration', () => {
it('should detect proxy intake request', () => {
let configuration = computeTransportConfiguration({
clientToken,
proxy: 'https://www.proxy.com',
})
expect(
configuration.isIntakeUrl(`https://www.proxy.com/?ddforward=${encodeURIComponent('/api/v2/rum?foo=bar')}`)
).toBe(true)

it('should not detect request done on the same host as the proxy', () => {
const configuration = computeTransportConfiguration({
clientToken,
[proxyConfigurationName]: 'https://www.proxy.com',
})
expect(configuration.isIntakeUrl('https://www.proxy.com/foo')).toBe(false)
configuration = computeTransportConfiguration({
clientToken,
proxy: 'https://www.proxy.com/custom/path',
})
expect(
configuration.isIntakeUrl(
`https://www.proxy.com/custom/path?ddforward=${encodeURIComponent('/api/v2/rum?foo=bar')}`
)
).toBe(true)
})

it('should not detect request done on the same host as the proxy', () => {
const configuration = computeTransportConfiguration({
clientToken,
proxy: 'https://www.proxy.com',
})
expect(configuration.isIntakeUrl('https://www.proxy.com/foo')).toBe(false)
})
})
;[
Expand Down
90 changes: 0 additions & 90 deletions packages/rum-core/src/domain/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,31 +141,6 @@ describe('validateAndBuildRumConfiguration', () => {
})
})

describe('deprecated tracingSampleRate', () => {
it('defaults to undefined if the option is not provided', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBeUndefined()
})

it('is set to provided value', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, tracingSampleRate: 50 })!.traceSampleRate
).toBe(50)
})

it('does not validate the configuration if an incorrect value is provided', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, tracingSampleRate: 'foo' as any })
).toBeUndefined()
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Trace Sample Rate should be a number between 0 and 100')

displayErrorSpy.calls.reset()
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, tracingSampleRate: 200 })
).toBeUndefined()
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Trace Sample Rate should be a number between 0 and 100')
})
})

describe('traceSampleRate', () => {
it('defaults to undefined if the option is not provided', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBeUndefined()
Expand All @@ -189,47 +164,6 @@ describe('validateAndBuildRumConfiguration', () => {
})
})

describe('allowedTracingOrigins', () => {
it('is set to provided value', () => {
expect(
validateAndBuildRumConfiguration({
...DEFAULT_INIT_CONFIGURATION,
allowedTracingOrigins: ['foo'],
service: 'bar',
})!.allowedTracingUrls
).toEqual([{ match: 'foo', propagatorTypes: ['datadog'] }])
})

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

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

expect(typeof tracingUrlOptionMatch).toBe('function')
// Replicating behavior from allowedTracingOrigins, new function will treat the origin part of the URL
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', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, allowedTracingOrigins: ['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, allowedTracingOrigins: 'foo' as any })
).toBeUndefined()
expect(displayErrorSpy).toHaveBeenCalledOnceWith('Allowed Tracing Origins should be an array')
})
})

describe('allowedTracingUrls', () => {
it('defaults to an empty array', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.allowedTracingUrls).toEqual([])
Expand Down Expand Up @@ -344,30 +278,6 @@ describe('validateAndBuildRumConfiguration', () => {
})
})

describe('deprecated trackInteractions', () => {
it('defaults to false', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackUserInteractions).toBeFalse()
})

it('is set to provided value', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackInteractions: true })!
.trackUserInteractions
).toBeTrue()
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackInteractions: false })!
.trackUserInteractions
).toBeFalse()
})

it('the provided value is cast to boolean', () => {
expect(
validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, trackInteractions: 'foo' as any })!
.trackUserInteractions
).toBeTrue()
})
})

describe('trackUserInteractions', () => {
it('defaults to false', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.trackUserInteractions).toBeFalse()
Expand Down
Loading

0 comments on commit 503751f

Please sign in to comment.