Skip to content

Commit

Permalink
fix: Harvest generic events when max size is reached (#1250)
Browse files Browse the repository at this point in the history
  • Loading branch information
metal-messiah authored Nov 21, 2024
1 parent b1e4113 commit e00a469
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 40 deletions.
2 changes: 1 addition & 1 deletion .github/actions/build-ab/templates/released.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ window.NREUM={
enabled: true
},
performance: {
capture_marks: true,
capture_marks: false,
capture_measures: true
},
proxy: {}
Expand Down
22 changes: 10 additions & 12 deletions src/features/generic_events/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { now } from '../../../common/timing/now'
import { registerHandler } from '../../../common/event-emitter/register-handler'
import { SUPPORTABILITY_METRIC_CHANNEL } from '../../metrics/constants'
import { applyFnToProps } from '../../../common/util/traverse'
import { IDEAL_PAYLOAD_SIZE } from '../../../common/constants/agent-constants'
import { FEATURE_TO_ENDPOINT } from '../../../loaders/features/features'
import { UserActionsAggregator } from './user-actions/user-actions-aggregator'
import { isIFrameWindow } from '../../../common/dom/iframe'
Expand Down Expand Up @@ -177,9 +176,16 @@ export class Aggregate extends AggregateBase {
...obj
}

this.events.add(eventAttributes)

this.checkEventLimits()
const addedEvent = this.events.add(eventAttributes)
if (!addedEvent && !this.events.isEmpty()) {
/** could not add the event because it pushed the buffer over the limit
* so we harvest early, and try to add it again now that the buffer is cleared
* if it fails again, we do nothing
*/
this.ee.emit(SUPPORTABILITY_METRIC_CHANNEL, ['GenericEvents/Harvest/Max/Seen'])
this.harvestScheduler.runHarvest()
this.events.add(eventAttributes)
}
}

serializer (eventBuffer) {
Expand All @@ -189,12 +195,4 @@ export class Aggregate extends AggregateBase {
queryStringsBuilder () {
return { ua: this.agentRef.info.userAttributes, at: this.agentRef.info.atts }
}

checkEventLimits () {
// check if we've reached any harvest limits...
if (this.events.byteSize() > IDEAL_PAYLOAD_SIZE) {
this.ee.emit(SUPPORTABILITY_METRIC_CHANNEL, ['GenericEvents/Harvest/Max/Seen'])
this.harvestScheduler.runHarvest()
}
}
}
17 changes: 14 additions & 3 deletions tests/components/generic_events/aggregate/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,30 @@ test('should warn if invalid event is provide', async () => {
expect(console.debug).toHaveBeenCalledWith('New Relic Warning: https://github.com/newrelic/newrelic-browser-agent/blob/main/docs/warning-codes.md#44', undefined)
})

test('should only buffer 64kb of events at a time', async () => {
test('should harvest early if will exceed 1mb', async () => {
genericEventsAggregate.ee.emit('rumresp', [{ ins: 1 }])

await new Promise(process.nextTick)

genericEventsAggregate.harvestScheduler.runHarvest = jest.fn()
genericEventsAggregate.addEvent({ name: 'test', eventType: 'x'.repeat(63000) })
genericEventsAggregate.addEvent({ name: 'test', eventType: 'x'.repeat(900000) })

expect(genericEventsAggregate.harvestScheduler.runHarvest).not.toHaveBeenCalled()
genericEventsAggregate.addEvent({ name: 1000, eventType: 'x'.repeat(1000) })
genericEventsAggregate.addEvent({ name: 1000, eventType: 'x'.repeat(100000) })
expect(genericEventsAggregate.harvestScheduler.runHarvest).toHaveBeenCalled()
})

test('should not harvest if single event will exceed 1mb', async () => {
genericEventsAggregate.ee.emit('rumresp', [{ ins: 1 }])

await new Promise(process.nextTick)

genericEventsAggregate.harvestScheduler.runHarvest = jest.fn()
genericEventsAggregate.addEvent({ name: 'test', eventType: 'x'.repeat(1000000) })

expect(genericEventsAggregate.harvestScheduler.runHarvest).not.toHaveBeenCalled()
})

describe('sub-features', () => {
beforeEach(async () => {
genericEventsAggregate.ee.emit('rumresp', [{ ins: 1 }])
Expand Down
27 changes: 3 additions & 24 deletions tests/specs/ins/harvesting.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('ins harvesting', () => {
pageUrl: expect.any(String),
timestamp: expect.any(Number)
})
expect(clickUAs[1].actionDuration).toBeGreaterThan(0)
expect(clickUAs[1].actionDuration).toBeGreaterThanOrEqual(0)
expect(clickUAs[1].actionMs).toEqual(expect.stringMatching(/^\[\d+(,\d+){4}\]$/))
})

Expand Down Expand Up @@ -167,35 +167,14 @@ describe('ins harvesting', () => {
insightsCapture.waitForResult({ timeout: 10000 }),
browser.execute(function () {
let i = 0
while (i++ < 1010) {
while (i++ < 10000) {
newrelic.addPageAction('foobar')
}
})
])
expect(insightsResult.length).toBeTruthy()
})

it('should harvest early when buffer gets too large (one big event)', async () => {
const testUrl = await browser.testHandle.assetURL('instrumented.html', { init: { generic_events: { harvestTimeSeconds: 30 } } })
await browser.url(testUrl)
.then(() => browser.waitForAgentLoad())

const [insightsResult] = await Promise.all([
insightsCapture.waitForResult({ timeout: 10000 }),
browser.execute(function () {
newrelic.addPageAction('foobar', createLargeObject())
function createLargeObject () {
let i = 0; let obj = {}
while (i++ < 64000) {
obj[i] = 'x'
}
return obj
}
})
])
expect(insightsResult.length).toBeTruthy()
})

it('should not harvest if too large', async () => {
const testUrl = await browser.testHandle.assetURL('instrumented.html')
await browser.url(testUrl)
Expand All @@ -207,7 +186,7 @@ describe('ins harvesting', () => {
newrelic.addPageAction('foobar', createLargeObject())
function createLargeObject () {
let i = 0; let obj = {}
while (i++ < 100000) {
while (i++ < 1000000) {
obj[i] = Math.random()
}
return obj
Expand Down

0 comments on commit e00a469

Please sign in to comment.