fix: Release backlog memory when features are blocked by RUM (#1102)

Co-authored-by: Patrick Housley <[email protected]>

Author: cwli24

src/common/drain/drain.js

@@ -32,8 +32,9 @@ export function registerDrain (agentIdentifier, group) {
  * @param {*} group - The named "bucket" to be removed from the registry
  */
 export function deregisterDrain (agentIdentifier, group) {
-  curateRegistry(agentIdentifier)
+  if (!agentIdentifier || !registry[agentIdentifier]) return
   if (registry[agentIdentifier].get(group)) registry[agentIdentifier].delete(group)
+  drainGroup(agentIdentifier, group, false)
   if (registry[agentIdentifier].size) checkCanDrainAll(agentIdentifier)
 }
 
@@ -86,29 +87,33 @@ function checkCanDrainAll (agentIdentifier) {
    * the subscribed handlers for the group.
    * @param {*} group - The name of a particular feature's event "bucket".
    */
-function drainGroup (agentIdentifier, group) {
+function drainGroup (agentIdentifier, group, activateGroup = true) {
   const baseEE = agentIdentifier ? ee.get(agentIdentifier) : ee
   const handlers = defaultRegister.handlers // other storage in registerHandler
   if (!baseEE.backlog || !handlers) return
 
-  var bufferedEventsInGroup = baseEE.backlog[group]
-  var groupHandlers = handlers[group] // each group in the registerHandler storage
-  if (groupHandlers) {
-    // We don't cache the length of the buffer while looping because events might still be added while processing.
-    for (var i = 0; bufferedEventsInGroup && i < bufferedEventsInGroup.length; ++i) { // eslint-disable-line no-unmodified-loop-condition
-      emitEvent(bufferedEventsInGroup[i], groupHandlers)
-    }
+  // Only activated features being drained should run queued listeners on buffered events. Deactivated features only need to release memory.
+  if (activateGroup) {
+    const bufferedEventsInGroup = baseEE.backlog[group]
+    const groupHandlers = handlers[group] // each group in the registerHandler storage
+    if (groupHandlers) {
+      // We don't cache the length of the buffer while looping because events might still be added while processing.
+      for (let i = 0; bufferedEventsInGroup && i < bufferedEventsInGroup.length; ++i) { // eslint-disable-line no-unmodified-loop-condition
+        emitEvent(bufferedEventsInGroup[i], groupHandlers)
+      }
 
-    mapOwn(groupHandlers, function (eventType, handlerRegistrationList) {
-      mapOwn(handlerRegistrationList, function (i, registration) {
-        // registration is an array of: [targetEE, eventHandler]
-        registration[0].on(eventType, registration[1])
+      mapOwn(groupHandlers, function (eventType, handlerRegistrationList) {
+        mapOwn(handlerRegistrationList, function (i, registration) {
+          // registration is an array of: [targetEE, eventHandler]
+          registration[0].on(eventType, registration[1])
+        })
       })
-    })
+    }
   }
+
   if (!baseEE.isolatedBacklog) delete handlers[group]
   baseEE.backlog[group] = null
-  baseEE.emit('drain-' + group, [])
+  baseEE.emit('drain-' + group, []) // TODO: Code exists purely for a unit test and needs to be refined
 }
 
 /**

tests/specs/cleanup-on-halt.e2e.js

@@ -0,0 +1,32 @@
+import { testRumRequest } from '../../tools/testing-server/utils/expect-tests'
+
+describe('Memory leaks', () => {
+  it('does not occur on ee backlog when RUM flags are 0', async () => {
+    await browser.testHandle.scheduleReply('bamServer', {
+      test: testRumRequest,
+      body: JSON.stringify({ st: 0, sr: 0, err: 0, ins: 0, spa: 0, loaded: 1 })
+    })
+    // This test relies on features to call deregisterDrain when not enabled by flags which in turn should clear their backlogs.
+
+    await browser.url(await browser.testHandle.assetURL('obfuscate-pii.html')).then(() => browser.waitForAgentLoad())
+    const backlog = await browser.execute(function () {
+      const backlog = {}
+      for (const key in newrelic.ee.backlog) {
+        const array = newrelic.ee.backlog[key]
+        backlog[key] = array === null ? 0 : array.length
+      }
+      return backlog
+    })
+
+    expect(backlog).toEqual(expect.objectContaining({
+      ajax: 0, // ajax does not rely on any flags anyways so it's always drained
+      jserrors: 0,
+      metrics: 0,
+      page_action: 0,
+      page_view_event: 0, // no handler
+      page_view_timing: 0, // does not rely on any flags
+      session_trace: 0,
+      spa: 0
+    }))
+  })
+})

tests/specs/harvesting/disable-harvesting.e2e.js

@@ -8,7 +8,6 @@ describe('disable harvesting', () => {
         st: 1,
         err: 0,
         ins: 1,
-        cap: 1,
         spa: 1,
         loaded: 1
       })
@@ -32,7 +31,6 @@ describe('disable harvesting', () => {
         st: 1,
         err: 1,
         ins: 1,
-        cap: 1,
         spa: 0,
         loaded: 1
       })
@@ -59,7 +57,6 @@ describe('disable harvesting', () => {
         st: 1,
         err: 1,
         ins: 0,
-        cap: 1,
         spa: 1,
         loaded: 1
       })
@@ -81,7 +78,6 @@ describe('disable harvesting', () => {
         st: 0,
         err: 1,
         ins: 1,
-        cap: 1,
         spa: 1,
         loaded: 1
       })

tests/specs/session-replay/initialization.e2e.js

@@ -12,7 +12,6 @@ async function disqualifySR () {
       sts: 1,
       err: 1,
       ins: 1,
-      cap: 1,
       spa: 1,
       loaded: 1,
       sr: 0,

tests/specs/session-replay/session-state.e2e.js

@@ -85,6 +85,9 @@ describe.withBrowsersMatching(notIE)('session manager state behavior', () => {
       await browser.enableSessionReplay()
       await browser.url(await browser.testHandle.assetURL('instrumented.html', srConfig()))
         .then(() => browser.waitForSessionReplayRecording())
+
+      await browser.closeWindow()
+      await browser.switchToWindow((await browser.getWindowHandles())[0])
     })
   })
 

tests/specs/session-trace/trace-nodes.e2e.js

@@ -33,7 +33,7 @@ describe('Trace nodes', () => {
     await browser.testHandle.clearScheduledReplies('bamServer')
     await browser.testHandle.scheduleReply('bamServer', {
       test: testRumRequest,
-      body: JSON.stringify({ st: 0, sts: 0, sr: 0, err: 1, ins: 1, cap: 1, spa: 0, loaded: 1 })
+      body: JSON.stringify({ st: 0, sts: 0, sr: 0, err: 1, ins: 1, spa: 0, loaded: 1 })
     })
 
     const url = await browser.testHandle.assetURL('pagehide.html', stConfig())

tests/specs/soft_navigations/integration.e2e.js

@@ -32,7 +32,7 @@ describe('Soft navigations', () => {
   it('does not harvest when spa is blocked by rum response', async () => {
     await browser.testHandle.scheduleReply('bamServer', {
       test: testRumRequest,
-      body: `${JSON.stringify({ st: 1, err: 1, ins: 1, cap: 1, spa: 0, loaded: 1 })}`
+      body: `${JSON.stringify({ st: 1, err: 1, ins: 1, spa: 0, loaded: 1 })}`
     })
 
     let url = await browser.testHandle.assetURL('instrumented.html', config)

tests/unit/common/drain/drain.test.js

@@ -1,8 +1,9 @@
-let registerDrain, drain, ee
+let registerDrain, drain, deregisterDrain, ee, registerHandler
 beforeEach(async () => {
   jest.resetModules()
-  ;({ registerDrain, drain } = await import('../../../../src/common/drain/drain'))
+  ;({ registerDrain, drain, deregisterDrain } = await import('../../../../src/common/drain/drain'))
   ;({ ee } = await import('../../../../src/common/event-emitter/contextual-ee'))
+  ;({ registerHandler } = await import('../../../../src/common/event-emitter/register-handler'))
 })
 
 test('can register a feat and drain it', () => {
@@ -25,6 +26,15 @@ test('other unregistered drains do not affect feat reg & drain', () => {
   expect(emitSpy).toHaveBeenCalledWith('drain-page_view_event', expect.anything())
 })
 
+test('unregistering groups clears their handlers from the buffering system', () => {
+  registerDrain('abcd', 'pumbaa')
+  registerHandler('tusks', () => {}, 'pumbaa', ee)
+  expect(registerHandler.handlers.pumbaa).toEqual({ tusks: expect.anything() })
+
+  deregisterDrain('abcd', 'pumbaa')
+  expect(registerHandler.handlers.pumbaa).toBeUndefined()
+})
+
 describe('drain', () => {
   test('does not execute until all registered groups calls it and in order', () => {
     registerDrain('abcd', 'page_view_timing')