🐛[RUMF-330] fix intake requests exclusion (#281)

* fix wrongly placed spec * 🐛[RUMF-330] fix intake requests exclusion when several SDKs are setup on the same page, client tokens can differ. * ✅ setup different domain for test app and test intake * 👌 improve same origin check
DataDog · Feb 26, 2020 · 49612b9 · 49612b9
1 parent a80297b
commit 49612b9
Show file tree

Hide file tree

Showing 11 changed files with 95 additions and 63 deletions.
diff --git a/packages/core/src/specHelper.ts b/packages/core/src/specHelper.ts
@@ -1,7 +1,15 @@
+import { Configuration } from './configuration'
 import { Observable } from './observable'
 import { RequestDetails } from './requestCollection'
 import { noop } from './utils'
 
+export const SPEC_ENDPOINTS: Partial<Configuration> = {
+  internalMonitoringEndpoint: 'https://monitoring-intake.com/abcde?foo=bar',
+  logsEndpoint: 'https://logs-intake.com/abcde?foo=bar',
+  rumEndpoint: 'https://rum-intake.com/abcde?foo=bar',
+  traceEndpoint: 'https://trace-intake.com/abcde?foo=bar',
+}
+
 export function isSafari() {
   return /^((?!chrome|android).)*safari/i.test(navigator.userAgent)
 }

diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts
@@ -171,10 +171,6 @@ function hasToJSON(value: unknown): value is ObjectWithToJSON {
   return typeof value === 'object' && value !== null && value.hasOwnProperty('toJSON')
 }
 
-export function startsWith(candidate: string, search: string) {
-  return candidate.indexOf(search) === 0
-}
-
 export function includes(candidate: unknown[], search: unknown) {
   return candidate.indexOf(search) !== -1
 }

diff --git a/packages/rum/src/resourceUtils.ts b/packages/rum/src/resourceUtils.ts
@@ -1,4 +1,4 @@
-import { addMonitoringMessage, Configuration, includes, msToNs, ResourceKind, startsWith } from '@datadog/browser-core'
+import { addMonitoringMessage, Configuration, includes, msToNs, ResourceKind } from '@datadog/browser-core'
 
 import { PerformanceResourceDetails } from './rum'
 
@@ -90,9 +90,13 @@ export function isValidResource(url: string, configuration: Configuration) {
 
 function isBrowserAgentRequest(url: string, configuration: Configuration) {
   return (
-    startsWith(url, configuration.logsEndpoint) ||
-    startsWith(url, configuration.rumEndpoint) ||
-    startsWith(url, configuration.traceEndpoint) ||
-    (configuration.internalMonitoringEndpoint && startsWith(url, configuration.internalMonitoringEndpoint))
+    haveSameOrigin(url, configuration.logsEndpoint) ||
+    haveSameOrigin(url, configuration.rumEndpoint) ||
+    haveSameOrigin(url, configuration.traceEndpoint) ||
+    (configuration.internalMonitoringEndpoint && haveSameOrigin(url, configuration.internalMonitoringEndpoint))
   )
 }
+
+function haveSameOrigin(url1: string, url2: string) {
+  return new URL(url1).origin === new URL(url2).origin
+}
diff --git a/packages/rum/test/resourceUtils.spec.ts b/packages/rum/test/resourceUtils.spec.ts
@@ -1,4 +1,5 @@
-import { computePerformanceResourceDetails } from '../src/resourceUtils'
+import { Configuration, DEFAULT_CONFIGURATION, SPEC_ENDPOINTS } from '@datadog/browser-core'
+import { computePerformanceResourceDetails, isValidResource } from '../src/resourceUtils'
 
 function generateResourceWith(overrides: Partial<PerformanceResourceTiming>) {
   const completeTiming: Partial<PerformanceResourceTiming> = {
@@ -105,30 +106,49 @@ describe('computePerformanceResourceDetails', () => {
       expect(computePerformanceResourceDetails(generateResourceWith(overrides))).toBeUndefined()
     })
   })
+
+  it('should allow really fast document resource', () => {
+    expect(
+      computePerformanceResourceDetails(
+        generateResourceWith({
+          connectEnd: 0,
+          connectStart: 0,
+          domainLookupEnd: 0,
+          domainLookupStart: 0,
+          redirectEnd: 0,
+          redirectStart: 0,
+          requestStart: 0,
+          responseEnd: 50,
+          responseStart: 40,
+          secureConnectionStart: 0,
+        })
+      )
+    ).toEqual({
+      connect: { start: 0, duration: 0 },
+      dns: { start: 0, duration: 0 },
+      download: { start: 40e6, duration: 10e6 },
+      firstByte: { start: 0, duration: 40e6 },
+      redirect: undefined,
+      ssl: undefined,
+    })
+  })
 })
 
-it('should allow really fast document resource', () => {
-  expect(
-    computePerformanceResourceDetails(
-      generateResourceWith({
-        connectEnd: 0,
-        connectStart: 0,
-        domainLookupEnd: 0,
-        domainLookupStart: 0,
-        redirectEnd: 0,
-        redirectStart: 0,
-        requestStart: 0,
-        responseEnd: 50,
-        responseStart: 40,
-        secureConnectionStart: 0,
-      })
-    )
-  ).toEqual({
-    connect: { start: 0, duration: 0 },
-    dns: { start: 0, duration: 0 },
-    download: { start: 40e6, duration: 10e6 },
-    firstByte: { start: 0, duration: 40e6 },
-    redirect: undefined,
-    ssl: undefined,
+describe('isValidRequest', () => {
+  const configuration: Partial<Configuration> = {
+    ...DEFAULT_CONFIGURATION,
+    ...SPEC_ENDPOINTS,
+  }
+
+  it('should exclude requests on intakes endpoints', () => {
+    expect(isValidResource('https://rum-intake.com/abcde?foo=bar', configuration as Configuration)).toBe(false)
+  })
+
+  it('should exclude requests on intakes endpoints with different client parameters', () => {
+    expect(isValidResource('https://rum-intake.com/wxyz?foo=qux', configuration as Configuration)).toBe(false)
+  })
+
+  it('should allow requests on non intake domains', () => {
+    expect(isValidResource('https://my-domain.com/hello?a=b', configuration as Configuration)).toBe(true)
   })
 })
diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts
@@ -7,6 +7,7 @@ import {
   Omit,
   PerformanceObserverStubBuilder,
   RequestDetails,
+  SPEC_ENDPOINTS,
 } from '@datadog/browser-core'
 import sinon from 'sinon'
 
@@ -29,11 +30,8 @@ function getServerRequestBodies<T>(server: sinon.SinonFakeServer) {
 
 const configuration = {
   ...DEFAULT_CONFIGURATION,
-  internalMonitoringEndpoint: 'monitoring',
-  logsEndpoint: 'logs',
+  ...SPEC_ENDPOINTS,
   maxBatchSize: 1,
-  rumEndpoint: 'rum',
-  traceEndpoint: 'trace',
 }
 
 const internalMonitoring: InternalMonitoring = {
@@ -72,7 +70,7 @@ describe('rum handle performance entry', () => {
     },
     {
       description: 'type resource + valid request',
-      entry: { entryType: 'resource', name: 'valid' },
+      entry: { entryType: 'resource', name: 'https://resource.com/valid' },
       expectEntryToBeAdded: true,
     },
   ].forEach(

diff --git a/test/app/app.ts b/test/app/app.ts
@@ -2,20 +2,21 @@ import { datadogLogs } from '@datadog/browser-logs'
 import { datadogRum } from '@datadog/browser-rum'
 
 // fallback for server side rendering
-const origin = typeof location === 'object' ? location.origin : ''
+const hostname = typeof location === 'object' ? location.hostname : ''
+const intakeOrigin = `http://${hostname}:4000`
 
 datadogLogs.init({
   clientToken: 'key',
   forwardErrorsToLogs: true,
-  internalMonitoringEndpoint: `${origin}/monitoring`,
-  logsEndpoint: `${origin}/logs`,
-  rumEndpoint: `${origin}/rum`,
+  internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+  logsEndpoint: `${intakeOrigin}/logs`,
+  rumEndpoint: `${intakeOrigin}/rum`,
 })
 
 datadogRum.init({
   applicationId: 'rum',
   clientToken: 'key',
-  internalMonitoringEndpoint: `${origin}/monitoring`,
-  logsEndpoint: `${origin}/logs`,
-  rumEndpoint: `${origin}/rum`,
+  internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+  logsEndpoint: `${intakeOrigin}/logs`,
+  rumEndpoint: `${intakeOrigin}/rum`,
 })
diff --git a/test/e2e/scenario/helpers.ts b/test/e2e/scenario/helpers.ts
@@ -25,7 +25,7 @@ export interface ServerRumViewEvent extends RumViewEvent {
   }
 }
 
-const baseRequest = request.defaults({ baseUrl: 'http://localhost:3000' })
+const intakeRequest = request.defaults({ baseUrl: 'http://localhost:4000' })
 
 export async function flushEvents() {
   // wait to process event loop before switching page
@@ -97,7 +97,7 @@ export async function resetServerState() {
 
 async function fetch(url: string): Promise<string> {
   return new Promise((resolve, reject) => {
-    baseRequest.get(url, (err: any, response: any, body: string) => {
+    intakeRequest.get(url, (err: any, response: any, body: string) => {
       if (err) {
         reject(err)
       }

diff --git a/test/e2e/wdio.base.conf.js b/test/e2e/wdio.base.conf.js
@@ -1,5 +1,6 @@
 const { exec } = require('child_process')
-let serverProcess
+let appProcess
+let intakeProcess
 
 module.exports = {
   runner: 'local',
@@ -16,7 +17,8 @@ module.exports = {
   },
   e2eMode: process.env.E2E_MODE || 'bundle',
   onPrepare: function() {
-    serverProcess = exec('node test/server/server')
+    appProcess = exec('PORT=3000 node test/server/server')
+    intakeProcess = exec('PORT=4000 node test/server/server')
   },
   before: function() {
     require('ts-node').register({
@@ -25,6 +27,7 @@ module.exports = {
     })
   },
   onComplete: function() {
-    serverProcess.kill()
+    appProcess.kill()
+    intakeProcess.kill()
   },
 }
diff --git a/test/server/server.js b/test/server/server.js
@@ -12,7 +12,7 @@ const rumConfig = require('../../packages/rum/webpack.config')
 const fakeBackend = require('./fake-backend')
 const buildEnv = require('../../scripts/build-env')
 
-let port = 3000
+let port = process.env.PORT || 3000
 
 morgan.token('body', (req, res) => extractBody(req, res))
 const stream = fs.createWriteStream(path.join(__dirname, 'test-server.log'))

diff --git a/test/static/async-e2e-page.html b/test/static/async-e2e-page.html
@@ -10,15 +10,16 @@
     <button>click me</button>
     <script type="text/javascript">
       window.addEventListener('load', () => {
+        const intakeOrigin = `http://${window.location.hostname}:4000`
         const logs = document.createElement('script')
         logs.src = './datadog-logs-us.js'
         logs.onload = () => {
           window.DD_LOGS &&
             window.DD_LOGS.init({
               clientToken: 'key',
-              internalMonitoringEndpoint: `${window.location.origin}/monitoring`,
-              logsEndpoint: `${window.location.origin}/logs`,
-              rumEndpoint: `${window.location.origin}/rum`,
+              internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+              logsEndpoint: `${intakeOrigin}/logs`,
+              rumEndpoint: `${intakeOrigin}/rum`,
               forwardErrorsToLogs: true,
             })
         }
@@ -31,9 +32,9 @@
             window.DD_RUM.init({
               applicationId: 'rum',
               clientToken: 'key',
-              internalMonitoringEndpoint: `${window.location.origin}/monitoring`,
-              logsEndpoint: `${window.location.origin}/logs`,
-              rumEndpoint: `${window.location.origin}/rum`,
+              internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+              logsEndpoint: `${intakeOrigin}/logs`,
+              rumEndpoint: `${intakeOrigin}/rum`,
             })
         }
         document.getElementsByTagName('head')[0].appendChild(rum)

diff --git a/test/static/bundle-e2e-page.html b/test/static/bundle-e2e-page.html
@@ -7,12 +7,13 @@
     <title>bundle tests page</title>
     <script type="text/javascript" src="./datadog-logs-us.js"></script>
     <script type="text/javascript">
+      const intakeOrigin = `http://${window.location.hostname}:4000`
       window.DD_LOGS &&
         window.DD_LOGS.init({
           clientToken: 'key',
-          internalMonitoringEndpoint: `${window.location.origin}/monitoring`,
-          logsEndpoint: `${window.location.origin}/logs`,
-          rumEndpoint: `${window.location.origin}/rum`,
+          internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+          logsEndpoint: `${intakeOrigin}/logs`,
+          rumEndpoint: `${intakeOrigin}/rum`,
           forwardErrorsToLogs: true,
         })
     </script>
@@ -22,9 +23,9 @@
         window.DD_RUM.init({
           applicationId: 'rum',
           clientToken: 'key',
-          internalMonitoringEndpoint: `${window.location.origin}/monitoring`,
-          logsEndpoint: `${window.location.origin}/logs`,
-          rumEndpoint: `${window.location.origin}/rum`,
+          internalMonitoringEndpoint: `${intakeOrigin}/monitoring`,
+          logsEndpoint: `${intakeOrigin}/logs`,
+          rumEndpoint: `${intakeOrigin}/rum`,
         })
     </script>
   </head>