Skip to content

Commit

Permalink
Fix interaction start/end time being wrong on spa payloads of 3+ inte…
Browse files Browse the repository at this point in the history
…ractions
  • Loading branch information
cwli24 committed Nov 21, 2024
1 parent bd4b481 commit 614e976
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/features/soft_navigations/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ export class Aggregate extends AggregateBase {
serializer (eventBuffer) {
// The payload depacker takes the first ixn of a payload (if there are multiple ixns) and positively offset the subsequent ixns timestamps by that amount.
// In order to accurately portray the real start & end times of the 2nd & onward ixns, we hence need to negatively offset their start timestamps with that of the 1st ixn.
let firstIxnStartTime = 0 // the very 1st ixn does not require any offsetting
let firstIxnStartTime
const serializedIxnList = []
for (const interaction of eventBuffer) {
serializedIxnList.push(interaction.serialize(firstIxnStartTime))
if (!firstIxnStartTime) firstIxnStartTime = Math.floor(interaction.start)
if (firstIxnStartTime === undefined) firstIxnStartTime = Math.floor(interaction.start) // careful not to match or overwrite on 0 value!
}
return `bel.7;${serializedIxnList.join(';')}`
}
Expand Down
9 changes: 5 additions & 4 deletions src/features/soft_navigations/aggregate/interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export class Interaction extends BelNode {
get navTiming () {}

serialize (firstStartTimeOfPayload) {
const isFirstIxnOfPayload = firstStartTimeOfPayload === undefined
const addString = getAddStringContext(this.agentIdentifier)
const nodeList = []
let ixnType
Expand All @@ -126,7 +127,7 @@ export class Interaction extends BelNode {
const fields = [
numeric(this.belType),
0, // this will be overwritten below with number of attached nodes
numeric(this.start - firstStartTimeOfPayload), // relative to first node
numeric(this.start - (isFirstIxnOfPayload ? 0 : firstStartTimeOfPayload)), // the very 1st ixn does not require offset so it should fallback to a 0 while rest is offset by the very 1st ixn's start
numeric(this.end - this.start), // end -- relative to start
numeric(this.callbackEnd), // cbEnd -- relative to start; not used by BrowserInteraction events
numeric(this.callbackDuration), // not relative
Expand All @@ -145,9 +146,9 @@ export class Interaction extends BelNode {
const allAttachedNodes = addCustomAttributes(this.customAttributes || {}, addString) // start with all custom attributes
if (getInfo(this.agentIdentifier).atts) allAttachedNodes.push('a,' + addString(getInfo(this.agentIdentifier).atts)) // add apm provided attributes
/* Querypack encoder+decoder quirkiness:
- If first ixn node of payload is being processed, we use this node's start to offset. (firstStartTime should be 0--or undefined.)
- Else for subsequent ixn nodes, we use the first ixn node's start to offset. */
this.children.forEach(node => allAttachedNodes.push(node.serialize(firstStartTimeOfPayload || this.start))) // recursively add the serialized string of every child of this (ixn) bel node
- If first ixn node of payload is being processed, its children's start time must be offset by this node's start. (firstStartTime should be undefined.)
- Else for subsequent ixns in the same payload, we go back to using that first ixn node's start to offset their children's start. */
this.children.forEach(node => allAttachedNodes.push(node.serialize(isFirstIxnOfPayload ? this.start : firstStartTimeOfPayload))) // recursively add the serialized string of every child of this (ixn) bel node

fields[1] = numeric(allAttachedNodes.length)
nodeList.push(fields)
Expand Down
4 changes: 2 additions & 2 deletions tests/components/soft_navigations/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ test('.actionText and .setAttribute add attributes to ixn specifically', () => {

// This isn't just an API test; it double serves as data validation on the querypack payload output.
test('multiple finished ixns retain the correct start/end timestamps in payload', () => {
softNavAggregate.ee.emit(`${INTERACTION_API}-get`, [100])
softNavAggregate.ee.emit(`${INTERACTION_API}-get`, [0])
let ixnContext = getIxnContext(newrelic.interaction())
ixnContext.associatedInteraction.nodeId = 1
ixnContext.associatedInteraction.id = 'some_id'
Expand All @@ -290,7 +290,7 @@ test('multiple finished ixns retain the correct start/end timestamps in payload'

expect(softNavAggregate.interactionsToHarvest.get().length).toEqual(3)
// WARN: Double check decoded output & behavior or any introduced bugs before changing the follow line's static string.
expect(softNavAggregate.makeHarvestPayload().body).toEqual("bel.7;1,,2s,2s,,,'api,'http://localhost/,1,1,,2,!!!!'some_id,'1,!!;;1,,5k,5k,,,'api,'http://localhost/,1,1,,2,!!!!'some_other_id,'2,!!;;1,,go,8c,,,'api,'http://localhost/,1,1,,2,!!!!'some_another_id,'3,!!;")
expect(softNavAggregate.makeHarvestPayload().body).toEqual("bel.7;1,,,5k,,,'api,'http://localhost/,1,1,,2,!!!!'some_id,'1,!!;;1,,8c,5k,,,'api,'http://localhost/,1,1,,2,!!!!'some_other_id,'2,!!;;1,,jg,8c,,,'api,'http://localhost/,1,1,,2,!!!!'some_another_id,'3,!!;")
})

// This isn't just an API test; it double serves as data validation on the querypack payload output.
Expand Down

0 comments on commit 614e976

Please sign in to comment.