Skip to content

Commit

Permalink
feat: Stop reporting ajax events going to same beacon (#609)
Browse files Browse the repository at this point in the history
Co-authored-by: Patrick Housley <phousley@newrelic.com>
  • Loading branch information
cwli24 and patrickhousley authored Jul 27, 2023
1 parent d616f64 commit ca43edf
Show file tree
Hide file tree
Showing 15 changed files with 3,681 additions and 3,492 deletions.
7,014 changes: 3,577 additions & 3,437 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/common/config/state/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const model = () => {
return {
allow_bfcache: true, // *cli - temporary feature flag for BFCache work
privacy: { cookies_enabled: true }, // *cli - per discussion, default should be true
ajax: { deny_list: undefined, enabled: true, harvestTimeSeconds: 10 },
ajax: { deny_list: undefined, block_internal: true, enabled: true, harvestTimeSeconds: 10 },
distributed_tracing: {
enabled: undefined,
exclude_newrelic_header: undefined,
Expand Down
3 changes: 2 additions & 1 deletion src/common/config/state/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const model = {
releaseIds: {},
session: undefined,
xhrWrappable: typeof globalScope.XMLHttpRequest?.prototype?.addEventListener === 'function',
version: VERSION
version: VERSION,
denyList: undefined
}

const _cache = {}
Expand Down
22 changes: 11 additions & 11 deletions src/common/deny-list/deny-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,27 @@ export function setDenyList (denyListConfig) {
}

for (var i = 0; i < denyListConfig.length; i++) {
var url = denyListConfig[i]
let url = denyListConfig[i]
if (!url) continue // ignore bad values like undefined or empty strings

if (url.indexOf('http://') === 0) {
url = url.substring(7)
} else if (url.indexOf('https://') === 0) {
url = url.substring(8)
}

var firstSlash = url.indexOf('/')

const firstSlash = url.indexOf('/')
let host, pathname
if (firstSlash > 0) {
denyList.push({
hostname: url.substring(0, firstSlash),
pathname: url.substring(firstSlash)
})
host = url.substring(0, firstSlash)
pathname = url.substring(firstSlash)
} else {
denyList.push({
hostname: url,
pathname: ''
})
host = url
pathname = ''
}
let [hostname, port] = host.split(':')

denyList.push({ hostname, pathname })
}
}
/**
Expand Down
31 changes: 31 additions & 0 deletions src/common/deny-list/deny-list.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
describe('Setting deny list', () => {
let DlMod
beforeEach(() => {
jest.isolateModules(() => import('./deny-list.js').then(m => DlMod = m)) // give every test its own denyList (sandbox)
})

it('respects path', () => {
DlMod.setDenyList(['bam.nr-data.net/somepath'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '/somepath' })).toBeFalsy() // shouldCollectEvent returns 'false' when there IS a match...
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '/someotherpath' })).toBeTruthy()
})

it('ignores port', () => {
DlMod.setDenyList(['bam.nr-data.net:1234'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', port: '4321' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'http', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
})

it('ignores protocol', () => {
DlMod.setDenyList(['http://bam.nr-data.net'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', protocol: 'https' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '', protocol: 'http' })).toBeTruthy()
})
})
56 changes: 25 additions & 31 deletions src/features/ajax/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { registerHandler as register } from '../../../common/event-emitter/regis
import { stringify } from '../../../common/util/stringify'
import { nullable, numeric, getAddStringContext, addCustomAttributes } from '../../../common/serialize/bel-serializer'
import { handle } from '../../../common/event-emitter/handle'
import { getConfigurationValue, getInfo } from '../../../common/config/config'
import { getConfiguration, getInfo, getRuntime } from '../../../common/config/config'
import { HarvestScheduler } from '../../../common/harvest/harvest-scheduler'
import { setDenyList, shouldCollectEvent } from '../../../common/deny-list/deny-list'
import { FEATURE_NAME } from '../constants'
Expand All @@ -19,15 +19,25 @@ export class Aggregate extends AggregateBase {
static featureName = FEATURE_NAME
constructor (agentIdentifier, aggregator) {
super(agentIdentifier, aggregator, FEATURE_NAME)
const agentInit = getConfiguration(agentIdentifier)
const allAjaxIsEnabled = agentInit.ajax.enabled !== false

register('xhr', storeXhr, this.featureName, this.ee)
if (!allAjaxIsEnabled) {
drain(this.agentIdentifier, this.featureName)
return // feature will only collect timeslice metrics & ajax trace nodes if it's not fully enabled
}

const denyList = getRuntime(agentIdentifier).denyList
setDenyList(denyList)

let ajaxEvents = []
let spaAjaxEvents = {}
let sentAjaxEvents = []
let scheduler

const ee = this.ee

const harvestTimeSeconds = getConfigurationValue(agentIdentifier, 'ajax.harvestTimeSeconds') || 10
const MAX_PAYLOAD_SIZE = getConfigurationValue(agentIdentifier, 'ajax.maxPayloadSize') || 1000000
const harvestTimeSeconds = agentInit.ajax.harvestTimeSeconds || 10
const MAX_PAYLOAD_SIZE = agentInit.ajax.maxPayloadSize || 1000000

// Exposes these methods to browser test files -- future TO DO: can be removed once these fns are extracted from the constructor into class func
this.storeXhr = storeXhr
Expand All @@ -39,9 +49,8 @@ export class Aggregate extends AggregateBase {
// remove from the spaAjaxEvents buffer, and let spa harvest it
delete spaAjaxEvents[interaction.id]
})

ee.on('interactionDiscarded', (interaction) => {
if (!spaAjaxEvents[interaction.id] || !allAjaxIsEnabled()) return
if (!spaAjaxEvents[interaction.id]) return

spaAjaxEvents[interaction.id].forEach(function (item) {
// move it from the spaAjaxEvents buffer to the ajaxEvents buffer for harvesting here
Expand All @@ -50,18 +59,15 @@ export class Aggregate extends AggregateBase {
delete spaAjaxEvents[interaction.id]
})

if (allAjaxIsEnabled()) setDenyList(getConfigurationValue(agentIdentifier, 'ajax.deny_list'))

register('xhr', storeXhr, this.featureName, this.ee)
const scheduler = new HarvestScheduler('events', {
onFinished: onEventsHarvestFinished,
getPayload: prepareHarvest
}, this)

if (allAjaxIsEnabled()) {
scheduler = new HarvestScheduler('events', {
onFinished: onEventsHarvestFinished,
getPayload: prepareHarvest
}, this)
ee.on(`drain-${this.featureName}`, () => { scheduler.startTimer(harvestTimeSeconds) })

ee.on(`drain-${this.featureName}`, () => { scheduler.startTimer(harvestTimeSeconds) })
}
drain(this.agentIdentifier, this.featureName)
return

function storeXhr (params, metrics, startTime, endTime, type) {
metrics.time = startTime
Expand All @@ -79,9 +85,7 @@ export class Aggregate extends AggregateBase {
// store as metric
aggregator.store('xhr', hash, params, metrics)

if (!allAjaxIsEnabled()) {
return
}
if (!allAjaxIsEnabled) return

if (!shouldCollectEvent(params)) {
if (params.hostname === getInfo(agentIdentifier).errorBeacon) {
Expand Down Expand Up @@ -172,7 +176,7 @@ export class Aggregate extends AggregateBase {
}

function onEventsHarvestFinished (result) {
if (result.retry && sentAjaxEvents.length > 0 && allAjaxIsEnabled()) {
if (result.retry && sentAjaxEvents.length > 0) {
ajaxEvents.unshift(...sentAjaxEvents)
sentAjaxEvents = []
}
Expand Down Expand Up @@ -234,15 +238,5 @@ export class Aggregate extends AggregateBase {
return this.payload.length * 2 > maxPayloadSize
}
}

function allAjaxIsEnabled () {
var enabled = getConfigurationValue(agentIdentifier, 'ajax.enabled')
if (enabled === false) {
return false
}
return true
}

drain(this.agentIdentifier, this.featureName)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,6 @@ function wait (ms = 0) {

function primeSessionAndReplay (sess = new SessionEntity({ agentIdentifier, key: 'SESSION', storage: new LocalMemory() })) {
session = sess
configure(agentIdentifier, { info, runtime: { session } }, 'test', true)
configure(agentIdentifier, { info, runtime: { session }, init: {} }, 'test', true)
sr = new SessionReplayAgg(agentIdentifier, new Aggregator({}))
}
7 changes: 4 additions & 3 deletions src/loaders/configure/configure.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { setAPI, setTopLevelCallers } from '../api/api'
import { addToNREUM, gosCDN, gosNREUMInitializedAgents } from '../../common/window/nreum'
import { setConfiguration, setInfo, setLoaderConfig, setRuntime } from '../../common/config/config'
import { activateFeatures, activatedFeatures } from '../../common/util/feature-flags'
import { activatedFeatures } from '../../common/util/feature-flags'
import { isWorkerScope } from '../../common/constants/runtime'

export function configure (agentIdentifier, opts = {}, loaderType, forceDrain) {
Expand All @@ -15,15 +15,16 @@ export function configure (agentIdentifier, opts = {}, loaderType, forceDrain) {

setConfiguration(agentIdentifier, init || {})
setLoaderConfig(agentIdentifier, loader_config || {})
setRuntime(agentIdentifier, runtime)

info.jsAttributes ??= {}
if (isWorkerScope) { // add a default attr to all worker payloads
info.jsAttributes.isWorker = true
}

setInfo(agentIdentifier, info)

runtime.denyList = init.ajax?.block_internal ? (init.ajax.deny_list || []).concat(info.beacon, info.errorBeacon) : init.ajax?.deny_list
setRuntime(agentIdentifier, runtime)

setTopLevelCallers()
const api = setAPI(agentIdentifier, forceDrain)
gosNREUMInitializedAgents(agentIdentifier, api, 'api')
Expand Down
1 change: 1 addition & 0 deletions tests/assets/spa/fetch-exceed-max-spa-nodes.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{config}
<script>
window.NREUM.init = {
...window.NREUM.init,
distributed_tracing: {
enabled: true,
allowed_origins: ['http://test-1.fake-nr-beacon.com:3333']
Expand Down
9 changes: 6 additions & 3 deletions tests/browser/xhr/ajax-events.browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test('storeXhr for a SPA ajax request buffers in spaAjaxEvents', function (t) {
{ // params
method: 'PUT',
status: 200,
host: 'https://example.com',
hostname: 'example.com',
pathname: '/pathname'
},
{ // metrics
Expand Down Expand Up @@ -69,7 +69,7 @@ test('storeXhr for a non-SPA ajax request buffers in ajaxEvents', function (t) {
{ // params
method: 'PUT',
status: 200,
host: 'https://example.com',
hostname: 'example.com',
pathname: '/pathname'
},
{ // metrics
Expand Down Expand Up @@ -107,7 +107,7 @@ test('on interactionDiscarded, saved SPA events are buffered in ajaxEvents', fun
{ // params
method: 'PUT',
status: 200,
host: 'https://example.com',
hostname: 'example.com',
pathname: '/pathname'
},
{ // metrics
Expand Down Expand Up @@ -160,6 +160,7 @@ test('prepareHarvest correctly serializes an AjaxRequest events payload', functi
method: expected.method,
status: expected.status,
host: expected.domain,
hostname: 'example.com',
pathname: expected.path
},
{ // metrics
Expand Down Expand Up @@ -255,6 +256,7 @@ test('prepareHarvest correctly serializes a very large AjaxRequest events payloa
method: expected().method,
status: expected().status,
host: expected().domain,
hostname: 'example.com',
pathname: expected().path
},
{ // metrics
Expand Down Expand Up @@ -345,6 +347,7 @@ test('prepareHarvest correctly exits if maxPayload is too small', function (t) {
method: expected().method,
status: expected().status,
host: expected().domain,
hostname: 'example.com',
pathname: expected().path
},
{ // metrics
Expand Down
1 change: 0 additions & 1 deletion tests/specs/session-replay/harvest.e2e.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import { notIE } from '../../../tools/browser-matcher/common-matchers.mjs'
import { config, decodeAttributes, getSR } from './helpers'

Expand Down
18 changes: 18 additions & 0 deletions tests/specs/xhr/deny-list.e2e.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
describe('Ajax events to beacon endpoint', () => {
it('not captured when blocked', async () => {
let url = await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: true } } })
let nextAjaxReq = browser.testHandle.expectAjaxEvents(10000, true)
await browser.url(url)

await expect(nextAjaxReq).resolves.toBeUndefined()
})

it('captured when allowed', async () => {
let url = await browser.testHandle.assetURL('spa/ajax-deny-list.html', { init: { ajax: { block_internal: false } } })
let nextAjaxReq = browser.testHandle.expectAjaxEvents(10000)
await browser.url(url)

let correctXhrEvent = (await nextAjaxReq)?.request.body.some(ajaxNode => ajaxNode.domain.startsWith('bam-test-1.nr-local.net') && ajaxNode.path === '/json')
expect(correctXhrEvent).toEqual(true)
})
})
3 changes: 2 additions & 1 deletion tools/testing-server/test-handle.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,8 @@ module.exports = class TestHandle {
loader: 'full',
config: {
licenseKey: this.#testId
}
},
init: { ajax: { block_internal: false } }
},
query
),
Expand Down
2 changes: 1 addition & 1 deletion tools/wdio/plugins/testing-server/default-asset-query.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const query = {
init: {
allow_bfcache: true,
privacy: { cookies_enabled: false },
ajax: { deny_list: [], enabled: true, harvestTimeSeconds: 5 },
ajax: { deny_list: [], block_internal: false, enabled: true, harvestTimeSeconds: 5 },
distributed_tracing: {},
session: { domain: undefined, expiresMs: 14400000, inactiveMs: 1800000 },
ssl: false,
Expand Down
2 changes: 1 addition & 1 deletion tools/webpack/loaders/istanbul/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createInstrumenter } from 'istanbul-lib-instrument'
import loaderUtils from 'loader-utils'
import validateOptions from 'schema-utils'
import convert from 'convert-source-map'
import schema from './options.json' assert { type: "json" }
import schema from './options.json' assert { type: 'json' }

export default function (source, sourceMap) {
const options = Object.assign({ produceSourceMap: true }, loaderUtils.getOptions(this))
Expand Down

0 comments on commit ca43edf

Please sign in to comment.