-
Notifications
You must be signed in to change notification settings - Fork 140
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
⚗️ [RUM-1020] Collect core web vitals target selectors #2418
Conversation
8d5be59
to
b842e5e
Compare
Codecov Report
@@ Coverage Diff @@
## main #2418 +/- ##
==========================================
+ Coverage 93.96% 94.00% +0.03%
==========================================
Files 222 222
Lines 6465 6535 +70
Branches 1417 1448 +31
==========================================
+ Hits 6075 6143 +68
- Misses 390 392 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
b842e5e
to
eec8702
Compare
...rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackLargestContentfulPaint.spec.ts
Outdated
Show resolved
Hide resolved
...ages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackFirstInputTimings.spec.ts
Outdated
Show resolved
Hide resolved
packages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackFirstInputTimings.ts
Outdated
Show resolved
Hide resolved
packages/rum-core/src/domain/rumEventsCollection/view/viewMetrics/trackCommonViewMetrics.ts
Outdated
Show resolved
Hide resolved
c2653c8
to
7c3e131
Compare
const rumPerformanceEntries = entries.filter((entry): entry is RumPerformanceEntry => | ||
objectHasValue(RumPerformanceEntryType, entry) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought: I wonder if we really need to keep filtering entries like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting 🤔. Do you remember why it was done in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FMU, I did this in 15da132 , where I changed entries.getEntriesByType('xxx')
to entries.filter(entry => entry.entryType === 'xxx')
and then we iterated over this, but I don't see any good reason to do it anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FMU, we still have this case to filter
browser-sdk/packages/rum-core/src/browser/performanceCollection.ts
Lines 154 to 155 in fbe0200
if (supportPerformanceObject()) { | |
const performanceEntries = performance.getEntries() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right that's why :D Ok nevermind then. We can always change things if/when we work on my suggestion here: #2355 (comment)
packages/rum-core/src/domain/view/viewMetrics/trackInteractionToNextPaint.ts
Outdated
Show resolved
Hide resolved
12b3189
to
62947b1
Compare
62947b1
to
bb0e1ee
Compare
42f000e
to
df23557
Compare
Motivation
Currently the web vitals are reported with a single value representing the vital score. These scores provide valuable insights to customers, allowing them to determine the performance quality of their website. However, to effectively improve them, it is crucial to identify the specific component to which a vital refers.
Changes
appendElement()
test helperTesting
I have gone over the contributing documentation.