🐛 [RUMF-71] prevent negative performance timing duration

packages/rum/src/resourceUtils.ts

@@ -39,29 +39,49 @@ export function computeResourceKind(timing: PerformanceResourceTiming) {
   return ResourceKind.OTHER
 }
 
+function formatTiming(start: number, end: number) {
+  if (start <= 0) {
+    return undefined
+  }
+  return { duration: msToNs(Math.max(end - start, 0)), start: msToNs(start) }
+}
+
+const durationPairs: Array<[keyof PerformanceResourceTiming, keyof PerformanceResourceTiming]> = [
+  ['connectStart', 'connectEnd'],
+  ['domainLookupStart', 'domainLookupEnd'],
+  ['responseStart', 'responseEnd'],
+  ['requestStart', 'responseStart'],
+  ['redirectStart', 'redirectEnd'],
+  ['secureConnectionStart', 'connectEnd'],
+]
+function reporteAbnormalEntry(entry: PerformanceResourceTiming) {
+  let error = ''
+  for (const [start, end] of durationPairs) {
+    if (entry[start] < 0) {
+      error += `${start} is negative\n`
+    } else if (entry[start] > entry[end]) {
+      error += `${start} is greater than ${end}\n`
+    }
+  }
+  if (error) {
+    addMonitoringMessage(`Got an abnormal PerformanceResourceTiming:
+${error}
+Entry: ${JSON.stringify(entry)}`)
+  }
+}
+
 export function computePerformanceResourceDetails(
   entry?: PerformanceResourceTiming
 ): PerformanceResourceDetails | undefined {
   if (entry && hasTimingAllowedAttributes(entry)) {
+    reporteAbnormalEntry(entry)
     return {
-      connect: { duration: msToNs(entry.connectEnd - entry.connectStart), start: msToNs(entry.connectStart) },
-      dns: {
-        duration: msToNs(entry.domainLookupEnd - entry.domainLookupStart),
-        start: msToNs(entry.domainLookupStart),
-      },
-      download: { duration: msToNs(entry.responseEnd - entry.responseStart), start: msToNs(entry.responseStart) },
-      firstByte: { duration: msToNs(entry.responseStart - entry.requestStart), start: msToNs(entry.requestStart) },
-      redirect:
-        entry.redirectStart > 0
-          ? { duration: msToNs(entry.redirectEnd - entry.redirectStart), start: msToNs(entry.redirectStart) }
-          : undefined,
-      ssl:
-        entry.secureConnectionStart > 0
-          ? {
-              duration: msToNs(entry.connectEnd - entry.secureConnectionStart),
-              start: msToNs(entry.secureConnectionStart),
-            }
-          : undefined,
+      connect: formatTiming(entry.connectStart, entry.connectEnd),
+      dns: formatTiming(entry.domainLookupStart, entry.domainLookupEnd),
+      download: formatTiming(entry.responseStart, entry.responseEnd),
+      firstByte: formatTiming(entry.requestStart, entry.responseStart),
+      redirect: formatTiming(entry.redirectStart, entry.redirectEnd),
+      ssl: formatTiming(entry.secureConnectionStart, entry.connectEnd),
     }
   }
   return undefined

packages/rum/src/rum.ts

@@ -55,11 +55,11 @@ interface PerformanceResourceDetailsElement {
 
 export interface PerformanceResourceDetails {
   redirect?: PerformanceResourceDetailsElement
-  dns: PerformanceResourceDetailsElement
-  connect: PerformanceResourceDetailsElement
+  dns?: PerformanceResourceDetailsElement
+  connect?: PerformanceResourceDetailsElement
   ssl?: PerformanceResourceDetailsElement
-  firstByte: PerformanceResourceDetailsElement
-  download: PerformanceResourceDetailsElement
+  firstByte?: PerformanceResourceDetailsElement
+  download?: PerformanceResourceDetailsElement
 }
 
 export interface RumResourceEvent {

packages/rum/test/rum.spec.ts

@@ -163,8 +163,46 @@ describe('rum handle performance entry', () => {
       new LifeCycle()
     )
     const resourceEvent = getEntry(addRumEvent, 0) as RumResourceEvent
-    expect(resourceEvent.http.performance!.connect.duration).toEqual(7 * 1e6)
-    expect(resourceEvent.http.performance!.download.duration).toEqual(75 * 1e6)
+    expect(resourceEvent.http.performance!.connect!.duration).toEqual(7 * 1e6)
+    expect(resourceEvent.http.performance!.download!.duration).toEqual(75 * 1e6)
+  })
+
+  it('should ignore negative timing start', () => {
+    const entry: Partial<PerformanceResourceTiming> = {
+      connectEnd: 10,
+      connectStart: -3,
+      entryType: 'resource',
+      name: 'http://localhost/test',
+      responseEnd: 100,
+      responseStart: 25,
+    }
+
+    handleResourceEntry(
+      configuration as Configuration,
+      entry as PerformanceResourceTiming,
+      addRumEvent,
+      new LifeCycle()
+    )
+    const resourceEvent = getEntry(addRumEvent, 0) as RumResourceEvent
+    expect(resourceEvent.http.performance!.connect).toBe(undefined)
+  })
+
+  it('should send positive durations only', () => {
+    const entry: Partial<PerformanceResourceTiming> = {
+      entryType: 'resource',
+      name: 'http://localhost/test',
+      responseEnd: 25,
+      responseStart: 100,
+    }
+
+    handleResourceEntry(
+      configuration as Configuration,
+      entry as PerformanceResourceTiming,
+      addRumEvent,
+      new LifeCycle()
+    )
+    const resourceEvent = getEntry(addRumEvent, 0) as RumResourceEvent
+    expect(resourceEvent.http.performance!.download!.duration).toBe(0)
   })
 })
 

test/e2e/scenario/agents.scenario.ts

@@ -95,7 +95,7 @@ describe('rum', () => {
     expect(timing.http.method).toEqual('GET')
     expect((timing.http as any).status_code).toEqual(200)
     expect(timing.duration).toBeGreaterThan(0)
-    expect(timing.http.performance!.download.start).toBeGreaterThan(0)
+    expect(timing.http.performance!.download!.start).toBeGreaterThan(0)
   })
 
   it('should send performance timings along the view events', async () => {