🐛 [RUMF-1337] Fix incorrect fetch duration (#1862)

* 🐛 [RUMF-1337] Fix incorrect fetch duration ♻️ remove unused imports ♻️ remove responseDurationInfo ♻️ remove unused import * 🔥 remove `resolveDuration` * 🔥 remove `resolveDuration` * 🐛 [RUMF-1449] workaround for Firefox memory leak when using Zone.js (#1860) * ♻️ [RUMF-1449] rename EventEmitter to EventTarget Use the native name rather than introducing our own concept. * 🚚 [RUMF-1449] move addEventListener functions to a dedicated module * 🐛 [RUMF-1449] implement a workaround for Firefox memory leak * 👌🚚 move stubZoneJs in its own module * 👷 Bump staging to staging-51 * 🔊 Collect computed and perf entry durations (#1861) * 🔊 Collect computed and perf entry durations * ✨Allow internal analytics subdomain (#1863) * 🔥 remove `resolveDuration` 🔥 remove `resolveDuration` Co-authored-by: Benoît <benoit.zugmeyer@datadoghq.com> Co-authored-by: ci.browser-sdk <ci.browser-sdk@datadoghq.com> Co-authored-by: Aymeric <aymeric.mortemousque@datadoghq.com>
DataDog · Dec 13, 2022 · d1b1a42 · d1b1a42
1 parent 6436ccb
commit d1b1a42
Show file tree

Hide file tree

Showing 7 changed files with 6 additions and 68 deletions.
diff --git a/packages/core/src/browser/fetchObservable.ts b/packages/core/src/browser/fetchObservable.ts
@@ -1,8 +1,8 @@
 import { instrumentMethod } from '../tools/instrumentMethod'
 import { callMonitored, monitor } from '../tools/monitor'
 import { Observable } from '../tools/observable'
-import type { Duration, ClocksState } from '../tools/timeUtils'
-import { elapsed, clocksNow, timeStampNow } from '../tools/timeUtils'
+import type { ClocksState } from '../tools/timeUtils'
+import { clocksNow } from '../tools/timeUtils'
 import { normalizeUrl } from '../tools/urlPolyfill'
 
 interface FetchContextBase {
@@ -20,7 +20,6 @@ export interface FetchStartContext extends FetchContextBase {
 export interface FetchResolveContext extends FetchContextBase {
   state: 'resolve'
   status: number
-  resolveDuration?: Duration
   response?: Response
   responseType?: string
   isAborted: boolean
@@ -96,7 +95,6 @@ function afterSend(
   const reportFetch = (response: Response | Error) => {
     const context = startContext as unknown as FetchResolveContext
     context.state = 'resolve'
-    context.resolveDuration = elapsed(startContext.startClocks.timeStamp, timeStampNow())
     if ('stack' in response || response instanceof Error) {
       context.status = 0
       context.isAborted = response instanceof DOMException && response.code === DOMException.ABORT_ERR

diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts
@@ -1,4 +1,4 @@
-import { isIE, RequestType, updateExperimentalFeatures, resetExperimentalFeatures } from '@datadog/browser-core'
+import { isIE, RequestType } from '@datadog/browser-core'
 import type { FetchStub, FetchStubManager } from '../../../core/test/specHelper'
 import { SPEC_ENDPOINTS, stubFetch, stubXhr, withXhr } from '../../../core/test/specHelper'
 import type { RumConfiguration } from './configuration'
@@ -100,33 +100,6 @@ describe('collect fetch', () => {
     })
   })
 
-  describe('with feature flag fetch_duration', () => {
-    beforeEach(() => updateExperimentalFeatures(['fetch_duration']))
-    afterEach(() => resetExperimentalFeatures())
-
-    it('should notify when response is defined', (done) => {
-      fetchStub(FAKE_URL).resolveWith({ status: 200, responseText: 'ok' })
-
-      fetchStubManager.whenAllComplete(() => {
-        expect(startSpy).toHaveBeenCalled()
-        expect(completeSpy).toHaveBeenCalled()
-        done()
-      })
-    })
-
-    it('should notify when response is undefined', (done) => {
-      updateExperimentalFeatures(['fetch_duration'])
-
-      fetchStub(FAKE_URL).rejectWith(new Error('some fetch error'))
-
-      fetchStubManager.whenAllComplete(() => {
-        expect(startSpy).toHaveBeenCalled()
-        expect(completeSpy).toHaveBeenCalled()
-        done()
-      })
-    })
-  })
-
   describe('tracing', () => {
     it('should trace requests by default', (done) => {
       fetchStub(FAKE_URL).resolveWith({ status: 200, responseText: 'ok' })

diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts
@@ -13,7 +13,6 @@ import {
   readBytesFromStream,
   elapsed,
   timeStampNow,
-  isExperimentalFeatureEnabled,
 } from '@datadog/browser-core'
 import type { RumSessionManager } from '..'
 import type { RumConfiguration } from './configuration'
@@ -46,7 +45,6 @@ export interface RequestCompleteEvent {
   status: number
   responseType?: string
   startClocks: ClocksState
-  resolveDuration?: Duration
   duration: Duration
   spanId?: TraceIdentifier
   traceId?: TraceIdentifier
@@ -130,7 +128,6 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration
         waitForResponseToComplete(context, (duration: Duration) => {
           tracer.clearTracingIfNeeded(context)
           lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, {
-            resolveDuration: context.resolveDuration,
             duration,
             method: context.method,
             requestIndex: context.requestIndex,
@@ -160,7 +157,7 @@ function getNextRequestIndex() {
 }
 
 function waitForResponseToComplete(context: RumFetchResolveContext, callback: (duration: Duration) => void) {
-  if (context.response && isExperimentalFeatureEnabled('fetch_duration')) {
+  if (context.response) {
     const responseClone = context.response.clone()
     if (responseClone.body) {
       readBytesFromStream(

diff --git a/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.spec.ts
@@ -96,9 +96,6 @@ describe('resourceCollection', () => {
       type: RumEventType.RESOURCE,
       _dd: {
         discarded: false,
-        resolveDuration: undefined,
-        durationDiff: undefined,
-        durationPercentageDiff: undefined,
       },
     })
     expect(rawRumEvents[0].domainContext).toEqual({
@@ -122,7 +119,6 @@ describe('resourceCollection', () => {
       LifeCycleEventType.REQUEST_COMPLETED,
       createCompletedRequest({
         duration: 500 as Duration,
-        resolveDuration: 50 as Duration,
         method: 'GET',
         startClocks: relativeToClocks(100 as RelativeTime),
         status: 200,
@@ -150,7 +146,6 @@ describe('resourceCollection', () => {
       LifeCycleEventType.REQUEST_COMPLETED,
       createCompletedRequest({
         duration: 100 as Duration,
-        resolveDuration: 50 as Duration,
         method: 'GET',
         startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp },
         status: 200,
@@ -176,9 +171,6 @@ describe('resourceCollection', () => {
       type: RumEventType.RESOURCE,
       _dd: {
         discarded: false,
-        resolveDuration: (50 * 1e6) as ServerDuration,
-        durationDiff: (50 * 1e6) as ServerDuration,
-        durationPercentageDiff: 50,
       },
     })
     expect(rawRumEvents[0].domainContext).toEqual({

diff --git a/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/resource/resourceCollection.ts
@@ -9,7 +9,7 @@ import {
   isNumber,
   isExperimentalFeatureEnabled,
 } from '@datadog/browser-core'
-import type { ClocksState, Duration, ServerDuration } from '@datadog/browser-core'
+import type { ClocksState, ServerDuration } from '@datadog/browser-core'
 import type { RumConfiguration } from '../../configuration'
 import type { RumPerformanceEntry, RumPerformanceResourceTiming } from '../../../browser/performanceCollection'
 import type {
@@ -67,8 +67,6 @@ function processRequest(
   const tracingInfo = computeRequestTracingInfo(request, configuration)
   const indexingInfo = computeIndexingInfo(sessionManager, startClocks)
 
-  const responseDurationInfo = computeResponseDurationInfo(request)
-
   const duration = toServerDuration(request.duration)
   const durationOverrideInfo = computeDurationOverrideInfo(duration, correspondingTimingOverrides?.resource.duration)
 
@@ -88,7 +86,6 @@ function processRequest(
     tracingInfo,
     correspondingTimingOverrides,
     indexingInfo,
-    responseDurationInfo,
     durationOverrideInfo
   )
   return {
@@ -179,22 +176,6 @@ function computeEntryTracingInfo(entry: RumPerformanceResourceTiming, configurat
   }
 }
 
-function computeResponseDurationInfo(request: RequestCompleteEvent) {
-  let durationDiff
-  let durationPercentageDiff
-  if (request.resolveDuration) {
-    durationDiff = toServerDuration((request.duration - request.resolveDuration) as Duration)
-    durationPercentageDiff = Math.round((durationDiff / toServerDuration(request.duration)) * 100)
-  }
-  return {
-    _dd: {
-      resolveDuration: toServerDuration(request.resolveDuration),
-      durationDiff,
-      durationPercentageDiff,
-    },
-  }
-}
-
 function computeDurationOverrideInfo(
   computedDuration: ServerDuration,
   performanceEntryDuration: ServerDuration | undefined

diff --git a/packages/rum-core/src/rawRumEvent.types.ts b/packages/rum-core/src/rawRumEvent.types.ts
@@ -42,9 +42,6 @@ export interface RawRumResourceEvent {
     span_id?: string // not available for initial document tracing
     rule_psr?: number
     discarded: boolean
-    resolveDuration?: ServerDuration
-    durationDiff?: ServerDuration
-    durationPercentageDiff?: number
   }
 }
 

diff --git a/test/e2e/scenario/rum/resources.scenario.ts b/test/e2e/scenario/rum/resources.scenario.ts
@@ -197,7 +197,7 @@ describe('rum resources', () => {
   })
 
   createTest('track redirect fetch timings')
-    .withRum({ enableExperimentalFeatures: ['fetch_duration'] })
+    .withRum()
     .run(async ({ serverEvents }) => {
       await browserExecuteAsync((done) => {
         fetch('/redirect?duration=200').then(