fix: Make Metrics harvest only on EoL for new Harvester (#1311)

src/common/harvest/harvester.js

@@ -57,7 +57,7 @@ export class Harvester {
     const shouldRetryOnFail = !localOpts.isFinalHarvest && submitMethod === xhrMethod // always retry all features harvests except for final
     let dataToSendArr; let ranSend = false
     if (!localOpts.directSend) { // primarily used by rum call to bypass makeHarvestPayload by providing payload directly
-      dataToSendArr = aggregateInst.makeHarvestPayload(shouldRetryOnFail) // be sure the 'this' of makeHarvestPayload is the aggregate w/ access to its harvestOpts
+      dataToSendArr = aggregateInst.makeHarvestPayload(shouldRetryOnFail, localOpts) // be sure the 'this' of makeHarvestPayload is the aggregate w/ access to its harvestOpts
       if (!dataToSendArr) return false // can be undefined if storage is empty or preharvest checks failed
     } else dataToSendArr = [localOpts.directSend]
 

src/features/metrics/aggregate/index.js

@@ -34,7 +34,7 @@ export class Aggregate extends AggregateBase {
     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
+  preHarvestChecks (opts) { return this.drained && opts.isFinalHarvest } // only allow any metrics to be sent after we get the right RUM flag and only on EoL
 
   storeSupportabilityMetrics (name, value) {
     if (this.blocked) return

src/features/utils/aggregate-base.js

@@ -71,18 +71,18 @@ export class AggregateBase extends FeatureBase {
   /**
    * Return harvest payload. A "serializer" function can be defined on a derived class to format the payload.
    * @param {Boolean} shouldRetryOnFail - harvester flag to backup payload for retry later if harvest request fails; this should be moved to harvester logic
-   * @param {object|undefined} target - the target app passed onto the event store manager to determine which app's data to return; if none provided, all apps data will be returned
+   * @param {object|undefined} opts.target - the target app passed onto the event store manager to determine which app's data to return; if none provided, all apps data will be returned
    * @returns {Array} Final payload tagged with their targeting browser app. The value of `payload` can be undefined if there are no pending events for an app. This should be a minimum length of 1.
    */
-  makeHarvestPayload (shouldRetryOnFail = false, target) {
-    if (this.events.isEmpty(this.harvestOpts, target)) return
+  makeHarvestPayload (shouldRetryOnFail = false, opts = {}) {
+    if (this.events.isEmpty(this.harvestOpts, opts.target)) return
     // Other conditions and things to do when preparing harvest that is required.
-    if (this.preHarvestChecks && !this.preHarvestChecks()) return
+    if (this.preHarvestChecks && !this.preHarvestChecks(opts)) return
 
-    if (shouldRetryOnFail) this.events.save(this.harvestOpts, target)
-    const returnedDataArr = this.events.get(this.harvestOpts, target)
+    if (shouldRetryOnFail) this.events.save(this.harvestOpts, opts.target)
+    const returnedDataArr = this.events.get(this.harvestOpts, opts.target)
     if (!returnedDataArr.length) throw new Error('Unexpected problem encountered. There should be at least one app for harvest!')
-    this.events.clear(this.harvestOpts, target)
+    this.events.clear(this.harvestOpts, opts.target)
 
     return returnedDataArr.map(({ targetApp, data }) => {
       // A serializer or formatter assists in creating the payload `body` from stored events on harvest when defined by derived feature class.

tests/components/metrics/aggregate.test.js

@@ -30,7 +30,7 @@ afterEach(() => {
   test(`${name} with no value creates a metric with just a count`, () => {
     createAndStoreMetric(undefined, isSupportability)
 
-    const records = metricsAggregate.events.get([type])[type]
+    const records = metricsAggregate.events.get({ aggregatorTypes: [type] })[0].data[type]
       .filter(x => x?.params?.name === metricName)
     expect(records.length).toEqual(1)
 
@@ -47,7 +47,7 @@ afterEach(() => {
     createAndStoreMetric(undefined, isSupportability)
     createAndStoreMetric(undefined, isSupportability)
 
-    const records = metricsAggregate.events.get([type])[type]
+    const records = metricsAggregate.events.get({ aggregatorTypes: [type] })[0].data[type]
       .filter(x => x?.params?.name === metricName)
     expect(records.length).toEqual(1)
 
@@ -62,7 +62,7 @@ afterEach(() => {
   test(`${name} with a value ${auxDescription}`, () => {
     createAndStoreMetric(isSupportability ? 500 : { time: 500 }, isSupportability)
 
-    const records = metricsAggregate.events.get([type])[type]
+    const records = metricsAggregate.events.get({ aggregatorTypes: [type] })[0].data[type]
       .filter(x => x?.params?.name === metricName)
     expect(records.length).toEqual(1)
 
@@ -80,7 +80,7 @@ afterEach(() => {
       .fill(null).map(() => faker.number.int({ min: 100, max: 1000 }))
     values.forEach(v => createAndStoreMetric(isSupportability ? v : { time: v }, isSupportability))
 
-    const records = metricsAggregate.events.get([type])[type]
+    const records = metricsAggregate.events.get({ aggregatorTypes: [type] })[0].data[type]
       .filter(x => x?.params?.name === metricName)
     expect(records.length).toEqual(1)
 
@@ -108,7 +108,7 @@ afterEach(() => {
     test('storeEvent (custom) with an invalid value type does not create a named metric object in metrics section', () => {
       createAndStoreMetric(faker.number.float(), false)
 
-      const records = metricsAggregate.events.get([CUSTOM_METRIC])[CUSTOM_METRIC]
+      const records = metricsAggregate.events.get({ aggregatorTypes: [CUSTOM_METRIC] })[0].data[CUSTOM_METRIC]
         .filter(x => x?.params?.name === metricName)
       expect(records.length).toEqual(1)
 
@@ -118,6 +118,15 @@ afterEach(() => {
   }
 })
 
+test('Metrics does not harvest on interval, only on EoL', () => {
+  createAndStoreMetric(undefined, true)
+  expect(metricsAggregate.events.isEmpty(metricsAggregate.harvestOpts)).toEqual(false) // double check so that harvest should proceed
+  metricsAggregate.drained = true // this is a requisite for harvest in preHarvestChecks
+
+  expect(mainAgent.runtime.harvester.triggerHarvestFor(metricsAggregate)).toEqual(false) // mimics what the harveseter does on interval
+  expect(mainAgent.runtime.harvester.triggerHarvestFor(metricsAggregate, { isFinalHarvest: true })).toEqual(true) // mimics what the harveseter does on EoL
+})
+
 function createAndStoreMetric (value, isSupportability) {
   const method = isSupportability ? (...args) => metricsAggregate.storeSupportabilityMetrics(...args) : (...args) => metricsAggregate.storeEventMetrics(...args)
   method(metricName, value)

tests/components/setup-agent.js

@@ -5,8 +5,8 @@ import { ee } from '../../src/common/event-emitter/contextual-ee'
 import { TimeKeeper } from '../../src/common/timing/time-keeper'
 import { getRuntime } from '../../src/common/config/runtime'
 import { setupAgentSession } from '../../src/features/utils/agent-session'
-import { EventAggregator } from '../../src/common/aggregate/event-aggregator'
 import { Harvester } from '../../src/common/harvest/harvester'
+import { EventStoreManager } from '../../src/features/utils/event-store-manager'
 
 /**
  * Sets up a new agent for component testing. This should be called only
@@ -39,7 +39,7 @@ export function setupAgent ({ agentOverrides = {}, info = {}, init = {}, loaderC
   const fakeAgent = {
     agentIdentifier,
     ee: eventEmitter,
-    sharedAggregator: new EventAggregator(),
+    sharedAggregator: new EventStoreManager({ licenseKey: info.licenseKey, appId: info.applicationID }, 2),
     ...agentOverrides
   }
   setNREUMInitializedAgent(agentIdentifier, fakeAgent)