feat(rum): report INP metric #1462

devcorpio · 2023-12-12T19:19:19Z

Summary

Related to #1293

From now on, the RUM agent will start reporting the INP metric.

INP will not be reported if there is no user activity
INP will be reported as 0 if the slowest detected interaction duration is < 40
INP will pick the slowest interaction if there have been less than 50 interactions
INP will result from calculating the P98 if there have been more than 50 interactions.
The metric will be reported when the user leaves the page, which can happen multiple times.
We will "reset" the tracked interactions every time we report the metric
INP will be attached to the page where the user "hard-navigated to".

For further details, please see the code which is heavily documented given the experimental nature of this metric.

Screenshot where we can see the INP metric on the User Experience page:

devcorpio · 2023-12-13T12:09:04Z

hi @v1v,

it seems our test steps are failing with this error:

Looking for info I found that might be related with Docker 7.0.0, link to the changelog

and a link to a conversation that a few people are having: docker/docker-py#3194

Do you know what can we do to fix this?

v1v · 2023-12-13T12:49:57Z

Do you know what can we do to fix this?

As far as I see python3.10 is the one set in the GitHub runners. I don't know whether we can use a pinned version for the GitHub runner or do some magic with downgrading the docker version.

Let me see if I can figure out how to solve this

v1v · 2023-12-13T12:58:08Z

871d1a0 might help, I followed the steps explained here.

devcorpio · 2023-12-13T13:08:52Z

.ci/scripts/test.sh

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


@v1v amazing, thank you for making this change so fast! for what I'm seeing it works perfectly 🎉

devcorpio · 2023-12-13T19:04:34Z

packages/rum-core/src/performance-monitoring/transaction.js

+    Makes sure that a transaction ended while the page was becoming hidden
+    it's not discarded for that very same reason.
+  */
+  endPageHidden(endTime) {


I created this function because while I was doing the last exploratory testing session I spotted a bug:

The logic where we detect if the website becomes hidden in order to end the ongoing transaction, had a race condition:

transaction.end()

update lastHiddenStart

unfortunately, the code where we check if a transaction should be discarded because "the page was hidden during the transaction" doesn't happen in a sync way in transaction.end(), it happens after executing a javascript Promise.

So, assuming that calling transaction.end() in that part of the code was a synchronous process was wrong. It was updating the lastHiddenStart before doing the end transaction check (even if the end was invoked before)

Fortunately, this was not affecting the page-load transaction, I only reproduced the described behaviour with the INP transaction and with click transactions. (not all the time, it depended on who was "winning" the race)

It was updating the lastHiddenStart before doing the end transaction check (even if the end was invoked before)

This seems wrong, we update the lastHiddenStart on Queue:Add event which should happen post the transaction was ended in async way.

We discussed this offline and planned to add another event transaction:skipped to update the last hidden start time.

this is the related commit df7423d 🚀

dev-utils/test-config.js

vigneshshanmugam

Great job @devcorpio 🎉, Thanks a ton for adding in lots of details around the core INP calculation logic and also adding relevant links to the spec where necessary. It was really nice to see them side by side and also understanding the spec/browser reporting.

packages/rum-core/src/performance-monitoring/metrics/inp/index.js

packages/rum-core/src/performance-monitoring/metrics/inp/process.js

packages/rum-core/src/performance-monitoring/metrics/inp/report.js

packages/rum-core/src/performance-monitoring/metrics/inp/process.js

packages/rum-core/src/common/context.js

packages/rum-core/src/performance-monitoring/metrics/inp/process.js

packages/rum-core/test/performance-monitoring/metrics/inp/report.spec.js

vigneshshanmugam

LGTM 🎉 , Added small suggestions. Nothing blocking.

vigneshshanmugam · 2023-12-27T05:48:34Z

packages/rum-core/src/common/observers/page-visibility.js

-  reportInp(transactionService)
+  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


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

vigneshshanmugam · 2023-12-27T05:52:50Z

packages/rum-core/src/common/constants.js

@@ -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'


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

yes, it makes sense 🚀

devcorpio force-pushed the report-inp-metric branch 3 times, most recently from 8995717 to 88874ad Compare December 13, 2023 11:33

devcorpio changed the title ~~DRAFT chore: wip commit INP~~ feat(rum): report INP metric Dec 13, 2023

devcorpio marked this pull request as ready for review December 13, 2023 12:28

devcorpio requested a review from vigneshshanmugam December 13, 2023 12:29

devcorpio commented Dec 13, 2023

View reviewed changes

feat(rum): report INP metric

5c1bc11

devcorpio force-pushed the report-inp-metric branch from bf05a00 to 5c1bc11 Compare December 13, 2023 15:04

devcorpio linked an issue Dec 13, 2023 that may be closed by this pull request

Add support for Interaction Next Paint (INP) #1293

Closed

devcorpio added 3 commits December 13, 2023 16:13

chore: recovering victor commit

ce52cbb

chore: remove needless entry from state

a7317e4

chore: make sure we send transaction ended in page hidden event

ca29650

devcorpio commented Dec 13, 2023

View reviewed changes

devcorpio added 2 commits December 14, 2023 10:22

chore: remove wrong comment from test

dffc15c

chore: patch firefox issue with saucelabs

68e4d26

devcorpio commented Dec 14, 2023

View reviewed changes

dev-utils/test-config.js Show resolved Hide resolved

paulb-elastic mentioned this pull request Dec 14, 2023

[RUM | User Experience] Update to remove FID and add INP elastic/observability-docs#3424

Closed

devcorpio mentioned this pull request Dec 20, 2023

ci: support releases with branch-protection #1458

Merged

vigneshshanmugam reviewed Dec 20, 2023

View reviewed changes

devcorpio added 4 commits December 21, 2023 10:55

chore: remove index file from inp folder

e47fac7

chore: reword about longest interaction sorting

71b0f89

chore: improve readability existing interaction

967d718

chore: reword interactionCount reasoning

e1be333

devcorpio force-pushed the report-inp-metric branch from 665b736 to e1be333 Compare December 21, 2023 10:47

devcorpio added 2 commits December 21, 2023 12:15

chore: merge unit tests in one

d80bd1f

chore: rearrange page context

c0ab253

devcorpio added 7 commits December 21, 2023 13:10

chore: stop lingering test functions in library code

8a99238

chore: remove needless optimization

39f4f2b

chore: add optimization again but improving the comment

f0b5979

chore: add missing tests

2877ba5

chore: create top level variable

087d811

chore: coordinate pagehidden multitransaction

f7c3ca2

chore: emit transaction discard event

df7423d

devcorpio force-pushed the report-inp-metric branch 2 times, most recently from 582edc1 to 55aca81 Compare December 22, 2023 15:46

chore: fix test

38f3a14

devcorpio force-pushed the report-inp-metric branch from 55aca81 to 38f3a14 Compare December 22, 2023 16:15

chore: fix android browser saucelabs issue

92f455b

devcorpio force-pushed the report-inp-metric branch from 9ca240d to 92f455b Compare December 22, 2023 17:49

devcorpio requested a review from vigneshshanmugam December 22, 2023 19:41

vigneshshanmugam previously approved these changes Dec 27, 2023

View reviewed changes

chore: apply pr suggestions

19bc012

devcorpio dismissed vigneshshanmugam’s stale review via 19bc012 December 27, 2023 09:43

shahzad31 approved these changes Dec 27, 2023

View reviewed changes

devcorpio merged commit 866f066 into main Dec 27, 2023
19 of 23 checks passed

devcorpio deleted the report-inp-metric branch December 27, 2023 10:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rum): report INP metric #1462

feat(rum): report INP metric #1462

devcorpio commented Dec 12, 2023 •

edited

devcorpio commented Dec 13, 2023

v1v commented Dec 13, 2023

v1v commented Dec 13, 2023

devcorpio Dec 13, 2023 •

edited

devcorpio Dec 13, 2023 •

edited

vigneshshanmugam Dec 20, 2023

vigneshshanmugam Dec 21, 2023

devcorpio Dec 22, 2023 •

edited

vigneshshanmugam left a comment

vigneshshanmugam left a comment •

edited

vigneshshanmugam Dec 27, 2023

vigneshshanmugam Dec 27, 2023

devcorpio Dec 27, 2023

feat(rum): report INP metric #1462

feat(rum): report INP metric #1462

Conversation

devcorpio commented Dec 12, 2023 • edited

Summary

devcorpio commented Dec 13, 2023

v1v commented Dec 13, 2023

v1v commented Dec 13, 2023

devcorpio Dec 13, 2023 • edited

Choose a reason for hiding this comment

devcorpio Dec 13, 2023 • edited

Choose a reason for hiding this comment

vigneshshanmugam Dec 20, 2023

Choose a reason for hiding this comment

vigneshshanmugam Dec 21, 2023

Choose a reason for hiding this comment

devcorpio Dec 22, 2023 • edited

Choose a reason for hiding this comment

vigneshshanmugam left a comment

Choose a reason for hiding this comment

vigneshshanmugam left a comment • edited

Choose a reason for hiding this comment

vigneshshanmugam Dec 27, 2023

Choose a reason for hiding this comment

vigneshshanmugam Dec 27, 2023

Choose a reason for hiding this comment

devcorpio Dec 27, 2023

Choose a reason for hiding this comment

devcorpio commented Dec 12, 2023 •

edited

devcorpio Dec 13, 2023 •

edited

devcorpio Dec 13, 2023 •

edited

devcorpio Dec 22, 2023 •

edited

vigneshshanmugam left a comment •

edited