Merge branch 'NR-443294-navigations' into tmp-user-frustrations

Author: ptang-nr

src/features/generic_events/aggregate/index.js

@@ -59,7 +59,7 @@ export class Aggregate extends AggregateBase {
 
       let addUserAction = () => { /** no-op */ }
       if (isBrowserScope && agentRef.init.user_actions.enabled) {
-        this.userActionAggregator = new UserActionsAggregator()
+        this.userActionAggregator = new UserActionsAggregator(agentRef.init.feature_flags.includes('user_frustrations'))
         this.harvestOpts.beforeUnload = () => addUserAction?.(this.userActionAggregator.aggregationEvent)
 
         addUserAction = (aggregatedUserAction) => {

src/features/generic_events/aggregate/user-actions/user-actions-aggregator.js

@@ -11,16 +11,16 @@ export class UserActionsAggregator {
   /** @type {AggregatedUserAction=} */
   #aggregationEvent = undefined
   #aggregationKey = ''
+  #ufEnabled = false
   #deadClickTimer = undefined
-  #domObserver = {
-    instance: undefined
-  }
+  #domObserver = undefined
 
   #errorClickTimer = undefined
 
-  constructor () {
-    if (MutationObserver) {
-      this.#domObserver.instance = new MutationObserver(this.treatAsLiveClick.bind(this))
+  constructor (userFrustrationsEnabled) {
+    if (userFrustrationsEnabled && MutationObserver) {
+      this.#domObserver = new MutationObserver(this.treatAsLiveClick.bind(this))
+      this.#ufEnabled = true
     }
   }
 
@@ -49,13 +49,13 @@ export class UserActionsAggregator {
     } else {
       // return the prev existing one (if there is one)
       const finishedEvent = this.#aggregationEvent
-      this.#deadClickCleanup()
-      this.#errorClickCleanup()
+      this.#ufEnabled && this.#deadClickCleanup()
+      this.#ufEnabled && this.#errorClickCleanup()
 
       // then start new event aggregation
       this.#aggregationKey = aggregationKey
       this.#aggregationEvent = new AggregatedUserAction(evt, selectorInfo)
-      if (evt.type === 'click' && (selectorInfo.hasButton || selectorInfo.hasLink)) {
+      if (this.#ufEnabled && evt.type === 'click' && (selectorInfo.hasButton || selectorInfo.hasLink)) {
         this.#deadClickSetup(this.#aggregationEvent)
         this.#errorClickSetup()
       }
@@ -95,14 +95,14 @@ export class UserActionsAggregator {
   }
 
   #deadClickCleanup () {
-    this.#domObserver.instance?.disconnect()
+    this.#domObserver?.disconnect()
     this.#deadClickTimer?.clear()
     this.#deadClickTimer = undefined
   }
 
   #startObserver () {
-    if (!this.isEvaluatingDeadClick() && this.#domObserver.instance) {
-      this.#domObserver.instance.observe(document, {
+    if (!this.isEvaluatingDeadClick() && this.#domObserver) {
+      this.#domObserver.observe(document, {
         attributes: true,
         characterData: true,
         childList: true,

src/features/session_replay/instrument/index.js

@@ -10,7 +10,7 @@ import { handle } from '../../../common/event-emitter/handle'
 import { DEFAULT_KEY, MODE, PREFIX } from '../../../common/session/constants'
 import { InstrumentBase } from '../../utils/instrument-base'
 import { hasReplayPrerequisite, isPreloadAllowed } from '../shared/utils'
-import { FEATURE_NAME, SR_EVENT_EMITTER_TYPES } from '../constants'
+import { FEATURE_NAME, SR_EVENT_EMITTER_TYPES, TRIGGERS } from '../constants'
 import { setupRecordReplayAPI } from '../../../loaders/api/recordReplay'
 import { setupPauseReplayAPI } from '../../../loaders/api/pauseReplay'
 
@@ -69,7 +69,7 @@ export class Instrument extends InstrumentBase {
   /**
    * This func is use for early pre-load recording prior to replay feature (agg) being loaded onto the page. It should only setup once, including if already called and in-progress.
    */
-  async #preloadStartRecording () {
+  async #preloadStartRecording (trigger) {
     if (this.#alreadyStarted) return
     this.#alreadyStarted = true
 
@@ -78,7 +78,7 @@ export class Instrument extends InstrumentBase {
 
       // If startReplay() has been used by this point, we must record in full mode regardless of session preload:
       // Note: recorder starts here with w/e the mode is at this time, but this may be changed later (see #apiStartOrRestartReplay else-case)
-      this.recorder ??= new Recorder({ ...this, mode: this.#mode, agentRef: this.#agentRef, timeKeeper: this.#agentRef.runtime.timeKeeper }) // if TK exists due to deferred state, pass it
+      this.recorder ??= new Recorder({ ...this, mode: this.#mode, agentRef: this.#agentRef, trigger, timeKeeper: this.#agentRef.runtime.timeKeeper }) // if TK exists due to deferred state, pass it
       this.recorder.startRecording()
       this.abortHandler = this.recorder.stopRecording
     } catch (err) {
@@ -95,7 +95,7 @@ export class Instrument extends InstrumentBase {
       if (this.featAggregate.mode !== MODE.FULL) this.featAggregate.initializeRecording(MODE.FULL, true)
     } else { // pre-load
       this.#mode = MODE.FULL
-      this.#preloadStartRecording()
+      this.#preloadStartRecording(TRIGGERS.API)
       // There's a race here wherein either:
       // a. Recorder has not been initialized, and we've set the enforced mode, so we're good, or;
       // b. Record has been initialized, possibly with the "wrong" mode, so we have to correct that + restart.

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'))

tests/specs/npm/index.e2e.js

@@ -95,6 +95,7 @@ describe('basic npm agent', () => {
     expect(agentSession.localStorage).toEqual({})
   })
 
+  // Note: the order of this test and the "session manager cannot be imported" test is important
   it('vite-react-wrapper sends basic calls', async () => {
     const [rumHarvests] = await Promise.all([
       rumCapture.waitForResult({ totalCount: 1 }),

tests/unit/features/generic_events/aggregate/user-actions/user-actions-aggregator.test.js

@@ -80,16 +80,32 @@ describe('UserActionsAggregator - Dead Clicks', () => {
   let aggregator
   beforeEach(() => {
     jest.useFakeTimers()
-    aggregator = new UserActionsAggregator()
+    aggregator = new UserActionsAggregator(true)
   })
   afterEach(() => {
     jest.useRealTimers()
   })
+
+  test('should NOT set deadClick if user frustrations is disabled', () => {
+    aggregator = new UserActionsAggregator(false)
+    const link = document.createElement('a')
+    document.body.appendChild(link)
+    const evt = { type: 'click', target: link }
+    aggregator.process(evt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(false)
+
+    jest.advanceTimersByTime(2000)
+
+    const userAction = aggregator.aggregationEvent
+    expect(userAction.deadClick).toBe(false)
+  })
+
   test('should set deadClick to true if no change detected after 2 seconds - buttons', () => {
     const btn = document.createElement('button')
     document.body.appendChild(btn)
     const evt = { type: 'click', target: btn }
     aggregator.process(evt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(true)
 
     jest.advanceTimersByTime(2000)
 
@@ -102,6 +118,7 @@ describe('UserActionsAggregator - Dead Clicks', () => {
     document.body.appendChild(link)
     const evt = { type: 'click', target: link }
     aggregator.process(evt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(true)
 
     jest.advanceTimersByTime(2000)
 
@@ -114,6 +131,7 @@ describe('UserActionsAggregator - Dead Clicks', () => {
     document.body.appendChild(span)
     const evt = { type: 'click', target: span }
     aggregator.process(evt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(false)
 
     jest.advanceTimersByTime(2000)
 
@@ -126,6 +144,7 @@ describe('UserActionsAggregator - Dead Clicks', () => {
     document.body.appendChild(btn)
     const evt = { type: 'click', target: btn }
     aggregator.process(evt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(true)
 
     // Simulate a DOM mutation before the timer ends
     btn.setAttribute('data-test', 'mutated')
@@ -159,6 +178,7 @@ describe('UserActionsAggregator - Dead Clicks', () => {
     const keydownEvt = { type: 'keydown', target: btn }
 
     aggregator.process(clickEvt)
+    expect(aggregator.isEvaluatingDeadClick()).toBe(true)
     const finishedEvent = aggregator.process(keydownEvt) // Ends aggregation before timer
 
     jest.advanceTimersByTime(2000)
@@ -171,11 +191,31 @@ describe('UserActionsAggregator - Error Clicks', () => {
   let aggregator
   beforeEach(() => {
     jest.useFakeTimers()
-    aggregator = new UserActionsAggregator()
+    aggregator = new UserActionsAggregator(true)
   })
   afterEach(() => {
     jest.useRealTimers()
   })
+  test('should NOT set errorClick if user frustrations is disabled', () => {
+    aggregator = new UserActionsAggregator(false)
+    const btn = document.createElement('button')
+    btn.onclick = () => {
+      console.log('Simulating an error')
+      throw new Error('Simulated error')
+    }
+    document.body.appendChild(btn)
+    const evt = {
+      type: 'click',
+      target: btn
+    }
+    aggregator.process(evt, ['id', 'className', 'tagName', 'type'])
+    jest.advanceTimersByTime(1999)
+    aggregator.markAsErrorClick() // Simulate the error click
+    jest.advanceTimersByTime(1)
+
+    const userAction = aggregator.aggregationEvent
+    expect(userAction.errorClick).toBe(false)
+  })
   test('should set errorClick to true if an error is detected within 2 seconds - buttons', () => {
     const btn = document.createElement('button')
     btn.onclick = () => {