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: Upgrade to web-vitals@4.0.0 #152

Merged
merged 4 commits into from
May 22, 2024
Merged

Conversation

wolfgangcodes
Copy link
Contributor

@wolfgangcodes wolfgangcodes commented May 16, 2024

Which problem is this PR solving?

Web vitals but released 4.0.0 which includes an updates to field names and additional for LCPAttribution, INPAttribution and TTFBAttribution and deprecated onFID. See the full changes here.

Short description of the changes

Upgrade to web-vitals@4.0.0. Full change log of the underlying changes can be found here.

For this release, we'll send both the updated field names and the deprecated field names, which will be removed in the next major release.
Additionally, onFID has been deprecated and will removed in the next major release.

For this instrumentation we have updated attribute payloads as follows:

LCP:

  • Updated resource_load_time -> resource_load_duration
  • Deprecated resource_load_time

INP:

  • Updated element -> interaction_target
  • Deprecated element
  • Updated event_type -> interaction_type
  • Deprecated event_type
  • Added interaction_time
  • Added input_delay
  • Added interaction_target_element
  • Added next_paint_time
  • Added presentation_delay
  • Added processing_duration

TTFB:

  • Updated waiting_time -> waiting_duration
  • Deprecated waiting_time
  • Updated dns_time -> dns_duration
  • Deprecated dns_time
  • Updated connection_time -> connection_duration
  • Deprecated connection_time
  • Updated request_time -> request_duration
  • Deprecated request_time
  • Added cache_duration

How to verify that this has the expected result

See that the new new and updated properties are being reported.

@wolfgangcodes wolfgangcodes changed the title feat!: Upgrade to web-vitals@4.0.0 feat: Upgrade to web-vitals@4.0.0 May 16, 2024
@wolfgangcodes wolfgangcodes requested a review from pkanal May 16, 2024 19:43
@wolfgangcodes wolfgangcodes marked this pull request as ready for review May 16, 2024 19:43
@wolfgangcodes wolfgangcodes requested review from a team as code owners May 16, 2024 19:43
Copy link
Contributor

@pkanal pkanal left a comment

Choose a reason for hiding this comment

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

🎉 Amazing! Can we these new attributes to our docs for web vitals attributes? Up to you if you want to do it as a separate PR, that works too!

[`${attrPrefix}.presentation_delay`]: presentationDelay,
[`${attrPrefix}.processing_duration`]: processingDuration,
// These will be deprecated in a future version
[`${attrPrefix}.element`]: interactionTarget,
Copy link
Contributor

@ahrbnsn ahrbnsn May 21, 2024

Choose a reason for hiding this comment

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

definitely defer to purvi & other experts here, but i'd like to vote to keep {attrPrefix}.element as the designated field (or at least dual write) — one struggle for folks less familiar with the specific data shape of various events is trying to remember what field name goes with what event type. ie instead of making people remember for inp, the element is listed under interaction_target, for cls, it's the largest_shift_target, and for lcp, it's element, keep all targets referenced under the same name — the element

more in-depth writeup/thoughts here: #61

For metrics which include reference to a specific element — largest contentful paint, cumulative layout score, interaction to next paint — that element is always included in [metric].element, instead of a metric-specific key (inp.element instead of inp.event_target, cls.element instead of cls.largest_shift_target. lcp.element remains lcp.element). Although we do lose specific signal around why the element is being referenced, I think there's benefit to folks only needing to remember one field name. Each metric has only one specific, metric-related reason for referencing an element; I don't think we're losing too much signal by shedding the specificity in the key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's great additional context! I think that makes me more inclined to dual write. My _evolved thinking is that we should map 1-1 with the shape of web vitals data, and we can _additionally _ in a separate namespace provide more ergonomic access and/or derived data. Since we're still in EA and not removing any fields, let's merge this, and we open up a follow-up PR to tackle any changes as our thinking evolves.

@wolfgangcodes wolfgangcodes merged commit 090e82d into main May 22, 2024
8 checks passed
@wolfgangcodes wolfgangcodes deleted the wg/upgrade-web-vitals-4.0.0 branch May 22, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants