Fix metrics harvest on session reset before drain & autoStart considerations in harvester

Author: cwli24

docs/warning-codes.md

@@ -92,4 +92,6 @@
 ### 45
 `An internal agent process failed to execute.`
 ### 46
-`A reserved eventType was provided to recordCustomEvent(...) -- The event was not recorded.`
+`A reserved eventType was provided to recordCustomEvent(...) -- The event was not recorded.`
+### 47
+`A feature was marked for auto starting but has still not started importing its aggregate.`

src/common/harvest/harvester.js

@@ -15,10 +15,16 @@ export class Harvester {
   constructor (agentRef) {
     this.agentRef = agentRef
 
-    const featuresInstruments = Object.values(agentRef.features)
-    Promise.all(featuresInstruments.map(feature => feature.onAggregateImported)).then(loadedSuccessfullyArr => {
+    const autoFeaturesInstruments = Object.values(agentRef.features).filter(feature => feature.auto) // features without autoStart won't be fully initialized until later
+    const importingFeatures = []
+    // Since the harvester is expected to be initialized by PVE Aggregate module, at such time, all the features should be awaiting import though featAggregate may not be available just yet.
+    for (const feature of autoFeaturesInstruments) {
+      if (!feature.onAggregateImported) warn(47, feature.featureName)
+      else importingFeatures.push(feature)
+    }
+    Promise.all(importingFeatures.map(instrument => instrument.onAggregateImported)).then(loadedSuccessfullyArr => {
       // Double check that all aggregates have been initialized, successfully or not, before starting harvest schedule, which only queries the succesfully loaded ones.
-      const featuresToHarvest = featuresInstruments.filter((instrumentInstance, index) => loadedSuccessfullyArr[index])
+      const featuresToHarvest = importingFeatures.filter((instrumentInstance, index) => loadedSuccessfullyArr[index])
       this.initializedAggregates = featuresToHarvest.map(instrumentInstance => instrumentInstance.featAggregate)
       this.#startTimer(agentRef.init.harvest.interval)
     })

src/features/metrics/aggregate/index.js

@@ -19,8 +19,6 @@ export class Aggregate extends AggregateBase {
 
     this.waitForFlags(['err']).then(([errFlag]) => {
       if (errFlag) {
-        this.singleChecks() // checks that are run only one time, at script load
-        this.eachSessionChecks() // the start of every time user engages with page
         this.drain()
       } else {
         this.blocked = true // if rum response determines that customer lacks entitlements for spa endpoint, this feature shouldn't harvest
@@ -31,8 +29,13 @@ export class Aggregate extends AggregateBase {
     // Allow features external to the metrics feature to capture SMs and CMs through the event emitter
     registerHandler(SUPPORTABILITY_METRIC_CHANNEL, this.storeSupportabilityMetrics.bind(this), this.featureName, this.ee)
     registerHandler(CUSTOM_METRIC_CHANNEL, this.storeEventMetrics.bind(this), this.featureName, this.ee)
+
+    this.singleChecks() // checks that are run only one time, at script load
+    this.eachSessionChecks() // the start of every time user engages with page
   }
 
+  preHarvestChecks () { return this.drained } // only allow any metrics to be sent if we know for sure it has gotten the go-ahead RUM flag
+
   storeSupportabilityMetrics (name, value) {
     if (this.blocked) return
     const type = SUPPORTABILITY_METRIC

src/features/utils/instrument-base.js

@@ -43,7 +43,6 @@ export class InstrumentBase extends FeatureBase {
     /**
      * @type {Promise} Assigned immediately after @see importAggregator runs. Serves as a signal for when the inner async fn finishes execution. Useful for features to await
      * one another if there are inter-features dependencies.
-     * TODO: This is only used for the SPA feature component tests and should be refactored out.
     */
     this.onAggregateImported = undefined
 
@@ -58,6 +57,9 @@ export class InstrumentBase extends FeatureBase {
         registerDrain(agentRef.agentIdentifier, this.featureName)
         this.auto = true
         this.importAggregator(agentRef)
+        this.onAggregateImported.then(loaded => { // "subscribe" the feature aggregate to harvest interval when it's ready
+          if (loaded) agentRef.runtime.harvester.initializedAggregates.push(this.featAggregate)
+        })
       }))
     }
   }

tests/unit/features/utils/instrument-base.test.js

@@ -54,9 +54,10 @@ test('should construct a new instrument', () => {
   expect(registerDrain).toHaveBeenCalledWith(agentIdentifier, featureName)
 })
 
-test('should wait for feature opt-in to import the aggregate', () => {
+test('should wait for feature opt-in to import the aggregate', async () => {
   const instrument = new InstrumentBase(agentBase, featureName, false)
-  jest.spyOn(instrument, 'importAggregator').mockImplementation(jest.fn)
+  agentBase.runtime = { harvester: { initializedAggregates: [] } }
+  jest.spyOn(instrument, 'importAggregator').mockImplementation(() => { instrument.onAggregateImported = Promise.resolve(true) })
 
   expect(registerDrain).not.toHaveBeenCalled()
   expect(instrument.auto).toEqual(false)
@@ -67,6 +68,9 @@ test('should wait for feature opt-in to import the aggregate', () => {
   expect(registerDrain).toHaveBeenCalledWith(agentIdentifier, featureName)
   expect(instrument.importAggregator).toHaveBeenCalledTimes(1)
   expect(instrument.auto).toEqual(true)
+
+  await new Promise(process.nextTick)
+  expect(agentBase.runtime.harvester.initializedAggregates).toContain(instrument.featAggregate)
 })
 
 test('should import aggregator on window load', async () => {