🐛 [RUMF-1005] Fix dd-request-id endpoint query param (#1018)

* 🐛 Fix dd-request-id endpoint query param
DataDog · Sep 6, 2021 · 5d97167 · 5d97167
1 parent b72928f
commit 5d97167
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 12 deletions.
diff --git a/packages/core/src/domain/configuration/endpointBuilder.ts b/packages/core/src/domain/configuration/endpointBuilder.ts
@@ -1,5 +1,5 @@
 import { BuildEnv } from '../../boot/init'
-import { generateUUID, includes } from '../../tools/utils'
+import { includes } from '../../tools/utils'
 import { InitConfiguration } from './configuration'
 
 export const ENDPOINTS = {
@@ -101,9 +101,8 @@ export function createEndpointBuilder(
     if (shouldUseIntakeV2(endpointType)) {
       parameters +=
         `&dd-api-key=${clientToken}&` +
-        `dd-evp-origin-version=${sdkVersion}&` +
-        `dd-evp-origin=browser&` +
-        `dd-request-id=${generateUUID()}`
+        `dd-evp-origin-version=${encodeURIComponent(sdkVersion)}&` +
+        `dd-evp-origin=browser`
     }
     return parameters
   }

diff --git a/packages/core/src/domain/configuration/transportConfiguration.spec.ts b/packages/core/src/domain/configuration/transportConfiguration.spec.ts
@@ -49,7 +49,7 @@ describe('transportConfiguration', () => {
     it('should add new intake query parameters when "support-intake-v2" enabled', () => {
       const configuration = computeTransportConfiguration({ clientToken, intakeApiVersion: 2 }, buildEnv, true)
       expect(configuration.rumEndpoint).toMatch(
-        `&dd-api-key=${clientToken}&dd-evp-origin-version=(.*)&dd-evp-origin=browser&dd-request-id=(.*)`
+        `&dd-api-key=${clientToken}&dd-evp-origin-version=(.*)&dd-evp-origin=browser`
       )
     })
   })

diff --git a/packages/core/src/domain/internalMonitoring.spec.ts b/packages/core/src/domain/internalMonitoring.spec.ts
@@ -194,7 +194,7 @@ describe('internal monitoring', () => {
       })
 
       expect(server.requests.length).toEqual(1)
-      expect(server.requests[0].url).toEqual(configuration.internalMonitoringEndpoint!)
+      expect(server.requests[0].url).toContain(configuration.internalMonitoringEndpoint!)
 
       expect(JSON.parse(server.requests[0].requestBody)).toEqual({
         date: FAKE_DATE,

diff --git a/packages/core/src/transport/httpRequest.spec.ts b/packages/core/src/transport/httpRequest.spec.ts
@@ -21,7 +21,7 @@ describe('httpRequest', () => {
     request.send('{"foo":"bar1"}\n{"foo":"bar2"}', 10)
 
     expect(server.requests.length).toEqual(1)
-    expect(server.requests[0].url).toEqual(ENDPOINT_URL)
+    expect(server.requests[0].url).toContain(ENDPOINT_URL)
     expect(server.requests[0].requestBody).toEqual('{"foo":"bar1"}\n{"foo":"bar2"}')
   })
 
@@ -39,7 +39,7 @@ describe('httpRequest', () => {
     request.send('{"foo":"bar1"}\n{"foo":"bar2"}', BATCH_BYTES_LIMIT)
 
     expect(server.requests.length).toEqual(1)
-    expect(server.requests[0].url).toEqual(ENDPOINT_URL)
+    expect(server.requests[0].url).toContain(ENDPOINT_URL)
     expect(server.requests[0].requestBody).toEqual('{"foo":"bar1"}\n{"foo":"bar2"}')
   })
 
@@ -63,3 +63,43 @@ describe('httpRequest', () => {
     expect(server.requests.length).toEqual(1)
   })
 })
+
+describe('httpRequest parameters', () => {
+  const BATCH_BYTES_LIMIT = 100
+  let server: sinon.SinonFakeServer
+  let sendBeaconSpy: jasmine.Spy<(url: string, data?: BodyInit | null) => boolean>
+  beforeEach(() => {
+    server = sinon.fakeServer.create()
+    sendBeaconSpy = jasmine.createSpy()
+    navigator.sendBeacon = sendBeaconSpy
+  })
+
+  afterEach(() => {
+    server.restore()
+  })
+
+  it('should add batch_time', () => {
+    const request = new HttpRequest('https://my.website', BATCH_BYTES_LIMIT, true)
+
+    request.send('{"foo":"bar1"}', 10)
+
+    expect(sendBeaconSpy.calls.argsFor(0)[0]).toContain(`&batch_time=`)
+  })
+
+  it('should not add batch_time', () => {
+    const request = new HttpRequest('https://my.website', BATCH_BYTES_LIMIT, false)
+
+    request.send('{"foo":"bar1"}', 10)
+
+    expect(sendBeaconSpy.calls.argsFor(0)[0]).not.toContain(`&batch_time=`)
+  })
+
+  it('should have dd-request-id', () => {
+    const request = new HttpRequest('https://my.website', BATCH_BYTES_LIMIT)
+
+    request.send('{"foo":"bar1"}', 10)
+
+    expect(navigator.sendBeacon).toHaveBeenCalled()
+    expect(server.requests[0].url).toContain(`?dd-request-id=`)
+  })
+})
diff --git a/packages/core/src/transport/httpRequest.ts b/packages/core/src/transport/httpRequest.ts
@@ -1,4 +1,5 @@
 import { monitor, addErrorToMonitoringBatch, addMonitoringMessage } from '../domain/internalMonitoring'
+import { generateUUID, includes } from '../tools/utils'
 
 let hasReportedXhrError = false
 
@@ -14,7 +15,11 @@ export class HttpRequest {
   constructor(private endpointUrl: string, private bytesLimit: number, private withBatchTime: boolean = false) {}
 
   send(data: string | FormData, size: number) {
-    const url = this.withBatchTime ? addBatchTime(this.endpointUrl) : this.endpointUrl
+    let url = addQueryParameter(this.endpointUrl, 'dd-request-id', generateUUID())
+    if (this.withBatchTime) {
+      url = addQueryParameter(url, 'batch_time', new Date().getTime().toString())
+    }
+
     const tryBeacon = !!navigator.sendBeacon && size < this.bytesLimit
     if (tryBeacon) {
       try {
@@ -63,8 +68,8 @@ export class HttpRequest {
   }
 }
 
-function addBatchTime(url: string) {
-  return `${url}${url.indexOf('?') === -1 ? '?' : '&'}batch_time=${new Date().getTime()}`
+function addQueryParameter(url: string, key: string, value: string) {
+  return `${url}${includes(url, '?') ? '&' : '?'}${key}=${value}`
 }
 
 let hasReportedBeaconError = false

diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts
@@ -90,7 +90,7 @@ describe('logs', () => {
       )
 
       expect(server.requests.length).toEqual(1)
-      expect(server.requests[0].url).toEqual(baseConfiguration.logsEndpoint!)
+      expect(server.requests[0].url).toContain(baseConfiguration.logsEndpoint!)
       expect(getLoggedMessage(server, 0)).toEqual({
         date: FAKE_DATE,
         foo: 'bar',