chore: modify the logic as per review

packages/rum-core/src/common/patching/index.js

@@ -28,12 +28,13 @@ import { patchFetch } from './fetch-patch'
 import { patchHistory } from './history-patch'
 import { patchEventTarget } from './event-target-patch'
 import EventHandler from '../event-handler'
-
+import { now } from '../utils'
 import { HISTORY, FETCH, XMLHTTPREQUEST, EVENT_TARGET } from '../constants'
 
 const patchEventHandler = new EventHandler()
-
+let patchedTime = null
 let alreadyPatched = false
+
 function patchAll() {
   if (!alreadyPatched) {
     alreadyPatched = true
@@ -43,6 +44,7 @@ function patchAll() {
     patchFetch(function(event, task) {
       patchEventHandler.send(FETCH, [event, task])
     })
+    patchedTime = now()
     patchHistory(function(event, task) {
       patchEventHandler.send(HISTORY, [event, task])
     })
@@ -54,4 +56,4 @@ function patchAll() {
   return patchEventHandler
 }
 
-export { patchAll, patchEventHandler }
+export { patchAll, patchEventHandler, patchedTime }

packages/rum-core/src/performance-monitoring/capture-navigation.js

@@ -37,6 +37,7 @@ import {
   PERF,
   isPerfTimelineSupported
 } from '../common/utils'
+import * as patch from '../common/patching'
 
 /**
  * Navigation Timing Spans
@@ -119,36 +120,13 @@ function createResourceTimingSpan(resourceTimingEntry) {
   return span
 }
 
-function isBetween(spanStart, resourceStart) {
-  /**
-   * Delta used for comparing the timings between patched
-   * API span timing and ResourceTiming data
-   */
-  const deltaTimeInMs = 2
-
-  return (
-    spanStart >= resourceStart - deltaTimeInMs &&
-    spanStart <= resourceStart + deltaTimeInMs
-  )
-}
-
 /**
  * Checks if the span is already captured via XHR/Fetch patch by
- * comparing the given resource URL aganist the list of span urls
- * including the query parameters
+ * comparing the given resource startTime(fetchStart) aganist the
+ * patch code execution time.
  */
-function isAlreadyCaptured(apiSpans, entry) {
-  let captured = false
-
-  for (let i = 0; i < apiSpans.length; i++) {
-    const { url, start } = apiSpans[i]
-
-    if (url === entry.name && isBetween(start, entry.startTime)) {
-      captured = true
-      break
-    }
-  }
-  return captured
+function isCapturedByPatching(resourceStartTime, requestPatchTime) {
+  return requestPatchTime != null && resourceStartTime > requestPatchTime
 }
 
 /**
@@ -159,7 +137,7 @@ function isIntakeAPIEndpoint(url) {
   return /intake\/v\d+\/rum\/events/.test(url)
 }
 
-function createResourceTimingSpans(entries, apiSpans, trStart, trEnd) {
+function createResourceTimingSpans(entries, requestPatchTime, trStart, trEnd) {
   const spans = []
   for (let i = 0; i < entries.length; i++) {
     const { initiatorType, name, startTime, responseEnd } = entries[i]
@@ -186,7 +164,8 @@ function createResourceTimingSpans(entries, apiSpans, trStart, trEnd) {
     if (
       (initiatorType === RESOURCE_INITIATOR_TYPES[0] ||
         initiatorType === RESOURCE_INITIATOR_TYPES[1]) &&
-      (isIntakeAPIEndpoint(name) || isAlreadyCaptured(apiSpans, entries[i]))
+      (isIntakeAPIEndpoint(name) ||
+        isCapturedByPatching(startTime, requestPatchTime))
     ) {
       continue
     }
@@ -220,24 +199,6 @@ function createUserTimingSpans(entries, trStart, trEnd) {
   return userTimingSpans
 }
 
-function getAPISpans({ spans }) {
-  const apiSpans = []
-
-  for (let i = 0; i < spans.length; i++) {
-    const span = spans[i]
-    if (span.type === 'external' && span.subtype === 'http') {
-      const context = span.context
-      if (context) {
-        apiSpans.push({
-          url: context.http.url,
-          start: span._start
-        })
-      }
-    }
-  }
-  return apiSpans
-}
-
 /**
  * Navigation timing marks are reported only for page-load transactions
  *
@@ -338,7 +299,6 @@ function captureNavigation(transaction) {
    * for few extra spans than soft navigations which
    * happens on single page applications
    */
-
   if (transaction.type === PAGE_LOAD) {
     /**
      * Adjust custom marks properly to fit in the transaction timeframe
@@ -382,11 +342,9 @@ function captureNavigation(transaction) {
      * Capture resource timing information as spans
      */
     const resourceEntries = PERF.getEntriesByType(RESOURCE)
-    const apiSpans = getAPISpans(transaction)
-
     createResourceTimingSpans(
       resourceEntries,
-      apiSpans,
+      patch.patchedTime,
       trStart,
       trEnd
     ).forEach(span => transaction.spans.push(span))

packages/rum-core/test/performance-monitoring/capture-navigation.spec.js

@@ -154,7 +154,7 @@ describe('Capture hard navigation', function() {
   it('should createResourceTimingSpans', function() {
     const spans = createResourceTimingSpans(
       resourceEntries,
-      ['http://ajax-filter.test'],
+      null,
       transactionStart,
       transactionEnd
     )
@@ -194,14 +194,14 @@ describe('Capture hard navigation', function() {
     ]
     const spans = createResourceTimingSpans(
       entries,
-      [],
+      150,
       transactionStart,
       transactionEnd
     )
     expect(spans).toEqual([])
   })
 
-  it('should add/filter XHR/Fetch spans from RT data', function() {
+  it('should add/filter XHR/Fetch spans from RT data based on patch time', function() {
     const entries = [
       {
         name: 'http://localhost:8000/fetch',
@@ -218,51 +218,22 @@ describe('Capture hard navigation', function() {
         responseEnd: 150
       }
     ]
-    const getCount = (apiCalls = []) =>
+    const getCount = requestPatchTime =>
       createResourceTimingSpans(
         entries,
-        apiCalls,
+        requestPatchTime,
         transactionStart,
         transactionEnd
       ).length
 
     expect(getCount()).toBe(2)
-    // different url same time
-    expect(
-      getCount([
-        {
-          url: 'http://localhost:8000/data',
-          start: 100
-        }
-      ])
-    ).toBe(2)
-    // same url with different query params
-    expect(
-      getCount([
-        {
-          url: 'http://localhost:8000/data?query=bar',
-          start: 99
-        }
-      ])
-    ).toBe(2)
-    // same url different time
-    expect(
-      getCount([
-        {
-          url: 'http://localhost:8000/fetch',
-          start: 10
-        }
-      ])
-    ).toBe(2)
-    // same url same time
-    expect(
-      getCount([
-        {
-          url: 'http://localhost:8000/fetch',
-          start: 24
-        }
-      ])
-    ).toBe(1)
+    // same time as start of 1st resource
+    expect(getCount(25)).toBe(2)
+    // after first res start time
+    expect(getCount(30)).toBe(1)
+    expect(getCount(100)).toBe(1)
+    // after both resources
+    expect(getCount(101)).toBe(0)
   })
 
   it('should createUserTimingSpans', function() {