feat: Standardize all feature behavior to wait for RUM response (#927)

src/common/drain/drain.js

@@ -26,13 +26,24 @@ export function registerDrain (agentIdentifier, group) {
   if (!registry[agentIdentifier].get(group)) registry[agentIdentifier].set(group, item)
 }
 
+/**
+ * Removes an item from the registry and immediately re-checks if the registry is ready to "drain all"
+ * @param {*} agentIdentifier - A 16 character string uniquely identifying the agent.
+ * @param {*} group - The named "bucket" to be removed from the registry
+ */
+export function deregisterDrain (agentIdentifier, group) {
+  curateRegistry(agentIdentifier)
+  if (registry[agentIdentifier].get(group)) registry[agentIdentifier].delete(group)
+  if (registry[agentIdentifier].size) checkCanDrainAll(agentIdentifier)
+}
+
 /**
  * Registers the specified agent with the centralized event buffer registry if it is not already registered.
  * Agents without an identifier (as in the case of some tests) will be excluded from the registry.
  * @param {string} agentIdentifier - A 16 character string uniquely identifying an agent.
  */
 function curateRegistry (agentIdentifier) {
-  if (!agentIdentifier) return
+  if (!agentIdentifier) throw new Error('agentIdentifier required')
   if (!registry[agentIdentifier]) registry[agentIdentifier] = new Map()
 }
 
@@ -48,54 +59,56 @@ export function drain (agentIdentifier = '', featureName = 'feature', force = fa
   // If the feature for the specified agent is not in the registry, that means the instrument file was bypassed.
   // This could happen in tests, or loaders that directly import the aggregator. In these cases it is safe to
   // drain the feature group immediately rather than waiting to drain all at once.
-  if (!agentIdentifier || !registry[agentIdentifier].get(featureName) || force) return drainGroup(featureName)
+  if (!agentIdentifier || !registry[agentIdentifier].get(featureName) || force) return drainGroup(agentIdentifier, featureName)
 
   // When `drain` is called, this feature is ready to drain (staged).
   registry[agentIdentifier].get(featureName).staged = true
 
-  // Only when the event-groups for all features are ready to drain (staged) do we execute the drain. This has the effect
+  checkCanDrainAll(agentIdentifier)
+}
+
+/** Checks all items in the registry to see if they have been "staged".  If ALL items are staged, it will drain all registry items (drainGroup).  It not, nothing will happen */
+function checkCanDrainAll (agentIdentifier) {
+// Only when the event-groups for all features are ready to drain (staged) do we execute the drain. This has the effect
   // that the last feature to call drain triggers drain for all features.
   const items = [...registry[agentIdentifier]]
   if (items.every(([key, values]) => values.staged)) {
     items.sort((a, b) => a[1].priority - b[1].priority)
     items.forEach(([group]) => {
       registry[agentIdentifier].delete(group)
-      drainGroup(group)
+      drainGroup(agentIdentifier, group)
     })
   }
+}
 
-  /**
+/**
    * Drains all the buffered (backlog) events for a particular feature's event-group by emitting each event to each of
    * the subscribed handlers for the group.
    * @param {*} group - The name of a particular feature's event "bucket".
    */
-  function drainGroup (group) {
-    const baseEE = agentIdentifier ? ee.get(agentIdentifier) : ee
-    const handlers = defaultRegister.handlers
-    if (!baseEE.backlog || !handlers) return
+function drainGroup (agentIdentifier, group) {
+  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]
-    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)
-      }
-
-      mapOwn(groupHandlers, function (eventType, handlerRegistrationList) {
-        mapOwn(handlerRegistrationList, function (i, registration) {
-          // registration is an array of: [targetEE, eventHandler]
-          registration[0].on(eventType, registration[1])
-        })
-      })
+  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)
     }
 
-    delete handlers[group]
-
-    // Keep the feature's event-group as a property of the event emitter so we know it was already created and drained.
-    baseEE.backlog[group] = null
-    baseEE.emit('drain-' + group, [])
+    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, [])
 }
 
 /**

src/common/drain/drain.test.js

@@ -61,18 +61,15 @@ describe('drain', () => {
     expect(emitSpy).toHaveBeenCalledWith('drain-feature', expect.anything())
   })
 
-  test('works on the global ee when agent id not provided', () => {
-    let emitSpy = jest.spyOn(ee, 'emit')
-    drain()
-    expect(emitSpy).toHaveBeenCalledWith('drain-feature', expect.anything())
+  test('fails when agent id not provided', () => {
+    expect(() => drain()).toThrow()
   })
 })
 
 test('non-feat groups can register and drain too alongside features', () => {
   registerDrain('abcd', 'page_view_event')
   registerDrain('abcd', 'simba')
 
-  console.log(JSON.stringify(ee.get('abcd')))
   let emitSpy = jest.spyOn(ee.get('abcd'), 'emit')
   drain('abcd', 'simba')
   expect(emitSpy).not.toHaveBeenCalled()

src/common/event-emitter/contextual-ee.component-test.js

@@ -160,7 +160,7 @@ describe('event-emitter buffer', () => {
     const { ee } = await import('./contextual-ee')
     const { drain } = await import('../drain/drain')
     const mockListener = jest.fn()
-    const eventType = faker.datatype.uuid()
+    const eventType = faker.string.uuid()
     const eventArgs = ['a', 'b', 'c']
 
     ee.on(eventType, mockListener)

src/common/event-emitter/contextual-ee.js

@@ -49,13 +49,32 @@ function ee (old, debugId) {
     context,
     buffer: bufferEventsByGroup,
     abort,
-    aborted: false,
     isBuffering,
     debugId,
-    backlog: isolatedBacklog ? {} : old && typeof old.backlog === 'object' ? old.backlog : {}
+    backlog: isolatedBacklog ? {} : old && typeof old.backlog === 'object' ? old.backlog : {},
+    isolatedBacklog
+  }
 
+  function abort () {
+    emitter._aborted = true
+    Object.keys(emitter.backlog).forEach(key => {
+      delete emitter.backlog[key]
+    })
   }
 
+  Object.defineProperty(emitter, 'aborted', {
+    get: () => {
+      let aborted = emitter._aborted || false
+
+      if (aborted) return aborted
+      else if (old) {
+        aborted = old.aborted
+      }
+
+      return aborted
+    }
+  })
+
   return emitter
 
   function context (contextOrStore) {
@@ -141,14 +160,3 @@ function ee (old, debugId) {
     return emitter.backlog
   }
 }
-
-function abort () {
-  globalInstance.aborted = true
-  // The global backlog can be referenced directly by other emitters,
-  // so we need to delete its contents as opposed to replacing it.
-  // Otherwise, these references to the old backlog would still exist
-  // and the keys will not be garbage collected.
-  Object.keys(globalInstance.backlog).forEach(key => {
-    delete globalInstance.backlog[key]
-  })
-}

src/common/util/feature-flags.js

@@ -3,49 +3,31 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import { ee } from '../event-emitter/contextual-ee'
-import { handle } from '../event-emitter/handle'
-import { FEATURE_NAMES } from '../../loaders/features/features'
 import { dispatchGlobalEvent } from '../dispatch/global-event'
 
-const bucketMap = {
-  stn: [FEATURE_NAMES.sessionTrace],
-  err: [FEATURE_NAMES.jserrors, FEATURE_NAMES.metrics],
-  ins: [FEATURE_NAMES.pageAction],
-  spa: [FEATURE_NAMES.spa, FEATURE_NAMES.softNav],
-  sr: [FEATURE_NAMES.sessionReplay, FEATURE_NAMES.sessionTrace]
-}
-
 const sentIds = new Set()
 
-/** Note that this function only processes each unique flag ONCE, with the first occurrence of each flag and numeric value determining its switch on/off setting. */
+/** A map of feature flags and their values as provided by the rum call -- scoped by agent ID */
+export const activatedFeatures = {}
+
+/**
+ * Sets the activatedFeatures object, dispatches the global loaded event,
+ * and emits the rumresp flag to features
+ * @param {{[key:string]:number}} flags key-val pair of flag names and numeric
+ * @param {string} agentIdentifier agent instance identifier
+ * @returns {void}
+ */
 export function activateFeatures (flags, agentIdentifier) {
   const sharedEE = ee.get(agentIdentifier)
+  activatedFeatures[agentIdentifier] ??= {}
   if (!(flags && typeof flags === 'object')) return
   if (sentIds.has(agentIdentifier)) return
 
-  Object.entries(flags).forEach(([flag, num]) => {
-    if (bucketMap[flag]) {
-      bucketMap[flag].forEach(feat => {
-        if (!num) handle('block-' + flag, [], undefined, feat, sharedEE)
-        else handle('feat-' + flag, [], undefined, feat, sharedEE)
-        handle('rumresp-' + flag, [Boolean(num)], undefined, feat, sharedEE) // this is a duplicate of feat-/block- but makes awaiting for 1 event easier than 2
-      })
-    } else if (num) handle('feat-' + flag, [], undefined, undefined, sharedEE) // not sure what other flags are overlooked, but there's a test for ones not in the map --
-    activatedFeatures[flag] = Boolean(num)
-  })
+  sharedEE.emit('rumresp', [flags])
+  activatedFeatures[agentIdentifier] = flags
 
-  // Let the features waiting on their respective flags know that RUM response was received and that any missing flags are interpreted as bad entitlement / "off".
-  // Hence, those features will not be hanging forever if their flags aren't included in the response.
-  Object.keys(bucketMap).forEach(flag => {
-    if (activatedFeatures[flag] === undefined) {
-      bucketMap[flag]?.forEach(feat => handle('rumresp-' + flag, [false], undefined, feat, sharedEE))
-      activatedFeatures[flag] = false
-    }
-  })
   sentIds.add(agentIdentifier)
 
   // let any window level subscribers know that the agent is running
   dispatchGlobalEvent({ loaded: true })
 }
-
-export const activatedFeatures = {}

src/common/util/feature-flags.test.js

@@ -3,7 +3,6 @@ import * as eventEmitterModule from '../event-emitter/contextual-ee'
 import * as handleModule from '../event-emitter/handle'
 import * as drainModule from '../drain/drain'
 import { activateFeatures, activatedFeatures } from './feature-flags'
-import { FEATURE_NAMES } from '../../loaders/features/features'
 
 jest.enableAutomock()
 jest.unmock('./feature-flags')
@@ -12,15 +11,15 @@ let agentIdentifier
 
 beforeEach(() => {
   agentIdentifier = faker.string.uuid()
-
+  const emitterFn = jest.fn()
   eventEmitterModule.ee.get = jest.fn(() => ({
-    [faker.string.uuid()]: faker.string.uuid()
+    emit: emitterFn
   }))
 })
 
 afterEach(() => {
   Object.keys(activatedFeatures)
-    .forEach(key => delete activatedFeatures[key])
+    .forEach(key => delete activatedFeatures[agentIdentifier][key])
 })
 
 test.each([
@@ -31,58 +30,53 @@ test.each([
 
   expect(handleModule.handle).not.toHaveBeenCalled()
   expect(drainModule.drain).not.toHaveBeenCalled()
-  expect(activatedFeatures).toEqual({})
+  expect(activatedFeatures[agentIdentifier]).toEqual({})
 })
 
-const bucketMap = {
-  stn: [FEATURE_NAMES.sessionTrace],
-  err: [FEATURE_NAMES.jserrors, FEATURE_NAMES.metrics],
-  ins: [FEATURE_NAMES.pageAction],
-  spa: [FEATURE_NAMES.spa],
-  sr: [FEATURE_NAMES.sessionReplay, FEATURE_NAMES.sessionTrace]
-}
-
 test('emits the right events when feature flag = 1', () => {
-  const flags = {}
-  Object.keys(bucketMap).forEach(flag => { flags[flag] = 1 })
+  const flags = {
+    stn: 1,
+    err: 1,
+    ins: 1,
+    spa: 1,
+    sr: 1
+  }
   activateFeatures(flags, agentIdentifier)
 
-  const sharedEE = jest.mocked(eventEmitterModule.ee.get).mock.results[0].value
+  const sharedEE = eventEmitterModule.ee.get(agentIdentifier).emit
 
-  // each flag gets emitted to each of its mapped features, and a feat- AND a rumresp- for every emit, so (1+2+1+1+2+2)*2 = 16
-  expect(handleModule.handle).toHaveBeenCalledTimes(16)
-  expect(handleModule.handle).toHaveBeenNthCalledWith(1, 'feat-stn', [], undefined, FEATURE_NAMES.sessionTrace, sharedEE)
-  expect(handleModule.handle).toHaveBeenLastCalledWith('rumresp-sr', [true], undefined, FEATURE_NAMES.sessionTrace, sharedEE)
+  expect(sharedEE).toHaveBeenCalledTimes(1)
+  expect(sharedEE).toHaveBeenLastCalledWith('rumresp', [flags])
 
-  Object.keys(flags).forEach(flag => { flags[flag] = true })
-  expect(activatedFeatures).toEqual(flags)
+  expect(activatedFeatures[agentIdentifier]).toEqual(flags)
 })
 
 test('emits the right events when feature flag = 0', () => {
-  const flags = {}
-  Object.keys(bucketMap).forEach(flag => { flags[flag] = 0 })
+  const flags = {
+    stn: 1,
+    err: 1,
+    ins: 1,
+    spa: 1,
+    sr: 1
+  }
   activateFeatures(flags, agentIdentifier)
 
-  const sharedEE = jest.mocked(eventEmitterModule.ee.get).mock.results[0].value
+  const sharedEE = eventEmitterModule.ee.get(agentIdentifier).emit
 
-  // each flag gets emitted to each of its mapped features, and a block- AND a rumresp- for every emit, so (1+2+1+1+2+2)*2 = 16
-  expect(handleModule.handle).toHaveBeenCalledTimes(16)
-  expect(handleModule.handle).toHaveBeenNthCalledWith(1, 'block-stn', [], undefined, FEATURE_NAMES.sessionTrace, sharedEE)
-  expect(handleModule.handle).toHaveBeenLastCalledWith('rumresp-sr', [false], undefined, FEATURE_NAMES.sessionTrace, sharedEE)
+  expect(sharedEE).toHaveBeenCalledTimes(1)
+  expect(sharedEE).toHaveBeenLastCalledWith('rumresp', [flags])
 
-  Object.keys(flags).forEach(flag => { flags[flag] = false })
-  expect(activatedFeatures).toEqual(flags)
+  expect(activatedFeatures[agentIdentifier]).toEqual(flags)
 })
 
 test('only the first activate of the same feature is respected', () => {
   activateFeatures({ stn: 1 }, agentIdentifier)
-  activateFeatures({ stn: 0 }, agentIdentifier)
 
-  const sharedEE1 = jest.mocked(eventEmitterModule.ee.get).mock.results[0].value
-  const sharedEE2 = jest.mocked(eventEmitterModule.ee.get).mock.results[1].value
+  const sharedEE = eventEmitterModule.ee.get(agentIdentifier).emit
 
-  expect(handleModule.handle).toHaveBeenNthCalledWith(1, 'feat-stn', [], undefined, 'session_trace', sharedEE1)
-  expect(handleModule.handle).toHaveBeenNthCalledWith(2, 'rumresp-stn', [true], undefined, 'session_trace', sharedEE1)
-  expect(handleModule.handle).not.toHaveBeenNthCalledWith(1, 'feat-stn', [], undefined, 'session_trace', sharedEE2)
-  expect(activatedFeatures.stn).toBeTruthy()
+  expect(sharedEE).toHaveBeenNthCalledWith(1, 'rumresp', [{ stn: 1 }])
+
+  sharedEE.mockClear()
+  activateFeatures({ stn: 0 }, agentIdentifier)
+  expect(activatedFeatures[agentIdentifier].stn).toBeTruthy()
 })