Skip to content

Commit

Permalink
fix: Allow the page view feature to have access to an event buffer (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
metal-messiah authored Jan 9, 2025
1 parent 8abf047 commit 64babe1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 9 deletions.
1 change: 0 additions & 1 deletion src/features/page_view_event/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export class Aggregate extends AggregateBase {

postHarvestCleanup ({ status, responseText, xhr }) {
const rumEndTime = now()
this.blocked = true // this prevents harvester from polling this feature's event buffer (DNE) on interval; in other words, harvests will skip PVE

if (status >= 400 || status === 0) {
warn(18, status)
Expand Down
6 changes: 4 additions & 2 deletions src/features/utils/aggregate-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ export class AggregateBase extends FeatureBase {

// This switch needs to be after doOnceForAllAggregate which may new sharedAggregator and reset mainAppKey.
switch (this.featureName) {
// PVE has no need for eventBuffer, and SessionTrace + Replay have their own storage mechanisms.
case FEATURE_NAMES.pageViewEvent:
// SessionTrace + Replay have their own storage mechanisms.
case FEATURE_NAMES.sessionTrace:
case FEATURE_NAMES.sessionReplay:
break
Expand All @@ -28,6 +27,9 @@ export class AggregateBase extends FeatureBase {
case FEATURE_NAMES.metrics:
this.events = agentRef.sharedAggregator
break
/** All other features get EventBuffer in the ESM by default. Note: PVE is included here, but event buffer will always be empty so future harvests will still not happen by interval or EOL.
This was necessary to prevent race cond. issues where the event buffer was checked before the feature could "block" itself.
Its easier to just keep an empty event buffer in place. */
default:
this.events = new EventStoreManager(agentRef.mainAppKey, 1)
break
Expand Down
32 changes: 32 additions & 0 deletions tests/components/page_view_event/aggregate.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { setupAgent } from '../setup-agent'
import { Instrument as PageViewEvent } from '../../../src/features/page_view_event/instrument'

let mainAgent, pveAggregate

beforeAll(async () => {
mainAgent = setupAgent()
mainAgent.info.errorBeacon = undefined // this prevents Harvester from actually running its `send` method
})
beforeEach(async () => {
const pveInstrument = new PageViewEvent(mainAgent)
await new Promise(process.nextTick)
pveAggregate = pveInstrument.featAggregate
})

test('PageViewEvent does not throw on Harvester driven processes', () => {
expect(pveAggregate.blocked).toEqual(false)
expect(() => pveAggregate.makeHarvestPayload(true)).not.toThrow()

// The following both don't send anything since PVE buffer is meant to stay empty, so they return false.
expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate)).toEqual(false) // mimics what the harvester does on interval
expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate, { isFinalHarvest: true })).toEqual(false) // mimics what the harvester does on EoL

expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate, {
directSend: {
targetApp: 'someApp',
payload: 'blah'
},
needResponse: true,
sendEmptyBody: true
})).toEqual(true) // mimics the manual trigger in PVE `sendRum`; this should return true as it actually tries to "send"
})
11 changes: 5 additions & 6 deletions tests/unit/features/utils/aggregate-base.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,16 @@ test('does not initialized Aggregator more than once with multiple features', as
expect(mainAgent.mainAppKey).toBeUndefined()

new AggregateBase(mainAgent, FEATURE_NAMES.pageViewEvent)
expect(EventStoreManager).toHaveBeenCalledTimes(1)
expect(EventStoreManager).toHaveBeenCalledTimes(2) // once for runtime.sharedAgg + once for PVE.events
expect(EventStoreManager).toHaveBeenCalledWith(mainAgent.mainAppKey, 2) // 2 = initialize EventAggregator
expect(mainAgent.mainAppKey).toBeTruthy()
expect(mainAgent.sharedAggregator).toBeTruthy()

new AggregateBase(mainAgent, FEATURE_NAMES.jserrors) // this feature should be using that same aggregator as its .events
expect(EventStoreManager).toHaveBeenCalledTimes(1)
expect(EventStoreManager).toHaveBeenCalledTimes(2)

new AggregateBase(mainAgent, FEATURE_NAMES.pageViewTiming) // PVT should use its own EventStoreManager
expect(EventStoreManager).toHaveBeenCalledTimes(2)
expect(EventStoreManager).toHaveBeenCalledTimes(3)
expect(EventStoreManager).toHaveBeenCalledWith(mainAgent.mainAppKey, 1) // 1 = initialize EventBuffer
})

Expand All @@ -175,10 +175,9 @@ test('does initialize separate Aggregators with multiple agents', async () => {

new AggregateBase(mainAgent, FEATURE_NAMES.pageViewEvent)
new AggregateBase(mainAgent2, FEATURE_NAMES.pageViewEvent)
expect(EventStoreManager).toHaveBeenCalledTimes(2)
expect(EventStoreManager).not.toHaveBeenCalledWith(expect.any(Object), 1)
expect(EventStoreManager).toHaveBeenCalledTimes(4) // runtime.sharedAgg + PVE.events but multiply by having 2 agents

new AggregateBase(mainAgent, FEATURE_NAMES.jserrors) // still does not initialize sharedAgg again on the same agent
new AggregateBase(mainAgent2, FEATURE_NAMES.jserrors)
expect(EventStoreManager).toHaveBeenCalledTimes(2)
expect(EventStoreManager).toHaveBeenCalledTimes(4)
})

0 comments on commit 64babe1

Please sign in to comment.