Skip to content

Commit

Permalink
✨ [RUMF-1469] introduce a new proxy initialization parameter (#1947)
Browse files Browse the repository at this point in the history 
* ✨ [RUMF-1469] introduce a new `proxy` initialization parameter

* 🐛 use `startsWith` helper to fix IE compatibility in tests

* 👌 [RUMF-1469] redesign how we compute urls

* ♻️ [RUMF-1469] rename buildIntakeUrl() to urlPrefix

This makes things a bit more explicit

* ✅ [RUMF-1469] use the new proxy option in e2e tests and fix them

The new logic to determine whether an URL looks like an intake URL is a
bit more strict, so we need the `ddforward=/api/v2/rum?` URL parameter.
Use a separate parameter to tell that it's coming from the EventBridge

* 👌 simplify ddforward parsing

* 🐛✅ fix e2e intake forwarding to datadog
  • Loading branch information
@BenoitZugmeyer
BenoitZugmeyer authored Feb 8, 2023
1 parent 0e9722d commit 454644d
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 55 deletions.
7 changes: 6 additions & 1 deletion packages/core/src/domain/configuration/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ export interface InitConfiguration {
trackLongTasks?: boolean | undefined

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

Expand Down Expand Up @@ -164,14 +168,15 @@ 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,
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: configuration.proxyUrl !== undefined ? !!configuration.proxyUrl : undefined,
use_proxy: proxy !== undefined ? !!proxy : undefined,
silent_multiple_init: configuration.silentMultipleInit,
track_session_across_subdomains: configuration.trackSessionAcrossSubdomains,
track_resources: configuration.trackResources,
Expand Down
52 changes: 51 additions & 1 deletion packages/core/src/domain/configuration/endpointBuilder.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { BuildEnvWindow } from '../../../test/specHelper'
import { startsWith } from '../../tools/utils'
import type { InitConfiguration } from './configuration'
import { createEndpointBuilder } from './endpointBuilder'

Expand Down Expand Up @@ -36,7 +37,47 @@ describe('endpointBuilder', () => {
})
})

describe('proxyUrl', () => {
describe('proxy configuration', () => {
it('should replace the intake endpoint by the proxy and set the intake path and parameters in the attribute ddforward', () => {
expect(
createEndpointBuilder({ ...initConfiguration, proxy: 'https://proxy.io/path' }, 'rum', []).build('xhr')
).toMatch(
`https://proxy.io/path\\?ddforward=${encodeURIComponent(
`/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, proxy: '/path' }, 'rum', []).build('xhr'),
`${location.origin}/path?ddforward`
)
).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')
Expand All @@ -47,6 +88,15 @@ describe('endpointBuilder', () => {
)}`
)
})

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

describe('tags', () => {
Expand Down
97 changes: 67 additions & 30 deletions packages/core/src/domain/configuration/endpointBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,44 +29,47 @@ export function createEndpointBuilder(
endpointType: EndpointType,
configurationTags: string[]
) {
const { clientToken } = initConfiguration

const host = buildEndpointHost(initConfiguration, endpointType)
const baseUrl = `https://${host}/api/v2/${INTAKE_TRACKS[endpointType]}`
const proxyUrl = initConfiguration.proxyUrl && normalizeUrl(initConfiguration.proxyUrl)
const buildUrlWithParameters = createEndpointUrlWithParametersBuilder(initConfiguration, endpointType)

return {
build(api: 'xhr' | 'fetch' | 'beacon', retry?: RetryInfo) {
const tags = [`sdk_version:${__BUILD_ENV__SDK_VERSION__}`, `api:${api}`].concat(configurationTags)
if (retry) {
tags.push(`retry_count:${retry.count}`, `retry_after:${retry.lastFailureStatus}`)
}
const parameters = [
'ddsource=browser',
`ddtags=${encodeURIComponent(tags.join(','))}`,
`dd-api-key=${clientToken}`,
`dd-evp-origin-version=${encodeURIComponent(__BUILD_ENV__SDK_VERSION__)}`,
'dd-evp-origin=browser',
`dd-request-id=${generateUUID()}`,
]

if (endpointType === 'rum') {
parameters.push(`batch_time=${timeStampNow()}`)
}
if (initConfiguration.internalAnalyticsSubdomain) {
parameters.reverse()
}
const endpointUrl = `${baseUrl}?${parameters.join('&')}`

return proxyUrl ? `${proxyUrl}?ddforward=${encodeURIComponent(endpointUrl)}` : endpointUrl
},
buildIntakeUrl() {
return proxyUrl ? `${proxyUrl}?ddforward` : baseUrl
const parameters = buildEndpointParameters(initConfiguration, endpointType, configurationTags, api, retry)
return buildUrlWithParameters(parameters)
},
urlPrefix: buildUrlWithParameters(''),
endpointType,
}
}

/**
* Create a function used to build a full endpoint url from provided parameters. The goal of this
* function is to pre-compute some parts of the URL to avoid re-computing everything on every
* request, as only parameters are changing.
*/
function createEndpointUrlWithParametersBuilder(
initConfiguration: InitConfiguration,
endpointType: EndpointType
): (parameters: string) => string {
const path = `/api/v2/${INTAKE_TRACKS[endpointType]}`

const { proxy, proxyUrl } = initConfiguration
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}`
}

function buildEndpointHost(initConfiguration: InitConfiguration, endpointType: EndpointType) {
const { site = INTAKE_SITE_US1, internalAnalyticsSubdomain } = initConfiguration

Expand All @@ -79,3 +82,37 @@ function buildEndpointHost(initConfiguration: InitConfiguration, endpointType: E
const subdomain = site !== INTAKE_SITE_AP1 ? `${ENDPOINTS[endpointType]}.` : ''
return `${subdomain}browser-intake-${domainParts.join('-')}.${extension!}`
}

/**
* Build parameters to be used for an intake request. Parameters should be re-built for each
* request, as they change randomly.
*/
function buildEndpointParameters(
{ clientToken, internalAnalyticsSubdomain }: InitConfiguration,
endpointType: EndpointType,
configurationTags: string[],
api: 'xhr' | 'fetch' | 'beacon',
retry: RetryInfo | undefined
) {
const tags = [`sdk_version:${__BUILD_ENV__SDK_VERSION__}`, `api:${api}`].concat(configurationTags)
if (retry) {
tags.push(`retry_count:${retry.count}`, `retry_after:${retry.lastFailureStatus}`)
}
const parameters = [
'ddsource=browser',
`ddtags=${encodeURIComponent(tags.join(','))}`,
`dd-api-key=${clientToken}`,
`dd-evp-origin-version=${encodeURIComponent(__BUILD_ENV__SDK_VERSION__)}`,
'dd-evp-origin=browser',
`dd-request-id=${generateUUID()}`,
]

if (endpointType === 'rum') {
parameters.push(`batch_time=${timeStampNow()}`)
}
if (internalAnalyticsSubdomain) {
parameters.reverse()
}

return parameters.join('&')
}
49 changes: 38 additions & 11 deletions packages/core/src/domain/configuration/transportConfiguration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,45 @@ 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)
})

it('should detect proxy intake request', () => {
let configuration = computeTransportConfiguration({ clientToken, proxyUrl: 'https://www.proxy.com' })
expect(configuration.isIntakeUrl('https://www.proxy.com/?ddforward=xxx')).toBe(true)

configuration = computeTransportConfiguration({ clientToken, proxyUrl: 'https://www.proxy.com/custom/path' })
expect(configuration.isIntakeUrl('https://www.proxy.com/custom/path?ddforward=xxx')).toBe(true)
})

it('should not detect request done on the same host as the proxy', () => {
const configuration = computeTransportConfiguration({ clientToken, proxyUrl: 'https://www.proxy.com' })
expect(configuration.isIntakeUrl('https://www.proxy.com/foo')).toBe(false)
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)
})
})
})
;[
{ site: 'datadoghq.eu' },
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/domain/configuration/transportConfiguration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ export function computeTransportConfiguration(initConfiguration: InitConfigurati
const tags = buildTags(initConfiguration)

const endpointBuilders = computeEndpointBuilders(initConfiguration, tags)
const intakeEndpoints = objectValues(endpointBuilders).map((builder) => builder.buildIntakeUrl())
const intakeUrlPrefixes = objectValues(endpointBuilders).map((builder) => builder.urlPrefix)

const replicaConfiguration = computeReplicaConfiguration(initConfiguration, intakeEndpoints, tags)
const replicaConfiguration = computeReplicaConfiguration(initConfiguration, intakeUrlPrefixes, tags)

return assign(
{
isIntakeUrl: (url: string) => intakeEndpoints.some((intakeEndpoint) => url.indexOf(intakeEndpoint) === 0),
isIntakeUrl: (url: string) => intakeUrlPrefixes.some((intakeEndpoint) => url.indexOf(intakeEndpoint) === 0),
replica: replicaConfiguration,
site: initConfiguration.site || INTAKE_SITE_US1,
},
Expand All @@ -48,7 +48,7 @@ function computeEndpointBuilders(initConfiguration: InitConfiguration, tags: str

function computeReplicaConfiguration(
initConfiguration: InitConfiguration,
intakeEndpoints: string[],
intakeUrlPrefixes: string[],
tags: string[]
): ReplicaConfiguration | undefined {
if (!initConfiguration.replica) {
Expand All @@ -65,7 +65,7 @@ function computeReplicaConfiguration(
rumEndpointBuilder: createEndpointBuilder(replicaConfiguration, 'rum', tags),
}

intakeEndpoints.push(...objectValues(replicaEndpointBuilders).map((builder) => builder.buildIntakeUrl()))
intakeUrlPrefixes.push(...objectValues(replicaEndpointBuilders).map((builder) => builder.urlPrefix))

return assign({ applicationId: initConfiguration.replica.applicationId }, replicaEndpointBuilders)
}
12 changes: 10 additions & 2 deletions test/e2e/lib/framework/pageSetups.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ export function html(parts: readonly string[], ...vars: string[]) {
function setupEventBridge(servers: Servers) {
const baseHostname = new URL(servers.base.url).hostname

// Send EventBridge events to the intake so we can inspect them in our E2E test cases. The URL
// needs to be similar to the normal Datadog intake (through proxy) to make the SDK completely
// ignore them.
const eventBridgeIntake = `${servers.intake.url}/?${new URLSearchParams({
ddforward: '/api/v2/rum?',
bridge: 'true',
}).toString()}`

return html`
<script type="text/javascript">
window.DatadogEventBridge = {
Expand All @@ -187,7 +195,7 @@ function setupEventBridge(servers: Servers) {
send(e) {
const { eventType, event } = JSON.parse(e)
const request = new XMLHttpRequest()
request.open('POST', \`${servers.intake.url}/?ddforward=bridge&event_type=\${eventType}\`, true)
request.open('POST', ${JSON.stringify(eventBridgeIntake)} + '&event_type=' + eventType, true)
request.send(JSON.stringify(event))
},
}
Expand All @@ -199,7 +207,7 @@ function formatConfiguration(initConfiguration: LogsInitConfiguration | RumInitC
let result = JSON.stringify(
{
...initConfiguration,
proxyUrl: servers.intake.url,
proxy: servers.intake.url,
},
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
(key, value) => (key === 'beforeSend' ? 'BEFORE_SEND' : value)
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/lib/framework/serverApps/intake.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function computeIntakeType(
throw new Error('ddforward is missing')
}

if (ddforward === 'bridge') {
if (req.query.bridge === 'true') {
const eventType = req.query.event_type
return {
isBridge: true,
Expand All @@ -53,8 +53,8 @@ function computeIntakeType(
}

let intakeType: IntakeType
const forwardUrl = new URL(ddforward)
const endpoint = forwardUrl.pathname.split('/').pop()
// ddforward = /api/v2/rum?key=value
const endpoint = ddforward.split(/[/?]/)[3]
if (endpoint === 'logs' || endpoint === 'rum') {
intakeType = endpoint
} else if (endpoint === 'replay' && req.busboy) {
Expand Down Expand Up @@ -136,7 +136,7 @@ async function forwardReplayToIntake(req: express.Request): Promise<any> {

function prepareIntakeRequest(req: express.Request) {
const ddforward = req.query.ddforward! as string
if (!/^https:\/\/(session-replay|rum|logs)\.browser-intake-datadoghq\.com\//.test(ddforward)) {
if (!/^\/api\/v2\//.test(ddforward)) {
throw new Error(`Unsupported ddforward: ${ddforward}`)
}
const options = {
Expand All @@ -147,7 +147,7 @@ function prepareIntakeRequest(req: express.Request) {
'User-Agent': req.headers['user-agent'],
},
}
return https.request(ddforward, options)
return https.request(new URL(ddforward, 'https://browser-intake-datadoghq.com'), options)
}

async function readStream(stream: NodeJS.ReadableStream): Promise<Buffer> {
Expand Down

0 comments on commit 454644d

Please sign in to comment.