Merge branch 'main' into aymeric/collect-cwv-node-attribution

package.json

@@ -55,7 +55,7 @@
     "eslint-plugin-import": "2.28.1",
     "eslint-plugin-jasmine": "4.1.3",
     "eslint-plugin-jsdoc": "46.5.1",
-    "eslint-plugin-local-rules": "1.3.2",
+    "eslint-plugin-local-rules": "2.0.0",
     "eslint-plugin-prefer-arrow": "1.2.3",
     "eslint-plugin-unicorn": "48.0.1",
     "express": "4.18.2",

packages/core/src/browser/addEventListener.ts

@@ -111,7 +111,7 @@ export function addEventListeners<Target extends EventTarget, EventName extends
   listener: (event: EventMapFor<Target>[EventName]) => void,
   { once, capture, passive }: AddEventListenerOptions = {}
 ) {
-  const wrappedListener = monitor(
+  const listenerWithMonitor = monitor(
     once
       ? (event: Event) => {
           stop()
@@ -123,11 +123,11 @@ export function addEventListeners<Target extends EventTarget, EventName extends
   const options = passive ? { capture, passive } : capture
 
   const add = getZoneJsOriginalValue(eventTarget, 'addEventListener')
-  eventNames.forEach((eventName) => add.call(eventTarget, eventName, wrappedListener, options))
+  eventNames.forEach((eventName) => add.call(eventTarget, eventName, listenerWithMonitor, options))
 
   function stop() {
     const remove = getZoneJsOriginalValue(eventTarget, 'removeEventListener')
-    eventNames.forEach((eventName) => remove.call(eventTarget, eventName, wrappedListener, options))
+    eventNames.forEach((eventName) => remove.call(eventTarget, eventName, listenerWithMonitor, options))
   }
 
   return {

packages/core/src/browser/pageExitObservable.spec.ts

@@ -1,31 +1,28 @@
 import type { Configuration } from '../domain/configuration'
-import { createNewEvent, restorePageVisibility, setPageVisibility } from '../../test'
+import { createNewEvent, restorePageVisibility, setPageVisibility, registerCleanupTask } from '../../test'
 import { resetExperimentalFeatures, addExperimentalFeatures, ExperimentalFeature } from '../tools/experimentalFeatures'
-import type { Subscription } from '../tools/observable'
 import type { PageExitEvent } from './pageExitObservable'
 import { PageExitReason, createPageExitObservable } from './pageExitObservable'
 
 describe('createPageExitObservable', () => {
-  let pageExitSubscription: Subscription
   let onExitSpy: jasmine.Spy<(event: PageExitEvent) => void>
   let configuration: Configuration
 
   beforeEach(() => {
     onExitSpy = jasmine.createSpy()
     configuration = {} as Configuration
-    pageExitSubscription = createPageExitObservable(configuration).subscribe(onExitSpy)
+    registerCleanupTask(createPageExitObservable(configuration).subscribe(onExitSpy).unsubscribe)
   })
 
   afterEach(() => {
-    pageExitSubscription.unsubscribe()
     restorePageVisibility()
     resetExperimentalFeatures()
   })
 
   it('notifies when the page fires pagehide if ff pagehide is enabled', () => {
     addExperimentalFeatures([ExperimentalFeature.PAGEHIDE])
     onExitSpy = jasmine.createSpy()
-    pageExitSubscription = createPageExitObservable(configuration).subscribe(onExitSpy)
+    registerCleanupTask(createPageExitObservable(configuration).subscribe(onExitSpy).unsubscribe)
 
     window.dispatchEvent(createNewEvent('pagehide'))
     window.dispatchEvent(createNewEvent('beforeunload'))

packages/core/src/transport/batch.ts

@@ -4,19 +4,21 @@ import { objectValues } from '../tools/utils/polyfills'
 import { isPageExitReason } from '../browser/pageExitObservable'
 import { computeBytesCount } from '../tools/utils/byteUtils'
 import { jsonStringify } from '../tools/serialisation/jsonStringify'
+import type { Subscription } from '../tools/observable'
 import type { HttpRequest } from './httpRequest'
 import type { FlushController, FlushEvent } from './flushController'
 
 export class Batch {
   private pushOnlyBuffer: string[] = []
   private upsertBuffer: { [key: string]: string } = {}
+  private flushSubscription: Subscription
 
   constructor(
     private request: HttpRequest,
     public flushController: FlushController,
     private messageBytesLimit: number
   ) {
-    this.flushController.flushObservable.subscribe((event) => this.flush(event))
+    this.flushSubscription = this.flushController.flushObservable.subscribe((event) => this.flush(event))
   }
 
   add(message: Context) {
@@ -27,6 +29,10 @@ export class Batch {
     this.addOrUpdate(message, key)
   }
 
+  stop() {
+    this.flushSubscription.unsubscribe()
+  }
+
   private flush(event: FlushEvent) {
     const messages = this.pushOnlyBuffer.concat(objectValues(this.upsertBuffer))
 

packages/core/src/transport/flushController.ts

@@ -33,10 +33,13 @@ export function createFlushController({
   pageExitObservable,
   sessionExpireObservable,
 }: FlushControllerOptions) {
-  const flushObservable = new Observable<FlushEvent>()
+  const pageExitSubscription = pageExitObservable.subscribe((event) => flush(event.reason))
+  const sessionExpireSubscription = sessionExpireObservable.subscribe(() => flush('session_expire'))
 
-  pageExitObservable.subscribe((event) => flush(event.reason))
-  sessionExpireObservable.subscribe(() => flush('session_expire'))
+  const flushObservable = new Observable<FlushEvent>(() => () => {
+    pageExitSubscription.unsubscribe()
+    sessionExpireSubscription.unsubscribe()
+  })
 
   let currentBytesCount = 0
   let currentMessagesCount = 0

packages/core/src/transport/startBatchWithReplica.ts

@@ -48,5 +48,10 @@ export function startBatchWithReplica<T extends Context>(
         replicaBatch.upsert(replica.transformMessage ? replica.transformMessage(message) : message, key)
       }
     },
+
+    stop: () => {
+      primaryBatch.stop()
+      replicaBatch?.stop()
+    },
   }
 }

packages/core/test/forEach.spec.ts

@@ -1,4 +1,5 @@
 import type { BuildEnvWindow } from './buildEnv'
+import { startLeakDetection, stopLeakDetection } from './leakDetection'
 
 beforeEach(() => {
   ;(window as unknown as BuildEnvWindow).__BUILD_ENV__SDK_VERSION__ = 'test'
@@ -7,10 +8,12 @@ beforeEach(() => {
   ;(window as any).DD_RUM = {}
   // prevent 'Some of your tests did a full page reload!' issue
   window.onbeforeunload = () => 'stop'
+  startLeakDetection()
 })
 
 afterEach(() => {
   clearAllCookies()
+  stopLeakDetection()
 })
 
 function clearAllCookies() {

packages/core/test/index.ts

@@ -1,6 +1,7 @@
 export * from './browserChecks'
 export * from './buildEnv'
 export * from './collectAsyncCalls'
+export * from './registerCleanupTask'
 export * from './requests'
 export * from './emulate/createNewEvent'
 export * from './emulate/location'

packages/core/test/leakDetection.ts

@@ -0,0 +1,58 @@
+import { display } from '../src/tools/display'
+import { isIE } from '../src/tools/utils/browserDetection'
+import { getCurrentJasmineSpec } from './getCurrentJasmineSpec'
+
+let originalAddEventListener: typeof EventTarget.prototype.addEventListener
+let originalRemoveEventListener: typeof EventTarget.prototype.removeEventListener
+let wrappedListeners: {
+  [key: string]: Map<EventListenerOrEventListenerObject | null, EventListenerOrEventListenerObject | null>
+}
+
+export function startLeakDetection() {
+  if (isIE()) {
+    return
+  }
+  wrappedListeners = {}
+
+  // eslint-disable-next-line @typescript-eslint/unbound-method
+  originalAddEventListener = EventTarget.prototype.addEventListener
+  // eslint-disable-next-line @typescript-eslint/unbound-method
+  originalRemoveEventListener = EventTarget.prototype.removeEventListener
+
+  EventTarget.prototype.addEventListener = function (event, listener, options) {
+    if (!wrappedListeners[event]) {
+      wrappedListeners[event] = new Map()
+    }
+    const wrappedListener = withLeakDetection(event, listener as EventListener)
+    wrappedListeners[event].set(listener, wrappedListener)
+    return originalAddEventListener.call(this, event, wrappedListener, options)
+  }
+  EventTarget.prototype.removeEventListener = function (event, listener, options) {
+    const wrappedListener = wrappedListeners[event]?.get(listener)
+    wrappedListeners[event]?.delete(listener)
+    return originalRemoveEventListener.call(this, event, wrappedListener || listener, options)
+  }
+}
+
+export function stopLeakDetection() {
+  if (isIE()) {
+    return
+  }
+  EventTarget.prototype.addEventListener = originalAddEventListener
+  EventTarget.prototype.removeEventListener = originalRemoveEventListener
+  wrappedListeners = {}
+}
+
+function withLeakDetection(eventName: string, listener: EventListener) {
+  const specWhenAdded = getCurrentJasmineSpec()!.fullName
+  return (event: Event) => {
+    const currentSpec = getCurrentJasmineSpec()!.fullName
+    if (specWhenAdded !== currentSpec) {
+      display.error(`Leaked listener
+  event names: "${eventName}"
+  attached with: "${specWhenAdded}"        
+  executed with: "${currentSpec}"`)
+    }
+    listener(event)
+  }
+}

packages/core/test/registerCleanupTask.ts

@@ -0,0 +1,9 @@
+const cleanupTasks: Array<() => void> = []
+
+export function registerCleanupTask(task: () => void) {
+  cleanupTasks.push(task)
+}
+
+afterEach(() => {
+  cleanupTasks.splice(0).forEach((task) => task())
+})

packages/logs/src/boot/startLogs.spec.ts

@@ -7,12 +7,13 @@ import {
   initEventBridgeStub,
   cleanupSyntheticsWorkerValues,
   mockSyntheticsWorkerValues,
+  registerCleanupTask,
 } from '@datadog/browser-core/test'
 
 import type { LogsConfiguration } from '../domain/configuration'
 import { validateAndBuildLogsConfiguration } from '../domain/configuration'
 import { HandlerType, Logger, StatusType } from '../domain/logger'
-import type { startLoggerCollection } from '../domain/logsCollection/logger/loggerCollection'
+import type { startLoggerCollection } from '../domain/logger/loggerCollection'
 import type { LogsEvent } from '../logsEvent.types'
 import { startLogs } from './startLogs'
 
@@ -43,6 +44,7 @@ describe('logs', () => {
   let interceptor: ReturnType<typeof interceptRequests>
   let requests: Request[]
   let handleLog: ReturnType<typeof startLoggerCollection>['handleLog']
+  let stopLogs: () => void
   let logger: Logger
   let consoleLogSpy: jasmine.Spy
   let displayLogSpy: jasmine.Spy
@@ -69,7 +71,8 @@ describe('logs', () => {
 
   describe('request', () => {
     it('should send the needed data', () => {
-      ;({ handleLog: handleLog } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
 
       handleLog({ message: 'message', status: StatusType.warn, context: { foo: 'bar' } }, logger, COMMON_CONTEXT)
 
@@ -91,12 +94,13 @@ describe('logs', () => {
     })
 
     it('should all use the same batch', () => {
-      ;({ handleLog } = startLogs(
+      ;({ handleLog, stop: stopLogs } = startLogs(
         initConfiguration,
         { ...baseConfiguration, batchMessagesLimit: 3 },
         () => COMMON_CONTEXT,
         logger
       ))
+      registerCleanupTask(stopLogs)
 
       handleLog(DEFAULT_MESSAGE, logger)
       handleLog(DEFAULT_MESSAGE, logger)
@@ -107,7 +111,8 @@ describe('logs', () => {
 
     it('should send bridge event when bridge is present', () => {
       const sendSpy = spyOn(initEventBridgeStub(), 'send')
-      ;({ handleLog: handleLog } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
 
       handleLog(DEFAULT_MESSAGE, logger)
 
@@ -126,13 +131,15 @@ describe('logs', () => {
       const sendSpy = spyOn(initEventBridgeStub(), 'send')
 
       let configuration = { ...baseConfiguration, sessionSampleRate: 0 }
-      ;({ handleLog } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
       handleLog(DEFAULT_MESSAGE, logger)
 
       expect(sendSpy).not.toHaveBeenCalled()
 
       configuration = { ...baseConfiguration, sessionSampleRate: 100 }
-      ;({ handleLog } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
       handleLog(DEFAULT_MESSAGE, logger)
 
       expect(sendSpy).toHaveBeenCalled()
@@ -141,12 +148,13 @@ describe('logs', () => {
 
   it('should not print the log twice when console handler is enabled', () => {
     logger.setHandler([HandlerType.console])
-    ;({ handleLog } = startLogs(
+    ;({ handleLog, stop: stopLogs } = startLogs(
       initConfiguration,
       { ...baseConfiguration, forwardConsoleLogs: ['log'] },
       () => COMMON_CONTEXT,
       logger
     ))
+    registerCleanupTask(stopLogs)
 
     /* eslint-disable-next-line no-console */
     console.log('foo', 'bar')
@@ -161,21 +169,24 @@ describe('logs', () => {
     })
 
     it('creates a session on normal conditions', () => {
-      ;({ handleLog } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
 
       expect(getCookie(SESSION_STORE_KEY)).not.toBeUndefined()
     })
 
     it('does not create a session if event bridge is present', () => {
       initEventBridgeStub()
-      ;({ handleLog } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
 
       expect(getCookie(SESSION_STORE_KEY)).toBeUndefined()
     })
 
     it('does not create a session if synthetics worker will inject RUM', () => {
       mockSyntheticsWorkerValues({ injectsRum: true })
-      ;({ handleLog } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      ;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT, logger))
+      registerCleanupTask(stopLogs)
 
       expect(getCookie(SESSION_STORE_KEY)).toBeUndefined()
     })