wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

[ETK/Error-Reporting][Sentry] Pass a dynamic release name based of the WPCOM git hash to Sentry

Open fullofcaffeine opened this issue 3 years ago • 10 comments
trafficstars

Project: p9oQ9f-18L-p2

Proposed Changes

In D84565-code, systems added a global constant called WPCOM_DEPLOYED_GIT_HASH that always points to the current git hash HEAD deployed in WPCOM. This hash is used to build a release name that is finally passed as part of the Sentry SDK init for the wpcom-gutenberg-wp-admin project. The final release name is a string in the following format: WPCOM_<WPCOM_DEPLOYED_GIT_HASH>.

In practice, this means that after each new WPCOM release, this constant gets updated with the new hash, and we get a new release* created in Sentry (to be done, the diff link will be pasted here later); finally, the SDK for the wpcom-gutenberg-wp-admin project in Sentry is instantiated with the new release name.

As a bonus, we try to keep the WPCOM-specific parts of the code isolated and to a minimum. The only part that references WPCOM is the WPCOM_DEPLOYED_GIT_HASH constant. The frontend JS data that's passed through wp_localize_script is called releaseName. This was done to make it slightly easier to generalize this logic in the future, in case we want to allow other WordPress instances (other than WPCOM) to setup Error Reporting and/or decouple from Sentry.

*a Sentry release.

Testing Instructions

  • Sync this build of ETK to your sandbox
  • In your browser devtools, open the JS console for your test simple site, and type: window.A8C_ETK_ErrorReporting_Config?.releaseName, it should return "WPCOM_<SHA>".

TODO

  • [ ] Pass the release name to the TC build that uploads source-maps for Gutenberg.
  • [ ] Improve/abstract release name in a global constant (see: https://github.com/Automattic/wp-calypso/pull/66186#discussion_r936073901).
  • [ ] Create release in Sentry after each WPCOM deploy (WIP - is @ D85265-code)

Pre-merge Checklist

  • [ ] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [ ] Have you checked for TypeScript, React or other console errors?
  • [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to

  • Systems-Request: pMz3w-fwg-p2;
  • Diff adding the WPCOM_DEPLOYED_GIT_HASH constant: D84565-code;
  • Diff to create the Sentry release after each WPCOM deploy: D85265-code;
  • https://github.com/Automattic/wp-calypso/pull/63313
  • https://github.com/Automattic/wp-calypso/pull/63313
  • https://github.com/Automattic/wp-calypso/pull/62965
  • https://github.com/Automattic/wp-calypso/pull/63260
  • https://github.com/Automattic/wp-calypso/pull/66315

fullofcaffeine avatar Aug 02 '22 21:08 fullofcaffeine

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Aug 02 '22 21:08 matticbot

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit pass/wpcom-deployed-git-hash-to-sentry on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

matticbot avatar Aug 02 '22 22:08 matticbot

Nice, this looks really straightforward to me. The only thing I noticed is that when I log the info in the browser, it looks like this:

I did notice this when testing in sandboxes. I think there's a bug in the function that fetches the git hash (introduced here: D84565-code), I'll have a look today.

fullofcaffeine avatar Aug 04 '22 21:08 fullofcaffeine

Oh nice. It looks like the hash file is present on my sandbox. I wonder if it reads in some whitespace or something.

noahtallen avatar Aug 04 '22 21:08 noahtallen

Oh nice. It looks like the hash file is present on my sandbox. I wonder if it reads in some whitespace or something.

Spot on! This has been fixed here: D85480-code.

fullofcaffeine avatar Aug 05 '22 17:08 fullofcaffeine

Unfortunately, I'm seeing this in the JS console on my sandbox:

> window.A8C_ETK_ErrorReporting_Config.releaseName
"WPCOM_NO_RELEASE" 

noahtallen avatar Aug 11 '22 00:08 noahtallen

Unfortunately, I'm seeing this in the JS console on my sandbox:

> window.A8C_ETK_ErrorReporting_Config.releaseName
"WPCOM_NO_RELEASE" 

Oh, I wonder if that's because the constant is still not defined at the time enqueue_script is called as part of the admin_enqueue_scripts action 🤔. Thanks for testing! I'll debug and fix this tomorrow morning.

fullofcaffeine avatar Aug 11 '22 01:08 fullofcaffeine

Oh my ... defined accepts a string and I'm passing the constant itself 🤦🏻 I'll fix this now.

fullofcaffeine avatar Aug 11 '22 01:08 fullofcaffeine

@noahtallen Fixed in https://github.com/Automattic/wp-calypso/pull/66186/commits/e705f2863f59dc9af72bc0c405c9a42b2ded8b7a. Thanks again for catching this!

fullofcaffeine avatar Aug 11 '22 01:08 fullofcaffeine