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(rum): report INP metric #1462

Merged
merged 22 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5c1bc11
feat(rum): report INP metric
devcorpio Nov 29, 2023
ce52cbb
chore: recovering victor commit
devcorpio Dec 13, 2023
a7317e4
chore: remove needless entry from state
devcorpio Dec 13, 2023
ca29650
chore: make sure we send transaction ended in page hidden event
devcorpio Dec 13, 2023
dffc15c
chore: remove wrong comment from test
devcorpio Dec 14, 2023
68e4d26
chore: patch firefox issue with saucelabs
devcorpio Dec 14, 2023
e47fac7
chore: remove index file from inp folder
devcorpio Dec 21, 2023
71b0f89
chore: reword about longest interaction sorting
devcorpio Dec 21, 2023
967d718
chore: improve readability existing interaction
devcorpio Dec 21, 2023
e1be333
chore: reword interactionCount reasoning
devcorpio Dec 21, 2023
d80bd1f
chore: merge unit tests in one
devcorpio Dec 21, 2023
c0ab253
chore: rearrange page context
devcorpio Dec 21, 2023
8a99238
chore: stop lingering test functions in library code
devcorpio Dec 21, 2023
39f4f2b
chore: remove needless optimization
devcorpio Dec 21, 2023
f0b5979
chore: add optimization again but improving the comment
devcorpio Dec 21, 2023
2877ba5
chore: add missing tests
devcorpio Dec 21, 2023
087d811
chore: create top level variable
devcorpio Dec 21, 2023
f7c3ca2
chore: coordinate pagehidden multitransaction
devcorpio Dec 21, 2023
df7423d
chore: emit transaction discard event
devcorpio Dec 22, 2023
38f3a14
chore: fix test
devcorpio Dec 22, 2023
92f455b
chore: fix android browser saucelabs issue
devcorpio Dec 22, 2023
19bc012
chore: apply pr suggestions
devcorpio Dec 27, 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
2 changes: 2 additions & 0 deletions packages/rum-core/src/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ const TRANSACTION_END = 'transaction:end'
const CONFIG_CHANGE = 'config:change'
const QUEUE_FLUSH = 'queue:flush'
const QUEUE_ADD_TRANSACTION = 'queue:add_transaction'
const TRANSACTION_DISCARD = 'transaction:discard'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we rename it as trasaction:ignore/skip ? mainly suggesting because ignored also aligns with our config settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it makes sense 🚀


/**
* Events types that are used to toggle auto instrumentations
Expand Down Expand Up @@ -203,6 +204,7 @@ export {
CONFIG_CHANGE,
QUEUE_FLUSH,
QUEUE_ADD_TRANSACTION,
TRANSACTION_DISCARD,
XMLHTTPREQUEST,
FETCH,
HISTORY,
Expand Down
40 changes: 31 additions & 9 deletions packages/rum-core/src/common/observers/page-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
*
*/

import { QUEUE_ADD_TRANSACTION, QUEUE_FLUSH } from '../constants'
import {
QUEUE_ADD_TRANSACTION,
QUEUE_FLUSH,
TRANSACTION_DISCARD
} from '../constants'
import { state } from '../../state'
import { now } from '../utils'
import { reportInp } from '../../performance-monitoring/metrics/inp/report'
Expand Down Expand Up @@ -98,15 +102,33 @@ function onPageHidden(configService, transactionService) {
function endManagedTransaction(configService, transactionService) {
const tr = transactionService.getCurrentTransaction()
if (tr) {
const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => {
configService.dispatchEvent(QUEUE_FLUSH)
state.lastHiddenStart = now()
// Make sure that we still update lastHiddenStart if the managed transaction
// ends up being discarded
const unobserveDiscard = configService.observeEvent(
TRANSACTION_DISCARD,
() => {
state.lastHiddenStart = now()

// unsubscribe listener to execute it only once.
// otherwise in SPAs we might be finding situations where we would be executing
// this logic as many times as we observed to this event
unobserve()
})
// unsubscribe listeners to execute it only once.
// otherwise in SPAs we might be finding situations where we would be executing
// this logic as many times as we observed to this event
unobserveDiscard()
unobserveQueueAdd()
}
)
const unobserveQueueAdd = configService.observeEvent(
QUEUE_ADD_TRANSACTION,
() => {
configService.dispatchEvent(QUEUE_FLUSH)
state.lastHiddenStart = now()

// unsubscribe listeners to execute it only once.
// otherwise in SPAs we might be finding situations where we would be executing
// this logic as many times as we observed to this event
unobserveQueueAdd()
unobserveDiscard()
}
)

tr.end()
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ import {
OUTCOME_FAILURE,
OUTCOME_SUCCESS,
OUTCOME_UNKNOWN,
QUEUE_ADD_TRANSACTION
QUEUE_ADD_TRANSACTION,
TRANSACTION_DISCARD
} from '../common/constants'
import {
truncateModel,
Expand Down Expand Up @@ -423,5 +424,7 @@ export default class PerformanceMonitoring {
if (filtered) {
return this.createTransactionDataModel(transaction)
}

this._configService.dispatchEvent(TRANSACTION_DISCARD)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
NAME_UNKNOWN,
TRANSACTION_START,
TRANSACTION_END,
TRANSACTION_DISCARD,
TEMPORARY_TYPE,
TRANSACTION_TYPE_ORDER,
LARGEST_CONTENTFUL_PAINT,
Expand Down Expand Up @@ -227,7 +228,7 @@ class TransactionService {
if (__DEV__) {
this._logger.debug(`startTransaction(${tr.id}, ${tr.name}, ${tr.type})`)
}
this._config.events.send(TRANSACTION_START, [tr])
this._config.dispatchEvent(TRANSACTION_START, [tr])
}

return tr
Expand Down Expand Up @@ -257,6 +258,7 @@ class TransactionService {
`transaction(${tr.id}, ${name}, ${type}) was discarded! The page was hidden during the transaction!`
)
}
this._config.dispatchEvent(TRANSACTION_DISCARD)
return
}

Expand All @@ -266,6 +268,7 @@ class TransactionService {
`transaction(${tr.id}, ${name}, ${type}) is ignored`
)
}
this._config.dispatchEvent(TRANSACTION_DISCARD)
return
}

Expand Down Expand Up @@ -337,7 +340,7 @@ class TransactionService {
const configContext = this._config.get('context')
addTransactionContext(tr, configContext)

this._config.events.send(TRANSACTION_END, [tr])
this._config.dispatchEvent(TRANSACTION_END, [tr])
if (__DEV__) {
this._logger.debug(
`end transaction(${tr.id}, ${tr.name}, ${tr.type})`,
Expand Down
174 changes: 86 additions & 88 deletions packages/rum-core/test/common/observers/page-visibility.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import {
QUEUE_FLUSH,
TRANSACTION_SERVICE,
CONFIG_SERVICE,
PERFORMANCE_MONITORING
PERFORMANCE_MONITORING,
TEMPORARY_TYPE
} from '../../../src/common/constants'
import * as utils from '../../../src/common/utils'
import {
Expand All @@ -40,8 +41,17 @@ import {
} from '../..'
import { spyOnFunction, waitFor } from '../../../../../dev-utils/jasmine'
import * as inpReporter from '../../../src/performance-monitoring/metrics/inp/report'
import * as inpProcessor from '../../../src/performance-monitoring/metrics/inp/process'

describe('observePageVisibility', () => {
// Starts performanceMonitoring to observe how transaction ends
function startsPerformanceMonitoring() {
const performanceMonitoring = serviceFactory.getService(
PERFORMANCE_MONITORING
)
performanceMonitoring.init()
}

let serviceFactory
let configService
let transactionService
Expand Down Expand Up @@ -93,17 +103,32 @@ describe('observePageVisibility', () => {
// the observed events by the agent...
;['visibilitychange', 'pagehide'].forEach(eventName => {
describe(`when page becomes hidden due to ${eventName.toUpperCase()} event`, () => {
it('should report INP', () => {
const reportSpy = spyOnFunction(inpReporter, 'reportInp')
it('should report INP', async () => {
// Arrange
const dispatchEventSpy = spyOn(
configService,
'dispatchEvent'
).and.callThrough()
const reportSpy = spyOnFunction(
inpReporter,
'reportInp'
).and.callThrough()
const calculateInpSpy = spyOnFunction(inpProcessor, 'calculateInp')
calculateInpSpy.and.returnValue(100) // makes sure the inp transaction is created
unobservePageVisibility = observePageVisibility(
configService,
transactionService
)
startsPerformanceMonitoring()

// Act
hidePageSynthetically(eventName)
await waitFor(() => dispatchEventSpy.calls.any())

// Assert
expect(reportSpy).toHaveBeenCalledTimes(1)
expect(reportSpy).toHaveBeenCalledWith(transactionService)
expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
})

describe('with every transaction already ended', () => {
Expand Down Expand Up @@ -135,14 +160,6 @@ describe('observePageVisibility', () => {
})

describe('with a transaction that has not ended yet', () => {
// Starts performanceMonitoring to observe how transaction ends
function startsPerformanceMonitoring() {
const performanceMonitoring = serviceFactory.getService(
PERFORMANCE_MONITORING
)
performanceMonitoring.init()
}

function createTransaction() {
const transaction = transactionService.startTransaction(
'test-tr',
Expand Down Expand Up @@ -173,84 +190,65 @@ describe('observePageVisibility', () => {
expect(endTransactionSpy).toHaveBeenCalledTimes(1)
})

it('should dispatch the QUEUE_FLUSH event once the transaction ends and added to the queue', async () => {
// Arrange
const transaction = createTransaction()
const dispatchEventSpy = spyOn(
configService,
'dispatchEvent'
).and.callThrough()
spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
transaction
)
startsPerformanceMonitoring()

// Act
unobservePageVisibility = observePageVisibility(
configService,
transactionService
)
hidePageSynthetically(eventName)
await waitFor(() => dispatchEventSpy.calls.any())

// Assert
expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
})

it('should remove the QUEUE_ADD_TRANSACTION event listener once the transaction ends and added to the queue', async () => {
// Arrange
const unobserveSpy = jasmine.createSpy()
const observeEvent = configService.observeEvent.bind(configService)
spyOn(configService, 'observeEvent').and.callFake((name, fn) => {
observeEvent(name, fn)
return unobserveSpy
describe('event listeners', () => {
;[
{
name: 'should fire when transaction is added to the queue',
createTransaction,
customAssertions: ({ dispatchEventSpy }) => {
// should dispatch the QUEUE_FLUSH event
expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
}
},
{
name: 'should fire when transaction is discarded',
createTransaction: () => {
const tr = createTransaction()
// make sure transaction will be discarded
tr.type = TEMPORARY_TYPE

return tr
},
customAssertions: utils.noop
}
].forEach(({ name, createTransaction, customAssertions }) => {
it(`${name}`, async () => {
// Arrange
const anyTime = 1234567834
spyOnFunction(utils, 'now').and.returnValue(anyTime)
const unobserveSpy = jasmine.createSpy()
const observeEvent = configService.observeEvent.bind(
configService
)
spyOn(configService, 'observeEvent').and.callFake((name, fn) => {
observeEvent(name, fn)
return unobserveSpy
})
const transaction = createTransaction()
const dispatchEventSpy = spyOn(
configService,
'dispatchEvent'
).and.callThrough()
spyOn(
transactionService,
'getCurrentTransaction'
).and.returnValue(transaction)
startsPerformanceMonitoring()

// Act
unobservePageVisibility = observePageVisibility(
configService,
transactionService
)
hidePageSynthetically(eventName)
await waitFor(() => dispatchEventSpy.calls.any())

// Assert
customAssertions({ dispatchEventSpy })
expect(unobserveSpy).toHaveBeenCalledTimes(2)
expect(state.lastHiddenStart).toBe(anyTime)
})
})
const transaction = createTransaction()
const dispatchEventSpy = spyOn(
configService,
'dispatchEvent'
).and.callThrough()
spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
transaction
)
startsPerformanceMonitoring()

// Act
unobservePageVisibility = observePageVisibility(
configService,
transactionService
)
hidePageSynthetically(eventName)
await waitFor(() => dispatchEventSpy.calls.any())

// Assert
expect(unobserveSpy).toHaveBeenCalledTimes(1)
})

it('should set lastHiddenStart once the transaction ends and added to the queue', async () => {
// Arrange
const anyTime = 1234567834
spyOnFunction(utils, 'now').and.returnValue(anyTime)
const transaction = createTransaction()
const dispatchEventSpy = spyOn(
configService,
'dispatchEvent'
).and.callThrough()
spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
transaction
)
startsPerformanceMonitoring()

// Act
unobservePageVisibility = observePageVisibility(
configService,
transactionService
)
hidePageSynthetically(eventName)
await waitFor(() => dispatchEventSpy.calls.any())

// Assert
expect(state.lastHiddenStart).toBe(anyTime)
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ import {
LOGGING_SERVICE,
CONFIG_SERVICE,
APM_SERVER,
PERFORMANCE_MONITORING
PERFORMANCE_MONITORING,
TRANSACTION_DISCARD
} from '../../src/common/constants'
import { state } from '../../src/state'
import patchEventHandler from '../common/patch'
Expand Down Expand Up @@ -195,6 +196,18 @@ describe('PerformanceMonitoring', function () {
expect(payload.spans[0].duration).toBe(parseInt(span._end - span._start))
})

it('should notify when a transaction has been filtered out', function () {
spyOn(configService, 'dispatchEvent')
var tr = new Transaction('transaction-no-duration', 'transaction-type')
tr.end()

var payload = performanceMonitoring.createTransactionPayload(tr)
expect(payload).toBeUndefined()
expect(configService.dispatchEvent).toHaveBeenCalledWith(
TRANSACTION_DISCARD
)
})

it('should sendPageLoadMetrics', function (done) {
const unMock = mockGetEntriesByType()
const transactionService = serviceFactory.getService(TRANSACTION_SERVICE)
Expand Down
Loading
Loading