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

🌸 Cherry-pick request for #37072 into #37076 #37145

Closed
jridgewell opened this issue Dec 7, 2021 · 10 comments
Closed

🌸 Cherry-pick request for #37072 into #37076 #37145

jridgewell opened this issue Dec 7, 2021 · 10 comments

Comments

@jridgewell
Copy link
Contributor

Issue (P0 Bug)

#37072

Pull Request(s)

#37144

Release Tracker(s)

#37076

Channels

Beta / Experimental, Stable

Formats

Websites, Stories

Justification

Publishers can see LCP time via amp-analytics, and it's jumped from extremely good 2 second scores to 50 years. That's laughably incorrect. Because of the publisher visible impact, this is a high priority fix.

Verification Steps

  1. Search for [news]
  2. Open dev tools to network panel
  3. Click first amp result
  4. Inspect any LCP metrics sent are reasonable (in milliseconds)

Summary

No response

Impact

No response

Action Items

No response

Notifications

/cc @ampproject/release-on-duty @ampproject/wg-approvers @ampproject/cherry-pick-approvers

@ampprojectbot
Copy link
Member

ampprojectbot commented Dec 7, 2021

🌸 Cherry-Pick Progress 🌸

Hi @jridgewell, thanks for filing this cherry-pick request!
Seeing that this affects Stable, status.amp.dev will be updated with progress of the fix.
Please update this tracker as each step is completed.

  • Cherry-pick request approved (this creates an incident on status.amp.dev)
  • Cherry-pick started (this sets the incident status to "Identified")
  • Fix deployed to release channels (this sets the incident status to "Monitoring")
  • Fix verified on release channels (this resolves the incident)

@erwinmombay
Copy link
Member

approved from @ampproject/wg-approvers

probably still need @ampproject/cherry-pick-approvers

@kristoferbaxter
Copy link
Contributor

Approved.

@raja-anbazhagan
Copy link
Contributor

I understand that this is holiday time. But is there any update on this cherry pick?

@samouri
Copy link
Member

samouri commented Dec 17, 2021

#37076

See #37072 (comment). Fix was rolled out since 7 days ago. Can you confirm if the issue is fixed? If it is reoccuring, it is possible we messed up CPing beta and released a new version with the same error.

@raja-anbazhagan
Copy link
Contributor

Strange. I see the AMP version as 2112032204000

image

@raja-anbazhagan
Copy link
Contributor

raja-anbazhagan commented Dec 17, 2021

Also, in AMP Release Calendar, I see 2112032204000 as stable and 2111242025001 as LTS. Is this how it should be? Anyway, I'm being served 2112032204000. tried from different machines. Its the same.

also, #37076 is in a re-opened state.

@samouri
Copy link
Member

samouri commented Dec 22, 2021

#37076

See #37072 (comment). Fix was rolled out since 7 days ago. Can you confirm if the issue is fixed? If it is reoccuring, it is possible we messed up CPing beta and released a new version with the same error.

I confirmed that this is exactly what happened. We had patched stable with a fix, but then reintroduced the bug since we did not patch Beta (the upcoming stable). Reverting stable back to 2111242025001 now.

Thank you @raja-anbazhagan for reporting this!

@samouri
Copy link
Member

samouri commented Dec 22, 2021

@raja-anbazhagan, the fix should be live now. Please let me know if you are seeing otherwise

@raja-anbazhagan
Copy link
Contributor

Thanks @samouri . I see the updated version in the console.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants