feat: Prevent early harvests when in retry period (#1548)

Co-authored-by: Chunwai Li <cli@newrelic.com>

Author: metal-messiah

src/features/utils/aggregate-base.js

@@ -35,7 +35,9 @@ export class AggregateBase extends FeatureBase {
     /** @type {Boolean} indicates if custom attributes are combined in each event payload for size estimation purposes. this is set to true in derived classes that need to evaluate custom attributes separately from the event payload */
     this.customAttributesAreSeparate = false
     /** @type {Boolean} indicates if the feature can harvest early. This is set to false in derived classes that need to block early harvests, like ajax under certain conditions */
-    this.canHarvestEarly = true // this is set to false in derived classes that need to block early harvests, like ajax under certain conditions
+    this.canHarvestEarly = true
+    /** @type {Boolean} indicates if the feature is actively in a retry deferral period */
+    this.isRetrying = false
 
     this.harvestOpts = {} // features aggregate classes can define custom opts for when their harvest is called
 
@@ -82,7 +84,7 @@ export class AggregateBase extends FeatureBase {
    * @returns void
    */
   decideEarlyHarvest () {
-    if (!this.canHarvestEarly) return
+    if (!this.canHarvestEarly || this.blocked || this.isRetrying) return
     const estimatedSize = this.events.byteSize() + (this.customAttributesAreSeparate ? this.agentRef.runtime.jsAttributesMetadata.bytes : 0)
     if (estimatedSize > IDEAL_PAYLOAD_SIZE) {
       this.agentRef.runtime.harvester.triggerHarvestFor(this)
@@ -169,8 +171,8 @@ export class AggregateBase extends FeatureBase {
    * @param {boolean=} result.retry - whether the harvest should be retried
    */
   postHarvestCleanup (result = {}) {
-    const harvestFailed = result.sent && result.retry
-    if (harvestFailed) this.events.reloadSave(this.harvestOpts, result.targetApp?.entityGuid)
+    this.isRetrying = result.sent && result.retry
+    if (this.isRetrying) this.events.reloadSave(this.harvestOpts, result.targetApp?.entityGuid)
     this.events.clearSave(this.harvestOpts, result.targetApp?.entityGuid)
   }
 

tests/assets/harvest-early-block-internal.html

@@ -19,7 +19,6 @@
       newrelic.setCustomAttribute("foo", "bar".repeat(16000));
       newrelic.addPageAction("test");
       newrelic.log("test");
-
       window.sendAjax = function () {
         fetch("https://pokeapi.co/api/v2/pokemon/moltres");
       };

tests/specs/harvesting/harvest-early.e2e.js

@@ -1,18 +1,20 @@
-import { testAjaxEventsRequest, testInsRequest, testInteractionEventsRequest, testLogsRequest, testBlobTraceRequest } from '../../../tools/testing-server/utils/expect-tests'
+import { testAjaxEventsRequest, testInsRequest, testInteractionEventsRequest, testLogsRequest, testMetricsRequest, testBlobTraceRequest } from '../../../tools/testing-server/utils/expect-tests'
 
 describe('should harvest early', () => {
   let ajaxEventsCapture
   let insightsCapture
   let interactionEventsCapture
   let loggingEventsCapture
+  let metricsCapture
   let testBlobTraceCapture
 
   beforeEach(async () => {
-    ;[ajaxEventsCapture, insightsCapture, interactionEventsCapture, loggingEventsCapture, testBlobTraceCapture] = await browser.testHandle.createNetworkCaptures('bamServer', [
+    [ajaxEventsCapture, insightsCapture, interactionEventsCapture, loggingEventsCapture, metricsCapture, testBlobTraceCapture] = await browser.testHandle.createNetworkCaptures('bamServer', [
       { test: testAjaxEventsRequest },
       { test: testInsRequest },
       { test: testInteractionEventsRequest },
       { test: testLogsRequest },
+      { test: testMetricsRequest },
       { test: testBlobTraceRequest }
     ])
   })
@@ -36,6 +38,30 @@ describe('should harvest early', () => {
     expect(Date.now() - timeStart).toBeLessThan(30000) // should have harvested early before 30 seconds
   })
 
+  it('should NOT re-attempt to harvest early when rate limited', async () => {
+    await browser.testHandle.scheduleReply('bamServer', {
+      test: testInsRequest,
+      statusCode: 429
+    })
+
+    await browser.url(await browser.testHandle.assetURL('harvest-early-block-internal.html', { init: { harvest: { interval: 30 } } }))
+      .then(() => browser.waitForAgentLoad())
+
+    /** not harvesting early in retry mode */
+    const [smHarvest] = await Promise.all([
+      metricsCapture.waitForResult({ totalCount: 1 }),
+      browser.execute(function () {
+        newrelic.addPageAction('test')
+      })
+        .then(() => browser.pause(10000))
+        .then(() => browser.refresh())
+    ])
+
+    const insHarvestEarlySeen = smHarvest[0].request.body.sm.find(sm => sm.params.name === 'generic_events/Harvest/Early/Seen')
+    // count (c) only exists if the same label is called more than once.  It should have only early harvested once (on page load), which caused it to be denied by 429 by the scheduleReply.  It should NOT try to early harvest twice since it is in retry mode.
+    expect(insHarvestEarlySeen.stats.c).toBeUndefined()
+  })
+
   /** if we track internal and spawn early requests, we can potentially create a feedback loop that goes on forever with large ajax requests describing themselves */
   it('should not harvest AJAX early when agent is tracking internal calls', async () => {
     await browser.url(await browser.testHandle.assetURL('harvest-early.html'))