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 21 commits
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
5 changes: 5 additions & 0 deletions .ci/scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ APM_SERVER_PORT=${APM_SERVER_PORT:-"8200"}
APM_SERVER_URL=${APM_SERVER_URL:-"http://apm-server:8200"}
KIBANA_URL=${KIBANA_URL:-"http://kibana:5601"}

# As long as there are issues with the ssl lets use 6.1.3
# see https://github.com/docker/docker-py/issues/3194
pip3 uninstall docker
pip3 install docker==6.1.3

pip install docker-compose>=1.25.4

# Tests are run within the node-puppeteer container and can fails.
Expand Down
3 changes: 2 additions & 1 deletion dev-utils/test-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ function getBrowserList(pkg = 'default') {
},
{
browserName: 'firefox',
browserVersion: 'latest',
// beware that if we update to 99 or more we will need to cope with https://github.com/karma-runner/karma-sauce-launcher/issues/275
browserVersion: '98',
vigneshshanmugam marked this conversation as resolved.
Show resolved Hide resolved
platformName: 'Windows 10',
'sauce:options': {
geckodriverVersion: '0.30.0' // reason: https://github.com/karma-runner/karma-sauce-launcher/issues/275
Expand Down
6 changes: 6 additions & 0 deletions packages/rum-core/src/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const USER_INTERACTION = 'user-interaction'
const HTTP_REQUEST_TYPE = 'http-request'
const TEMPORARY_TYPE = 'temporary'
const NAME_UNKNOWN = 'Unknown'
const PAGE_EXIT = 'page-exit'

const TRANSACTION_TYPE_ORDER = [
PAGE_LOAD,
Expand Down Expand Up @@ -109,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 @@ -144,6 +146,7 @@ const FIRST_CONTENTFUL_PAINT = 'first-contentful-paint'
const LARGEST_CONTENTFUL_PAINT = 'largest-contentful-paint'
const FIRST_INPUT = 'first-input'
const LAYOUT_SHIFT = 'layout-shift'
const EVENT = 'event'

/**
* Event types sent to APM Server on the queue
Expand Down Expand Up @@ -193,13 +196,15 @@ export {
PAGE_LOAD,
ROUTE_CHANGE,
NAME_UNKNOWN,
PAGE_EXIT,
TYPE_CUSTOM,
USER_TIMING_THRESHOLD,
TRANSACTION_START,
TRANSACTION_END,
CONFIG_CHANGE,
QUEUE_FLUSH,
QUEUE_ADD_TRANSACTION,
TRANSACTION_DISCARD,
XMLHTTPREQUEST,
FETCH,
HISTORY,
Expand Down Expand Up @@ -232,6 +237,7 @@ export {
TRUNCATED_TYPE,
FIRST_INPUT,
LAYOUT_SHIFT,
EVENT,
OUTCOME_SUCCESS,
OUTCOME_FAILURE,
OUTCOME_UNKNOWN,
Expand Down
10 changes: 8 additions & 2 deletions packages/rum-core/src/common/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/

import { Url } from './url'
import { PAGE_LOAD, NAVIGATION } from './constants'
import { PAGE_LOAD, PAGE_EXIT, NAVIGATION } from './constants'
import { getServerTimingInfo, PERF, isPerfTimelineSupported } from './utils'

const LEFT_SQUARE_BRACKET = 91 // [
Expand Down Expand Up @@ -186,7 +186,13 @@ export function addTransactionContext(
) {
const pageContext = getPageContext()
let responseContext = {}
if (transaction.type === PAGE_LOAD && isPerfTimelineSupported()) {

if (transaction.type === PAGE_EXIT) {
transaction.ensureContext()
if (transaction.context.page && transaction.context.page.url) {
pageContext.page.url = transaction.context.page.url
}
} else if (transaction.type === PAGE_LOAD && isPerfTimelineSupported()) {
let entries = PERF.getEntriesByType(NAVIGATION)
if (entries && entries.length > 0) {
responseContext = {
Expand Down
63 changes: 53 additions & 10 deletions packages/rum-core/src/common/observers/page-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@
*
*/

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'

/**
* @param configService
Expand Down Expand Up @@ -70,22 +75,60 @@ export function observePageVisibility(configService, transactionService) {
}

/**
* Ends an ongoing transaction if any and flushes the events queue
* Executes when page becomes hidden
* @param configService
* @param transactionService
*/
function onPageHidden(configService, transactionService) {
const tr = transactionService.getCurrentTransaction()
if (tr) {
const inpTr = reportInp(transactionService)
// we don't want to flush the queue for every transaction that is ended when page becomes hidden
// and given the async nature of the ending process of transactions
Copy link
Member

Choose a reason for hiding this comment

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

nit: might reword it as transaction ends are scheduled async as microtasks(promise), so we are coordinating the flushing process

// will need to coordinate the flushing process
if (inpTr) {
const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => {
configService.dispatchEvent(QUEUE_FLUSH)
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
// At this point the INP transaction is in the queue
// so let's end the managed transaction if any and flush
endManagedTransaction(configService, transactionService)
unobserve()
})
} else {
// In the absence of INP transaction
// let's just end the managed transaction if any and flush
endManagedTransaction(configService, transactionService)
}
}

// Ends an ongoing managed transaction if any and flushes the events queue
function endManagedTransaction(configService, transactionService) {
const tr = transactionService.getCurrentTransaction()
if (tr) {
// 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 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
5 changes: 5 additions & 0 deletions packages/rum-core/src/common/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,10 @@ function isPerfTypeSupported(type) {
)
}

function isPerfInteractionCountSupported() {
return 'interactionCount' in performance
}

/**
* The goal of this is to make sure that HAR files
* can be created containing beacons with the payload readable
Expand Down Expand Up @@ -475,6 +479,7 @@ export {
isPerfTimelineSupported,
isBrowser,
isPerfTypeSupported,
isPerfInteractionCountSupported,
isBeaconInspectionEnabled,
isRedirectInfoAvailable
}
6 changes: 5 additions & 1 deletion packages/rum-core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
// export public core APIs.

import { registerServices as registerErrorServices } from './error-logging'
import { registerServices as registerPerfServices } from './performance-monitoring'
import {
registerServices as registerPerfServices,
observeUserInteractions
} from './performance-monitoring'
import { ServiceFactory } from './common/service-factory'
import {
isPlatformSupported,
Expand Down Expand Up @@ -84,6 +87,7 @@ export {
ERROR_LOGGING,
EVENT_TARGET,
CLICK,
observeUserInteractions,
bootstrap,
observePageVisibility,
observePageClicks
Expand Down
4 changes: 3 additions & 1 deletion packages/rum-core/src/performance-monitoring/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
} from '../common/constants'

import { serviceCreators } from '../common/service-factory'
import { observeUserInteractions } from './metrics/inp/process'
import { reportInp } from './metrics/inp/report'

function registerServices() {
serviceCreators[TRANSACTION_SERVICE] = serviceFactory => {
Expand Down Expand Up @@ -65,4 +67,4 @@ function registerServices() {
}
}

export { registerServices }
export { registerServices, observeUserInteractions, reportInp }
Loading