✨ [RUMF-770] Disable tracing for cancelled requests (#635)

* ✨ [RUM] Disable tracing for cancelled requests * 🎨 [RUM] Move tracing logic to tracer * Apply suggestions from code review Co-authored-by: Bastien Caudan <[email protected]> * 🎨 [RUM] Move requestIndex to TracingContext * 👌 Move types to requestCollection Co-authored-by: Bastien Caudan <[email protected]>
DataDog · Dec 1, 2020 · 98adfcf · 98adfcf
1 parent f8c339b
commit 98adfcf
Show file tree

Hide file tree

Showing 4 changed files with 165 additions and 86 deletions.
diff --git a/packages/rum/src/domain/requestCollection.spec.ts b/packages/rum/src/domain/requestCollection.spec.ts
@@ -15,7 +15,7 @@ import {
 } from '@datadog/browser-core'
 import { LifeCycle, LifeCycleEventType } from './lifeCycle'
 import { RequestCompleteEvent, RequestStartEvent, trackFetch, trackXhr } from './requestCollection'
-import { Tracer } from './tracing/tracer'
+import { clearTracingIfCancelled, Tracer } from './tracing/tracer'
 
 const configuration = {
   ...DEFAULT_CONFIGURATION,
@@ -43,6 +43,7 @@ describe('collect fetch', () => {
     lifeCycle.subscribe(LifeCycleEventType.REQUEST_STARTED, startSpy)
     lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy)
     const tracerStub: Partial<Tracer> = {
+      clearTracingIfCancelled,
       traceFetch: () => undefined,
     }
     fetchProxy = trackFetch(lifeCycle, configuration as Configuration, tracerStub as Tracer)
@@ -84,6 +85,18 @@ describe('collect fetch', () => {
     })
   })
 
+  it('should not trace cancelled requests', (done) => {
+    fetchStub(FAKE_URL).resolveWith({ status: 0, responseText: 'fetch cancelled' })
+
+    fetchStubManager.whenAllComplete(() => {
+      const request = completeSpy.calls.argsFor(0)[0]
+
+      expect(request.status).toEqual(0)
+      expect(request.traceId).toEqual(undefined)
+      done()
+    })
+  })
+
   it('should assign a request id', (done) => {
     fetchStub(FAKE_URL).resolveWith({ status: 500, responseText: 'fetch error' })
 
@@ -123,6 +136,7 @@ describe('collect xhr', () => {
     lifeCycle.subscribe(LifeCycleEventType.REQUEST_STARTED, startSpy)
     lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy)
     const tracerStub: Partial<Tracer> = {
+      clearTracingIfCancelled,
       traceXhr: () => undefined,
     }
     trackXhr(lifeCycle, configuration as Configuration, tracerStub as Tracer)
@@ -198,4 +212,20 @@ describe('collect xhr', () => {
       },
     })
   })
+
+  it('should not trace cancelled requests', (done) => {
+    withXhr({
+      setup(xhr) {
+        xhr.open('GET', '/ok')
+        xhr.send()
+        xhr.complete(0)
+      },
+      onComplete() {
+        const request = completeSpy.calls.argsFor(0)[0]
+        expect(request.status).toEqual(0)
+        expect(request.traceId).toEqual(undefined)
+        done()
+      },
+    })
+  })
 })
diff --git a/packages/rum/src/domain/requestCollection.ts b/packages/rum/src/domain/requestCollection.ts
@@ -13,6 +13,16 @@ import { LifeCycle, LifeCycleEventType } from './lifeCycle'
 import { isAllowedRequestUrl } from './rumEventsCollection/resource/resourceUtils'
 import { startTracer, TraceIdentifier, Tracer } from './tracing/tracer'
 
+export interface CustomContext {
+  requestIndex: number
+  spanId?: TraceIdentifier
+  traceId?: TraceIdentifier
+}
+export interface RumFetchStartContext extends FetchStartContext, CustomContext {}
+export interface RumFetchCompleteContext extends FetchCompleteContext, CustomContext {}
+export interface RumXhrStartContext extends XhrStartContext, CustomContext {}
+export interface RumXhrCompleteContext extends XhrCompleteContext, CustomContext {}
+
 export interface RequestStartEvent {
   requestIndex: number
 }
@@ -27,14 +37,8 @@ export interface RequestCompleteEvent {
   responseType?: string
   startTime: number
   duration: number
-  traceId?: TraceIdentifier
   spanId?: TraceIdentifier
-}
-
-interface CustomContext {
-  traceId: TraceIdentifier | undefined
-  spanId: TraceIdentifier | undefined
-  requestIndex: number
+  traceId?: TraceIdentifier
 }
 
 export type RequestObservables = [Observable<RequestStartEvent>, Observable<RequestCompleteEvent>]
@@ -48,14 +52,10 @@ export function startRequestCollection(lifeCycle: LifeCycle, configuration: Conf
 }
 
 export function trackXhr(lifeCycle: LifeCycle, configuration: Configuration, tracer: Tracer) {
-  const xhrProxy = startXhrProxy<CustomContext & XhrStartContext, CustomContext & XhrCompleteContext>()
+  const xhrProxy = startXhrProxy<RumXhrStartContext, RumXhrCompleteContext>()
   xhrProxy.beforeSend((context, xhr) => {
     if (isAllowedRequestUrl(configuration, context.url)) {
-      const tracingResult = tracer.traceXhr(context, xhr)
-      if (tracingResult) {
-        context.traceId = tracingResult.traceId
-        context.spanId = tracingResult.spanId
-      }
+      tracer.traceXhr(context, xhr)
       context.requestIndex = getNextRequestIndex()
 
       lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, {
@@ -65,6 +65,7 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: Configuration, tra
   })
   xhrProxy.onRequestComplete((context) => {
     if (isAllowedRequestUrl(configuration, context.url)) {
+      tracer.clearTracingIfCancelled(context)
       lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, {
         duration: context.duration,
         method: context.method,
@@ -83,14 +84,10 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: Configuration, tra
 }
 
 export function trackFetch(lifeCycle: LifeCycle, configuration: Configuration, tracer: Tracer) {
-  const fetchProxy = startFetchProxy<CustomContext & FetchStartContext, CustomContext & FetchCompleteContext>()
+  const fetchProxy = startFetchProxy<RumFetchStartContext, RumFetchCompleteContext>()
   fetchProxy.beforeSend((context) => {
     if (isAllowedRequestUrl(configuration, context.url)) {
-      const tracingResult = tracer.traceFetch(context)
-      if (tracingResult) {
-        context.traceId = tracingResult.traceId
-        context.spanId = tracingResult.spanId
-      }
+      tracer.traceFetch(context)
       context.requestIndex = getNextRequestIndex()
 
       lifeCycle.notify(LifeCycleEventType.REQUEST_STARTED, {
@@ -100,6 +97,7 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: Configuration, t
   })
   fetchProxy.onRequestComplete((context) => {
     if (isAllowedRequestUrl(configuration, context.url)) {
+      tracer.clearTracingIfCancelled(context)
       lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, {
         duration: context.duration,
         method: context.method,