ref(replay): Update CLS and add TTFB for replays #12993
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Hey @c298lee I realize this is still in draft but once it's ready, would you mind adding a small PR description what motivates this change? Asking because I'm working on some CLS changes in the browserTracingIntegration and was just curious if there's something we should align on.
| : undefined | ||
| : undefined; | ||
| return getWebVital(metric, 'cumulative-layout-shift', node); | ||
| const lastEntry = metric.entries[metric.entries.length - 1] as (PerformanceEntry & { sources?: LayoutShiftAttribution[] }) | undefined; |
There was a problem hiding this comment.
Ahh, looks like this aligns with the browserTracing logic where we also take the last entry. Sounds good to me!
There was a problem hiding this comment.
Is there a chance we can misalign (say we change the behavior in perf later?)
If so, is there a world we create some convinience function on a shared/core packaged used by both? or is that just not worth it?
There was a problem hiding this comment.
I'm afraid we are going to missalign a bit at least: I opened #13056 with an experimental change and realized that it breaks how we listen to LCP for replay breadcrumbs.
However, I think we just need to be aware that we're tracking CLS a bit differently for replay than for performance insights. In Replay we can collect CLS whenever it emits (though I guess we need to debounce that a bit to acomodate for layout shift bursts) but we can't do the same easily for insights.
There was a problem hiding this comment.
I'll for now probably decouple the listeners for browserTracing and replay: #13056 (comment)
Relates to:
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).