Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

💥[RUMF-1554] Drop some deprecated config parameters #2238

Merged
merged 8 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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