Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Migrate RUM network call from GET to POST #521

Merged
merged 25 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
edc367e
feat: migrating rum to post network call
patrickhousley May 8, 2023
e2243e0
Merge branch 'main' into rum-post
patrickhousley May 9, 2023
030c33f
address pr comment
patrickhousley May 9, 2023
8715886
addressing pr comments
patrickhousley May 9, 2023
98f03c0
fixing rum tests
patrickhousley May 9, 2023
e922245
fix harvesting non-empty non-string values
patrickhousley May 11, 2023
35ac4e9
revert unnecessary change
patrickhousley May 11, 2023
320bcb6
Merge branch 'main' into rum-post
patrickhousley May 11, 2023
789c2bd
fixing integration tests
patrickhousley May 12, 2023
7d45e2b
fixing integration tests
patrickhousley May 15, 2023
e20f02b
Merge branch 'main' into rum-post
patrickhousley May 17, 2023
23c8433
Merge branch 'main' into rum-post
patrickhousley May 18, 2023
4125b32
revert changes back to main
patrickhousley May 18, 2023
c6f0f94
fix test
patrickhousley May 19, 2023
569419d
remove double-serialize of custom attributes
patrickhousley May 23, 2023
edee417
Merge branch 'main' into rum-post
patrickhousley May 23, 2023
179a8be
Merge branch 'main' into rum-post
patrickhousley May 25, 2023
205e6dd
Merge branch 'main' into rum-post
patrickhousley May 30, 2023
c3922d1
Merge branch 'main' into rum-post
patrickhousley May 31, 2023
b7a9d02
Merge branch 'main' into rum-post
patrickhousley Jun 1, 2023
31afde8
Update src/common/harvest/harvest.js
patrickhousley Jun 1, 2023
b283630
Merge branch 'main' into rum-post
patrickhousley Jun 1, 2023
bde03b8
addressing pr comment
patrickhousley Jun 1, 2023
b2d4bde
fix api tests
patrickhousley Jun 1, 2023
1192fed
fixing ie tests
patrickhousley Jun 1, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
469 changes: 469 additions & 0 deletions .github/actions/find-pull-request/package-lock.json

Large diffs are not rendered by default.

14 changes: 5 additions & 9 deletions src/common/harvest/harvest.js
patrickhousley marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class Harvest extends SharedContext {

var agentRuntime = getRuntime(this.sharedContext.agentIdentifier)

if (!payload.body) { // no payload body? nothing to send, just run onfinish stuff and return
if (!payload.body && !opts?.sendEmptyBody) { // no payload body? nothing to send, just run onfinish stuff and return
if (cbFinished) {
cbFinished({ sent: false })
}
Expand All @@ -108,7 +108,7 @@ export class Harvest extends SharedContext {
let url = ''
if (customUrl) url = customUrl
else if (raw) url = `${this.getScheme()}://${info.errorBeacon}/${endpoint}`
else url = `${this.getScheme()}://${info.errorBeacon}/${endpoint}/1/${info.licenseKey}`
else url = `${this.getScheme()}://${info.errorBeacon}${endpoint !== 'rum' ? `/${endpoint}` : ''}/1/${info.licenseKey}`

var baseParams = !raw && includeBaseParams ? this.baseQueryString() : ''
var payloadParams = payload.qs ? encodeObj(payload.qs, agentRuntime.maxBytes) : ''
Expand Down Expand Up @@ -208,6 +208,7 @@ export class Harvest extends SharedContext {
if (singlePayload.body) mapOwn(singlePayload.body, makeBody)
if (singlePayload.qs) mapOwn(singlePayload.qs, makeQueryString)
}

return { body: makeBody(), qs: makeQueryString() }
}

Expand All @@ -223,17 +224,12 @@ export class Harvest extends SharedContext {
}
}

function or (a, b) { return a || b }

export function getSubmitMethod (endpoint, opts) {
opts = opts || {}
var method
var useBody

if (opts.needResponse) { // currently: only STN needs a response
useBody = true
method = submitData.xhr
} else if (opts.unload && isBrowserScope) { // all the features' final harvest; neither methods work outside window context
if (opts.unload && isBrowserScope) { // all the features' final harvest; neither methods work outside window context
useBody = haveSendBeacon
method = haveSendBeacon ? submitData.beacon : submitData.img // really only IE doesn't have Beacon API for web browsers
} else {
Expand Down Expand Up @@ -263,7 +259,7 @@ function createAccumulator () {
var accumulator = {}
var hasData = false
return function (key, val) {
if (val !== null && val !== undefined && val.length) {
if (val !== null && val !== undefined && val.toString()?.length) {
metal-messiah marked this conversation as resolved.
Show resolved Hide resolved
accumulator[key] = val
hasData = true
}
Expand Down
169 changes: 169 additions & 0 deletions src/common/harvest/harvest.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { submitData } from '../util/submit-data'
import { Harvest } from './harvest'

jest.mock('../context/shared-context', () => ({
__esModule: true,
SharedContext: function () {
this.sharedContext = {
agentIdentifier: 'abcd'
}
}
}))
jest.mock('../config/config', () => ({
__esModule: true,
getConfigurationValue: jest.fn(),
getInfo: jest.fn().mockReturnValue({
errorBeacon: 'example.com',
licenseKey: 'abcd'
}),
getRuntime: jest.fn().mockReturnValue({
bytesSent: {},
queryBytesSent: {}
})
}))
jest.mock('../util/submit-data', () => ({
__esModule: true,
submitData: {
xhr: jest.fn(() => ({
addEventListener: jest.fn()
})),
beacon: jest.fn(),
img: jest.fn()
}
}))

describe('sendX', () => {
test.each([null, undefined, false])('should not send request when body is empty and sendEmptyBody is %s', (sendEmptyBody) => {
const sendCallback = jest.fn()
const harvester = new Harvest()
harvester.on('jserrors', () => ({
body: {},
qs: {}
}))

harvester.sendX({ endpoint: 'jserrors', cbFinished: sendCallback })

expect(sendCallback).toHaveBeenCalledWith({ sent: false })
expect(submitData.xhr).not.toHaveBeenCalled()
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test('should send request when body is empty and sendEmptyBody is true', () => {
const harvester = new Harvest()
harvester.on('jserrors', () => ({
body: {},
qs: {}
}))

harvester.sendX({ endpoint: 'jserrors', opts: { sendEmptyBody: true }, cbFinished: jest.fn() })

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining('https://example.com/jserrors/1/abcd?'),
body: undefined
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test.each([null, undefined, []])('should remove %s values from the body and query string when sending', (emptyValue) => {
const harvester = new Harvest()
harvester.on('jserrors', () => ({
body: { bar: 'foo', empty: emptyValue },
qs: { foo: 'bar', empty: emptyValue }
}))

harvester.sendX({ endpoint: 'jserrors', cbFinished: jest.fn() })

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining('&foo=bar'),
body: JSON.stringify({ bar: 'foo' })
}))
expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.not.stringContaining('&empty'),
body: expect.not.stringContaining('empty')
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test.each([1, false, true])('should not remove value %s (when it doesn\'t have a length) from the body and query string when sending', (nonStringValue) => {
const harvester = new Harvest()
harvester.on('jserrors', () => ({
body: { bar: 'foo', nonString: nonStringValue },
qs: { foo: 'bar', nonString: nonStringValue }
}))

harvester.sendX({ endpoint: 'jserrors', cbFinished: jest.fn() })

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining(`&nonString=${nonStringValue}`),
body: JSON.stringify({ bar: 'foo', nonString: nonStringValue })
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})
})

describe('send', () => {
test.each([null, undefined, false])('should not send request when body is empty and sendEmptyBody is %s', (sendEmptyBody) => {
const sendCallback = jest.fn()
const harvester = new Harvest()

harvester.send({ endpoint: 'rum', payload: { qs: {}, body: {} }, opts: { sendEmptyBody }, cbFinished: sendCallback })

expect(sendCallback).toHaveBeenCalledWith({ sent: false })
expect(submitData.xhr).not.toHaveBeenCalled()
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test('should send request when body is empty and sendEmptyBody is true', () => {
const harvester = new Harvest()

harvester.send({ endpoint: 'rum', payload: { qs: {}, body: {} }, opts: { sendEmptyBody: true } })

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining('https://example.com/1/abcd?'),
body: undefined
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test.each([null, undefined, []])('should remove %s values from the body and query string when sending', (emptyValue) => {
const harvester = new Harvest()

harvester.send({
endpoint: 'rum',
payload: { qs: { foo: 'bar', empty: emptyValue }, body: { bar: 'foo', empty: emptyValue } }
})

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining('&foo=bar'),
body: JSON.stringify({ bar: 'foo' })
}))
expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.not.stringContaining('&empty'),
body: expect.not.stringContaining('empty')
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})

test.each([1, false, true])('should not remove value %s (when it doesn\'t have a length) from the body and query string when sending', (nonStringValue) => {
const harvester = new Harvest()

harvester.send({
endpoint: 'rum',
payload: { qs: { foo: 'bar', nonString: nonStringValue }, body: { bar: 'foo', nonString: nonStringValue } }
})

expect(submitData.xhr).toHaveBeenCalledWith(expect.objectContaining({
url: expect.stringContaining(`&nonString=${nonStringValue}`),
body: JSON.stringify({ bar: 'foo', nonString: nonStringValue })
}))
expect(submitData.img).not.toHaveBeenCalled()
expect(submitData.beacon).not.toHaveBeenCalled()
})
})
4 changes: 2 additions & 2 deletions src/common/url/encode.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ export function obj (payload, maxBytes) {
var next
var i

if (typeof dataArray === 'string') {
if (typeof dataArray === 'string' || (!Array.isArray(dataArray) && dataArray !== null && dataArray !== undefined && dataArray.toString().length)) {
metal-messiah marked this conversation as resolved.
Show resolved Hide resolved
next = '&' + feature + '=' + qs(dataArray)
total += next.length
result += next
} else if (dataArray.length) {
} else if (Array.isArray(dataArray) && dataArray.length) {
total += 9
for (i = 0; i < dataArray.length; i++) {
next = qs(stringify(dataArray[i]))
Expand Down
7 changes: 7 additions & 0 deletions src/features/metrics/aggregate/endpoint-map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const endpointMap = {
rum: '1',
events: 'Events',
ins: 'Ins',
jserrors: 'Jserrors',
resources: 'Resources'
}
5 changes: 3 additions & 2 deletions src/features/metrics/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { windowAddEventListener } from '../../../common/event-listener/event-lis
import { isBrowserScope } from '../../../common/util/global-scope'
import { AggregateBase } from '../../utils/aggregate-base'
import { stringify } from '../../../common/util/stringify'
import { endpointMap } from './endpoint-map'

export class Aggregate extends AggregateBase {
static featureName = FEATURE_NAME
Expand Down Expand Up @@ -125,15 +126,15 @@ export class Aggregate extends AggregateBase {
// Capture per-agent bytes sent for each endpoint (see harvest) and RUM call (see page_view_event aggregator).
Object.keys(agentRuntime.bytesSent).forEach(endpoint => {
this.storeSupportabilityMetrics(
`PageSession/Endpoint/${endpoint.charAt(0).toUpperCase() + endpoint.slice(1)}/BytesSent`,
`PageSession/Endpoint/${endpointMap[endpoint]}/BytesSent`,
agentRuntime.bytesSent[endpoint]
)
})

// Capture per-agent query bytes sent for each endpoint (see harvest) and RUM call (see page_view_event aggregator).
Object.keys(agentRuntime.bytesSent).forEach(endpoint => {
this.storeSupportabilityMetrics(
`PageSession/Endpoint/${endpoint.charAt(0).toUpperCase() + endpoint.slice(1)}/QueryBytesSent`,
`PageSession/Endpoint/${endpointMap[endpoint]}/QueryBytesSent`,
agentRuntime.queryBytesSent[endpoint]
)
})
Expand Down
Loading