✨ [RUM-4013] DD_RUM: add handling stack in beforeSend context (#2730)

* ✨ [RUM-4013] add handling stack in beforeSend context * add handling stack for addErrors and addAction * 🧪 add e2e tests * add e2e and loose up unit tests * remove unused variable * 🧪 add missing test coverage * ✅ add e2e test for console errors * fix missing imports * remove unused imports * 👌 add type to e2e tests * 👌 fix pr comments
DataDog · May 30, 2024 · 717506e · 717506e
1 parent a4fe94f
commit 717506e
Show file tree

Hide file tree

Showing 20 changed files with 365 additions and 54 deletions.
diff --git a/packages/core/src/browser/fetchObservable.spec.ts b/packages/core/src/browser/fetchObservable.spec.ts
@@ -47,6 +47,7 @@ describe('fetch proxy', () => {
       expect(request.url).toEqual(FAKE_URL)
       expect(request.status).toEqual(500)
       expect(request.isAborted).toBe(false)
+      expect(request.handlingStack).toBeDefined()
       done()
     })
   })
@@ -61,6 +62,7 @@ describe('fetch proxy', () => {
       expect(request.status).toEqual(0)
       expect(request.isAborted).toBe(false)
       expect(request.error).toEqual(new Error('fetch error'))
+      expect(request.handlingStack).toBeDefined()
       done()
     })
   })
@@ -75,6 +77,7 @@ describe('fetch proxy', () => {
       expect(request.status).toEqual(0)
       expect(request.isAborted).toBe(true)
       expect(request.error).toEqual(new DOMException('The user aborted a request', 'AbortError'))
+      expect(request.handlingStack).toBeDefined()
       done()
     })
   })

diff --git a/packages/core/src/browser/fetchObservable.ts b/packages/core/src/browser/fetchObservable.ts
@@ -12,6 +12,7 @@ interface FetchContextBase {
   input: unknown
   init?: RequestInit
   url: string
+  handlingStack?: string
 }
 
 export interface FetchStartContext extends FetchContextBase {
@@ -44,14 +45,16 @@ function createFetchObservable() {
       return
     }
 
-    const { stop } = instrumentMethod(window, 'fetch', (call) => beforeSend(call, observable))
+    const { stop } = instrumentMethod(window, 'fetch', (call) => beforeSend(call, observable), {
+      computeHandlingStack: true,
+    })
 
     return stop
   })
 }
 
 function beforeSend(
-  { parameters, onPostCall }: InstrumentedMethodCall<Window, 'fetch'>,
+  { parameters, onPostCall, handlingStack }: InstrumentedMethodCall<Window, 'fetch'>,
   observable: Observable<FetchContext>
 ) {
   const [input, init] = parameters
@@ -72,6 +75,7 @@ function beforeSend(
     method,
     startClocks,
     url,
+    handlingStack,
   }
 
   observable.notify(context)

diff --git a/packages/core/src/browser/xhrObservable.spec.ts b/packages/core/src/browser/xhrObservable.spec.ts
@@ -51,6 +51,7 @@ describe('xhr observable', () => {
         expect(request.status).toBe(200)
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -66,6 +67,7 @@ describe('xhr observable', () => {
       onComplete() {
         const request = requests[0]
         expect(request.method).toBe('GET')
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -85,6 +87,7 @@ describe('xhr observable', () => {
         expect(request.status).toBe(404)
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -104,6 +107,7 @@ describe('xhr observable', () => {
         expect(request.status).toBe(500)
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -123,6 +127,7 @@ describe('xhr observable', () => {
         expect(request.status).toBe(0)
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -151,6 +156,7 @@ describe('xhr observable', () => {
         expect(request.isAborted).toBe(false)
         expect(xhr.status).toBe(0)
         expect(xhr.onreadystatechange).toHaveBeenCalledTimes(1)
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -171,6 +177,7 @@ describe('xhr observable', () => {
         expect(request.duration).toEqual(jasmine.any(Number))
         expect(request.isAborted).toBe(true)
         expect(xhr.status).toBe(0)
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -192,6 +199,7 @@ describe('xhr observable', () => {
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
         expect(xhr.onreadystatechange).toHaveBeenCalled()
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -213,6 +221,7 @@ describe('xhr observable', () => {
         expect(request.isAborted).toBe(false)
         expect(request.duration).toEqual(jasmine.any(Number))
         expect(xhr.onreadystatechange).toHaveBeenCalled()
+        expect(request.handlingStack).toBeDefined()
         done()
       },
     })
@@ -284,13 +293,15 @@ describe('xhr observable', () => {
         expect(firstRequest.status).toBe(200)
         expect(firstRequest.isAborted).toBe(false)
         expect(firstRequest.duration).toEqual(jasmine.any(Number))
+        expect(firstRequest.handlingStack).toBeDefined()
 
         const secondRequest = requests[1]
         expect(secondRequest.method).toBe('GET')
         expect(secondRequest.url).toContain('/ok?request=2')
         expect(secondRequest.status).toBe(400)
         expect(secondRequest.isAborted).toBe(false)
         expect(secondRequest.duration).toEqual(jasmine.any(Number))
+        expect(secondRequest.handlingStack).toBeDefined()
 
         expect(xhr.onreadystatechange).toHaveBeenCalledTimes(2)
         expect(listeners.load.length).toBe(0)

diff --git a/packages/core/src/browser/xhrObservable.ts b/packages/core/src/browser/xhrObservable.ts
@@ -19,6 +19,7 @@ export interface XhrStartContext extends Omit<XhrOpenContext, 'state'> {
   startClocks: ClocksState
   isAborted: boolean
   xhr: XMLHttpRequest
+  handlingStack?: string
 }
 
 export interface XhrCompleteContext extends Omit<XhrStartContext, 'state'> {
@@ -43,9 +44,14 @@ function createXhrObservable(configuration: Configuration) {
   return new Observable<XhrContext>((observable) => {
     const { stop: stopInstrumentingStart } = instrumentMethod(XMLHttpRequest.prototype, 'open', openXhr)
 
-    const { stop: stopInstrumentingSend } = instrumentMethod(XMLHttpRequest.prototype, 'send', (call) => {
-      sendXhr(call, configuration, observable)
-    })
+    const { stop: stopInstrumentingSend } = instrumentMethod(
+      XMLHttpRequest.prototype,
+      'send',
+      (call) => {
+        sendXhr(call, configuration, observable)
+      },
+      { computeHandlingStack: true }
+    )
 
     const { stop: stopInstrumentingAbort } = instrumentMethod(XMLHttpRequest.prototype, 'abort', abortXhr)
 
@@ -66,7 +72,7 @@ function openXhr({ target: xhr, parameters: [method, url] }: InstrumentedMethodC
 }
 
 function sendXhr(
-  { target: xhr }: InstrumentedMethodCall<XMLHttpRequest, 'send'>,
+  { target: xhr, handlingStack }: InstrumentedMethodCall<XMLHttpRequest, 'send'>,
   configuration: Configuration,
   observable: Observable<XhrContext>
 ) {
@@ -80,6 +86,7 @@ function sendXhr(
   startContext.startClocks = clocksNow()
   startContext.isAborted = false
   startContext.xhr = xhr
+  startContext.handlingStack = handlingStack
 
   let hasBeenReported = false
 

diff --git a/packages/core/src/tools/experimentalFeatures.ts b/packages/core/src/tools/experimentalFeatures.ts
@@ -14,6 +14,7 @@ export enum ExperimentalFeature {
   WRITABLE_RESOURCE_GRAPHQL = 'writable_resource_graphql',
   CUSTOM_VITALS = 'custom_vitals',
   TOLERANT_RESOURCE_TIMINGS = 'tolerant_resource_timings',
+  MICRO_FRONTEND = 'micro_frontend',
 }
 
 const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()

diff --git a/packages/core/src/tools/instrumentMethod.spec.ts b/packages/core/src/tools/instrumentMethod.spec.ts
@@ -59,6 +59,7 @@ describe('instrumentMethod', () => {
       target: object,
       parameters: jasmine.any(Object),
       onPostCall: jasmine.any(Function),
+      handlingStack: undefined,
     })
     expect(instrumentationSpy.calls.mostRecent().args[0].parameters[0]).toBe(2)
     expect(instrumentationSpy.calls.mostRecent().args[0].parameters[1]).toBe(3)
@@ -103,6 +104,22 @@ describe('instrumentMethod', () => {
     expect(instrumentationSpy).toHaveBeenCalled()
   })
 
+  it('computes the handling stack', () => {
+    const object = { method: () => 1 }
+    const instrumentationSpy = jasmine.createSpy()
+    instrumentMethod(object, 'method', instrumentationSpy, { computeHandlingStack: true })
+
+    function foo() {
+      object.method()
+    }
+
+    foo()
+
+    expect(instrumentationSpy.calls.mostRecent().args[0].handlingStack).toEqual(
+      jasmine.stringMatching(/^Error: \n {2}at foo @/)
+    )
+  })
+
   describe('stop()', () => {
     it('does not call the instrumentation anymore', () => {
       const object = { method: () => 1 }

diff --git a/packages/core/src/tools/instrumentMethod.ts b/packages/core/src/tools/instrumentMethod.ts
@@ -2,6 +2,7 @@ import { setTimeout } from './timer'
 import { callMonitored } from './monitor'
 import { noop } from './utils/functionUtils'
 import { arrayFrom, startsWith } from './utils/polyfills'
+import { createHandlingStack } from './stackTrace/handlingStack'
 
 /**
  * Object passed to the callback of an instrumented method call. See `instrumentMethod` for more
@@ -25,6 +26,11 @@ export type InstrumentedMethodCall<TARGET extends { [key: string]: any }, METHOD
    * result passed as argument.
    */
   onPostCall: (callback: PostCallCallback<TARGET, METHOD>) => void
+
+  /**
+   * The stack trace of the method call.
+   */
+  handlingStack?: string
 }
 
 type PostCallCallback<TARGET extends { [key: string]: any }, METHOD extends keyof TARGET> = (
@@ -65,7 +71,8 @@ type PostCallCallback<TARGET extends { [key: string]: any }, METHOD extends keyo
 export function instrumentMethod<TARGET extends { [key: string]: any }, METHOD extends keyof TARGET & string>(
   targetPrototype: TARGET,
   method: METHOD,
-  onPreCall: (this: null, callInfos: InstrumentedMethodCall<TARGET, METHOD>) => void
+  onPreCall: (this: null, callInfos: InstrumentedMethodCall<TARGET, METHOD>) => void,
+  { computeHandlingStack }: { computeHandlingStack?: boolean } = {}
 ) {
   let original = targetPrototype[method]
 
@@ -77,34 +84,14 @@ export function instrumentMethod<TARGET extends { [key: string]: any }, METHOD e
     }
   }
 
-  let instrumentation = createInstrumentedMethod(original, onPreCall)
+  let stopped = false
 
-  const instrumentationWrapper = function (this: TARGET): ReturnType<TARGET[METHOD]> | undefined {
-    if (typeof instrumentation !== 'function') {
-      return undefined
+  const instrumentation = function (this: TARGET): ReturnType<TARGET[METHOD]> | undefined {
+    if (stopped) {
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call
+      return original.apply(this, arguments as unknown as Parameters<TARGET[METHOD]>)
     }
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call
-    return instrumentation.apply(this, arguments as unknown as Parameters<TARGET[METHOD]>)
-  }
-  targetPrototype[method] = instrumentationWrapper as TARGET[METHOD]
 
-  return {
-    stop: () => {
-      if (targetPrototype[method] === instrumentationWrapper) {
-        targetPrototype[method] = original
-      } else {
-        instrumentation = original
-      }
-    },
-  }
-}
-
-function createInstrumentedMethod<TARGET extends { [key: string]: any }, METHOD extends keyof TARGET>(
-  original: TARGET[METHOD],
-  onPreCall: (this: null, callInfos: InstrumentedMethodCall<TARGET, METHOD>) => void
-): TARGET[METHOD] {
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
-  return function (this: TARGET) {
     const parameters = arrayFrom(arguments) as Parameters<TARGET[METHOD]>
 
     let postCallCallback: PostCallCallback<TARGET, METHOD> | undefined
@@ -116,6 +103,7 @@ function createInstrumentedMethod<TARGET extends { [key: string]: any }, METHOD
         onPostCall: (callback) => {
           postCallCallback = callback
         },
+        handlingStack: computeHandlingStack ? createHandlingStack() : undefined,
       },
     ])
 
@@ -128,7 +116,19 @@ function createInstrumentedMethod<TARGET extends { [key: string]: any }, METHOD
 
     // eslint-disable-next-line @typescript-eslint/no-unsafe-return
     return result
-  } as TARGET[METHOD]
+  }
+
+  targetPrototype[method] = instrumentation as TARGET[METHOD]
+
+  return {
+    stop: () => {
+      stopped = true
+      // If the instrumentation has been removed by a third party, keep the last one
+      if (targetPrototype[method] === instrumentation) {
+        targetPrototype[method] = original
+      }
+    },
+  }
 }
 
 export function instrumentSetter<TARGET extends { [key: string]: any }, PROPERTY extends keyof TARGET>(

diff --git a/packages/core/src/tools/stackTrace/handlingStack.spec.ts b/packages/core/src/tools/stackTrace/handlingStack.spec.ts
@@ -15,6 +15,6 @@ describe('createHandlingStack', () => {
   it('should create handling stack trace without internal calls', () => {
     userCallOne()
 
-    expect(handlingStack).toMatch('Error: \n {2}at userCallTwo @ (.*)\n {2}at userCallOne @ (.*)')
+    expect(handlingStack).toMatch(/^Error: \n\s+at userCallTwo @ (.*)\n\s+at userCallOne @ (.*)/)
   })
 })
diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts
@@ -183,11 +183,26 @@ describe('rum public api', () => {
           name: 'foo',
           startClocks: jasmine.any(Object),
           type: ActionType.CUSTOM,
+          handlingStack: jasmine.any(String),
         },
         { context: {}, user: {}, hasReplay: undefined },
       ])
     })
 
+    it('should generate a handling stack', () => {
+      rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
+
+      function triggerAction() {
+        rumPublicApi.addAction('foo', { bar: 'baz' })
+      }
+
+      triggerAction()
+
+      expect(addActionSpy).toHaveBeenCalledTimes(1)
+      const stacktrace = addActionSpy.calls.argsFor(0)[0].handlingStack
+      expect(stacktrace).toMatch(/^Error:\s+at triggerAction @/)
+    })
+
     describe('save context when sending an action', () => {
       it('saves the date', () => {
         const { clock } = setupBuilder.withFakeClock().build()

diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts
@@ -422,15 +422,20 @@ export function makeRumPublicApi(
 
     getInitConfiguration: monitor(() => deepClone(strategy.initConfiguration)),
 
-    addAction: monitor((name, context) => {
-      strategy.addAction({
-        name: sanitize(name)!,
-        context: sanitize(context) as Context,
-        startClocks: clocksNow(),
-        type: ActionType.CUSTOM,
+    addAction: (name, context) => {
+      const handlingStack = createHandlingStack()
+
+      callMonitored(() => {
+        strategy.addAction({
+          name: sanitize(name)!,
+          context: sanitize(context) as Context,
+          startClocks: clocksNow(),
+          type: ActionType.CUSTOM,
+          handlingStack,
+        })
+        addTelemetryUsage({ feature: 'add-action' })
       })
-      addTelemetryUsage({ feature: 'add-action' })
-    }),
+    },
 
     addError: (error, context) => {
       const handlingStack = createHandlingStack()