feat(browser): Send CLS as standalone span (experimental) #13056
Conversation
size-limit report 📦
|
| @@ -44,6 +44,85 @@ export function startAndEndSpan( | |||
| }); | |||
| } | |||
|
|
|||
| interface StandaloneWebVitalSpanOptions { | |||
There was a problem hiding this comment.
I extracted this from the INP code and only left vital-specific logic in the CLS and INP handlers. Most attributes and span properties are the same for INP and CLS (+soon LCP), so it makes sense to dedupe and save a couple of bytes.
|
Update: I changed my approach here to also work with |
| let standaloneCLsValue = 0; | ||
| let standaloneClsEntry: LayoutShift | undefined; | ||
|
|
||
| if (!supportsLayoutShift()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Starting with 0 solves the 0 CLS value problem (see #12989). However, since this happens outside the web-vitals library, we need to add a browser support check for CLS to avoid sending 0 values on browsers that don't support collecting CLS.
Lms24
requested review from
jjbayer,
mydea,
cleptric,
AbhiPrasad,
edwardgou-sentry and
c298lee
|
This does not block reviews but to follow up: After syncing with @c298lee we can also switch back to But we need to keep |
There was a problem hiding this comment.
Functionally I think this makes sense to me. Tested this out locally and the payload + behaviour looks correct.
I wasn't able to get webvital scoring to run on these standalone CLS spans, but I think that's because there are probably some relay changes to be made to not ignore these spans (I can take a look at this next week).
I think the cls html node in the description is fine. We probably will need to make some UI changes in the product to surface the description/span in some way.
1 question, should we also be attaching the pageload span id as an attribute? We may want a way to connect the standalone cls span to a pageload in the UI. Maybe there is a better otel mechanism for this?
Lms24
force-pushed
the
lms/feat-browser-cls-standalone-spans
branch
2 times, most recently
from
883bb92 to
69adb32
Compare
Lms24
force-pushed
the
lms/feat-browser-cls-standalone-spans
branch
from
69adb32 to
585a7ac
Compare
This PR adds an experimental feature to
browserTracingIntegrationto no longer tie CLS reporting to the ongoing pageload span but instead send a standalone CLS span similarly to how we report INP.The big advantage of this reporting strategy is that layout shifts happening after the pageload idle span ended, will also get reported. This should give users more accurate CLS values in the web vitals performance insights module.
There is a big challenge though we somehow needed to adress: CLS really is cumulative 😅 - the
web-vitalslibrary does not stop reporting CLS or reset the score on soft navigations (like very commonly used in SPAs). This is discussed a lot in the web-vitals library repo (GoogleChrome/web-vitals#119) but general consensus seems to be that it is not going to change (for actually good reasons).However, for our use case, where we specifically link CLS to the page that was initially loaded, it would be quite weird to report a high CLS score where a lot of layout shift only might have happened after a soft navigation.
To address this, we opted for the following, slightly customized, CLS reporting strategy:
web-vitalslibrary are ignored (as previously fwiw)To better illustrate the difference, here's a fancy diagram:
So to sum it up:
Span envelope Payload
Our integration tests show the full span envelope item payload:
For ingestion and product, there are still some open questions:
ref #13015