♻️ [RUMF-1046] instrument method implementation

packages/core/src/index.ts

@@ -49,6 +49,7 @@ export * from './tools/timeUtils'
 export * from './tools/utils'
 export * from './tools/createEventRateLimiter'
 export * from './tools/browserDetection'
+export { instrumentMethod } from './tools/instrumentMethod'
 export { ErrorSource, ErrorHandling, formatUnknownError, createHandlingStack, RawError } from './tools/error'
 export { Context, ContextArray, ContextValue } from './tools/context'
 export { areCookiesAuthorized, getCookie, setCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'

packages/core/src/tools/instrumentMethod.spec.ts

@@ -0,0 +1,106 @@
+import { instrumentMethod } from './instrumentMethod'
+
+describe('instrumentMethod', () => {
+  it('replaces the original method', () => {
+    const original = () => 1
+    const object = { method: original }
+
+    instrumentMethod(object, 'method', () => () => 2)
+
+    expect(object.method).not.toBe(original)
+    expect(object.method()).toBe(2)
+  })
+
+  it('sets a method originally undefined', () => {
+    const object: { method?: () => number } = {}
+
+    instrumentMethod(object, 'method', () => () => 2)
+
+    expect(object.method!()).toBe(2)
+  })
+
+  it('provides the original method to the instrumentation factory', () => {
+    const original = () => 1
+    const object = { method: original }
+    const instrumentationFactorySpy = jasmine.createSpy().and.callFake((original: () => number) => () => original() + 2)
+
+    instrumentMethod(object, 'method', instrumentationFactorySpy)
+
+    expect(instrumentationFactorySpy).toHaveBeenCalledOnceWith(original)
+    expect(object.method()).toBe(3)
+  })
+
+  it('calls the instrumentation with method arguments', () => {
+    const object = { method: (a: number, b: number) => a + b }
+    const instrumentationSpy = jasmine.createSpy()
+    instrumentMethod(object, 'method', () => instrumentationSpy)
+
+    object.method(2, 3)
+
+    expect(instrumentationSpy).toHaveBeenCalledOnceWith(2, 3)
+  })
+
+  it('allows other instrumentations from third parties', () => {
+    const object = { method: () => 1 }
+    const instrumentationSpy = jasmine.createSpy().and.returnValue(2)
+    instrumentMethod(object, 'method', () => instrumentationSpy)
+
+    thirdPartyInstrumentation(object)
+
+    expect(object.method()).toBe(4)
+    expect(instrumentationSpy).toHaveBeenCalled()
+  })
+
+  describe('stop()', () => {
+    it('restores the original behavior', () => {
+      const object = { method: () => 1 }
+      const { stop } = instrumentMethod(object, 'method', () => () => 2)
+
+      stop()
+
+      expect(object.method()).toBe(1)
+    })
+
+    it('does not call the instrumentation anymore', () => {
+      const object = { method: () => 1 }
+      const instrumentationSpy = jasmine.createSpy()
+      const { stop } = instrumentMethod(object, 'method', () => instrumentationSpy)
+
+      stop()
+
+      object.method()
+      expect(instrumentationSpy).not.toHaveBeenCalled()
+    })
+
+    describe('when the method has been instrumented by a third party', () => {
+      it('should not break the third party instrumentation', () => {
+        const object = { method: () => 1 }
+        const { stop } = instrumentMethod(object, 'method', () => () => 2)
+
+        thirdPartyInstrumentation(object)
+        const instrumentedMethod = object.method
+
+        stop()
+
+        expect(object.method).toBe(instrumentedMethod)
+      })
+
+      it('does not call the instrumentation', () => {
+        const object = { method: () => 1 }
+        const instrumentationSpy = jasmine.createSpy()
+        const { stop } = instrumentMethod(object, 'method', () => instrumentationSpy)
+
+        thirdPartyInstrumentation(object)
+
+        stop()
+
+        expect(instrumentationSpy).not.toHaveBeenCalled()
+      })
+    })
+  })
+
+  function thirdPartyInstrumentation(object: { method: () => number }) {
+    const originalMethod = object.method
+    object.method = () => originalMethod() + 2
+  }
+})

packages/core/src/tools/instrumentMethod.ts

@@ -0,0 +1,27 @@
+export function instrumentMethod<OBJECT extends { [key: string]: any }, METHOD extends keyof OBJECT>(
+  object: OBJECT,
+  method: METHOD,
+  instrumentationFactory: (
+    original: OBJECT[METHOD]
+  ) => (this: OBJECT, ...args: Parameters<OBJECT[METHOD]>) => ReturnType<OBJECT[METHOD]>
+) {
+  const original = object[method]
+
+  let instrumentation = instrumentationFactory(original)
+
+  const instrumentationWrapper = function (this: OBJECT): ReturnType<OBJECT[METHOD]> {
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+    return instrumentation.apply(this, (arguments as unknown) as Parameters<OBJECT[METHOD]>)
+  }
+  object[method] = instrumentationWrapper as OBJECT[METHOD]
+
+  return {
+    stop: () => {
+      if (object[method] === instrumentationWrapper) {
+        object[method] = original
+      } else {
+        instrumentation = original
+      }
+    },
+  }
+}

packages/rum-core/src/browser/locationChangeObservable.ts

@@ -1,4 +1,4 @@
-import { monitor, addEventListener, DOM_EVENT, Observable } from '@datadog/browser-core'
+import { addEventListener, DOM_EVENT, Observable, callMonitored, instrumentMethod } from '@datadog/browser-core'
 
 export interface LocationChange {
   oldLocation: Readonly<Location>
@@ -32,25 +32,35 @@ export function createLocationChangeObservable(location: Location) {
 }
 
 function trackHistory(onHistoryChange: () => void) {
-  // eslint-disable-next-line @typescript-eslint/unbound-method
-  const originalPushState = history.pushState
-  history.pushState = monitor(function (this: History['pushState']) {
-    originalPushState.apply(this, arguments as any)
-    onHistoryChange()
-  })
-  // eslint-disable-next-line @typescript-eslint/unbound-method
-  const originalReplaceState = history.replaceState
-  history.replaceState = monitor(function (this: History['replaceState']) {
-    originalReplaceState.apply(this, arguments as any)
-    onHistoryChange()
-  })
+  const { stop: stopInstrumentingPushState } = instrumentMethod(
+    history,
+    'pushState',
+    (original) =>
+      function () {
+        original.apply(this, arguments as any)
+        callMonitored(onHistoryChange)
+      }
+  )
+
+  const { stop: stopInstrumentingReplaceState } = instrumentMethod(
+    history,
+    'replaceState',
+    (original) =>
+      function () {
+        original.apply(this, arguments as any)
+        callMonitored(onHistoryChange)
+      }
+  )
+
   const { stop: removeListener } = addEventListener(window, DOM_EVENT.POP_STATE, onHistoryChange)
-  const stop = () => {
-    removeListener()
-    history.pushState = originalPushState
-    history.replaceState = originalReplaceState
+
+  return {
+    stop: () => {
+      stopInstrumentingPushState()
+      stopInstrumentingReplaceState()
+      removeListener()
+    },
   }
-  return { stop }
 }
 
 function trackHash(onHashChange: () => void) {