Skip to content

Commit

Permalink
Fix race cond on harvester initializedAggregates after optimization
Browse files Browse the repository at this point in the history
  • Loading branch information
cwli24 committed Jan 7, 2025
1 parent a9e22cd commit 84b6b0a
Show file tree
Hide file tree
Showing 13 changed files with 21 additions and 4 deletions.
6 changes: 6 additions & 0 deletions src/common/harvest/__mocks__/harvester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// create a jest mock of the Harvester class
export const Harvester = jest.fn().mockImplementation(() => ({
startTimer: jest.fn(),
triggerHarvestFor: jest.fn(),
initializedAggregates: []
}))
2 changes: 0 additions & 2 deletions src/features/page_view_event/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { timeToFirstByte } from '../../../common/vitals/time-to-first-byte'
import { now } from '../../../common/timing/now'
import { TimeKeeper } from '../../../common/timing/time-keeper'
import { applyFnToProps } from '../../../common/util/traverse'
import { Harvester } from '../../../common/harvest/harvester'

export class Aggregate extends AggregateBase {
static featureName = CONSTANTS.FEATURE_NAME
Expand All @@ -29,7 +28,6 @@ export class Aggregate extends AggregateBase {
return warn(43)
}
agentRef.runtime.timeKeeper = new TimeKeeper(agentRef.agentIdentifier)
agentRef.runtime.harvester = new Harvester(agentRef)

if (isBrowserScope) {
timeToFirstByte.subscribe(({ value, attrs }) => {
Expand Down
3 changes: 3 additions & 0 deletions src/features/utils/aggregate-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { activatedFeatures } from '../../common/util/feature-flags'
import { Obfuscator } from '../../common/util/obfuscate'
import { FEATURE_NAMES } from '../../loaders/features/features'
import { EventStoreManager } from './event-store-manager'
import { Harvester } from '../../common/harvest/harvester'

export class AggregateBase extends FeatureBase {
constructor (agentRef, featureName) {
Expand Down Expand Up @@ -145,5 +146,7 @@ export class AggregateBase extends FeatureBase {
if (!agentRef.mainAppKey) agentRef.mainAppKey = { licenseKey: agentRef.info.licenseKey, appId: agentRef.info.applicationID }
// Create a single Aggregator for this agent if DNE yet; to be used by jserror endpoint features.
if (!agentRef.sharedAggregator) agentRef.sharedAggregator = new EventStoreManager(agentRef.mainAppKey, 2)

if (!agentRef.runtime.harvester) agentRef.runtime.harvester = new Harvester(agentRef)
}
}
2 changes: 1 addition & 1 deletion src/features/utils/instrument-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class InstrumentBase extends FeatureBase {
const { Aggregate } = await lazyFeatureLoader(this.featureName, 'aggregate')
this.featAggregate = new Aggregate(agentRef, argsObjFromInstrument)

agentRef.runtime.harvester?.initializedAggregates.push(this.featAggregate) // "subscribe" the feature to harvest interval (PVE will start the timer)
agentRef.runtime.harvester.initializedAggregates.push(this.featAggregate) // "subscribe" the feature to future harvest intervals (PVE will start the timer)
loadedSuccessfully(true)
} catch (e) {
warn(34, e)
Expand Down
4 changes: 3 additions & 1 deletion tests/components/page_view_timing/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ let triggerVisChange
jest.mock('../../../src/common/window/page-visibility', () => ({
subscribeToVisibilityChange: jest.fn(cb => { triggerVisChange ??= cb })
}))
jest.mock('../../../src/common/harvest/harvester')

const expectedNetworkInfo = {
'net-type': expect.any(String),
Expand All @@ -54,7 +55,8 @@ describe('pvt aggregate tests', () => {
agentIdentifier,
info: { licenseKey: 'licenseKey', applicationID: 'applicationID' },
init: { page_view_timing: {} },
runtime: {}
ee: { on: jest.fn() },
runtime: { harvester: { initializedAggregates: [] } }
}
const { Aggregate } = await import('../../../src/features/page_view_timing/aggregate')

Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic, mockCurrentInfo
const agentIdentifier = 'abcdefg'
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/capturing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
const agentIdentifier = 'abcdefg'
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/initial-page-load.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
const agentIdentifier = 'abcdefg'
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/mutation-observer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
const agentIdentifier = 'abcdefg'
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/promises.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
jest.mock('../../../src/common/util/feature-flags', () => ({
activatedFeatures: { [agentIdentifier]: { spa: 1 } }
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
beforeAll(async () => {
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/route-change.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
const agentIdentifier = 'abcdefg'
Expand Down
1 change: 1 addition & 0 deletions tests/components/spa/timers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../src/common/config/runtime', () => ({
__esModule: true,
getRuntime: jest.fn().mockReturnValue({})
}))
jest.mock('../../../src/common/harvest/harvester')

let spaInstrument, spaAggregate, newrelic
const agentIdentifier = 'abcdefg'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ jest.mock('../../../../../src/common/config/runtime', () => ({
getRuntime: jest.fn(() => mockRuntime),
setRuntime: jest.fn()
}))
jest.mock('../../../../../src/common/harvest/harvester')

const pvtAgg = new Aggregate({
agentIdentifier: 'abcd',
Expand Down

0 comments on commit 84b6b0a

Please sign in to comment.