From d82f90673b29f5531b2fa28872590123ebf68878 Mon Sep 17 00:00:00 2001 From: William Lacroix Date: Wed, 14 Dec 2022 09:52:56 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"Revert=20"=F0=9F=90=9B=20[RUMF-1337]?= =?UTF-8?q?=20Fix=20incorrect=20fetch=20duration=20(#1862)"=20(#1873)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit acd78cb0ca21d8271287e62ae4068e72f3ba964e. --- packages/core/src/browser/fetchObservable.ts | 6 ++-- .../src/domain/requestCollection.spec.ts | 29 +------------------ .../rum-core/src/domain/requestCollection.ts | 5 +--- .../resource/resourceCollection.spec.ts | 8 ----- .../resource/resourceCollection.ts | 21 +------------- packages/rum-core/src/rawRumEvent.types.ts | 3 -- test/e2e/scenario/rum/resources.scenario.ts | 2 +- 7 files changed, 6 insertions(+), 68 deletions(-) diff --git a/packages/core/src/browser/fetchObservable.ts b/packages/core/src/browser/fetchObservable.ts index d0c2a71347..23c53c6a1c 100644 --- 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 index 57c7fa1812..adb1fb9e47 100644 --- 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 index 184b8c0cf7..abd953897b 100644 --- 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 index 10d7f23d7d..26cb76e2ca 100644 --- 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 index 0dc9c72f9f..bdc1f0dd77 100644 --- 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 index 6919e4975b..c3b7241399 100644 --- 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 index 991e3048b9..8b05bc8e5e 100644 --- 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(