From 37ae0a235079de46c45746ff35b0716b45bef851 Mon Sep 17 00:00:00 2001 From: Chunwai Li Date: Wed, 8 Jan 2025 10:55:46 -0600 Subject: [PATCH] Make Metrics harvest only on EoL for new harvester --- src/common/harvest/harvester.js | 2 +- src/features/metrics/aggregate/index.js | 2 +- src/features/utils/aggregate-base.js | 14 +++++++------- tests/components/metrics/aggregate.test.js | 19 ++++++++++++++----- tests/components/setup-agent.js | 4 ++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/common/harvest/harvester.js b/src/common/harvest/harvester.js index ba7fd072c..3f6ee2e72 100644 --- a/src/common/harvest/harvester.js +++ b/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] diff --git a/src/features/metrics/aggregate/index.js b/src/features/metrics/aggregate/index.js index a4df66c28..867829f62 100644 --- a/src/features/metrics/aggregate/index.js +++ b/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 diff --git a/src/features/utils/aggregate-base.js b/src/features/utils/aggregate-base.js index 5a5a95034..43f6a5d4a 100644 --- a/src/features/utils/aggregate-base.js +++ b/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. diff --git a/tests/components/metrics/aggregate.test.js b/tests/components/metrics/aggregate.test.js index 43330e9a2..039fad1e9 100644 --- a/tests/components/metrics/aggregate.test.js +++ b/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) diff --git a/tests/components/setup-agent.js b/tests/components/setup-agent.js index 863aeda45..9306abcd9 100644 --- a/tests/components/setup-agent.js +++ b/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)