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(site-kit): add logging when site kit disconnects #3472

Merged
merged 9 commits into from
Oct 18, 2024

Conversation

chickenn00dle
Copy link
Contributor

@chickenn00dle chickenn00dle commented Oct 11, 2024

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1206274567818686/1207849727795693

This PR adds newspack manager logging whenever we detect a site kit disconnection via:

  1. the delete_option_{option} hook for the has_connected_admin option.
  2. the update_user_metadata hook for when the disconnected_reason user meta is updated.
  3. a cron job that checks to see if there are any admin connection daily

For the first two cases, a backtrace is included in the log. For the final one, we simply log a message.

Screenshot 2024-10-15 at 14 59 47

How to test the changes in this Pull Request:

  1. Tunnel your local site so your URL is public and connect Google Site Kit
  2. Access admin via the local url (not the public one) to trigger a Site Kit disconnect.
  3. Using something like WP-Crontrol, verify a new daily newspack_googlesitekit_disconnection_logger cron event is present.
  4. Trigger the event and check for a log file in /tmp/ like /tmp/2024-XX-XX-newspack_googlesitekit_disconnect
  5. Verify the following logs are present:
  • No active Google Site Kit connections found
  • Google Site Kit has been disconnected for all admins
  • Google Site Kit has been disconnected with reason REASON

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle chickenn00dle force-pushed the feat/site-kit-add-logging-on-disconnect branch 2 times, most recently from fc03623 to b948ebf Compare October 11, 2024 20:47
@chickenn00dle chickenn00dle force-pushed the feat/site-kit-add-logging-on-disconnect branch from b948ebf to 856f034 Compare October 15, 2024 18:02
@chickenn00dle chickenn00dle force-pushed the feat/site-kit-add-logging-on-disconnect branch from 3e50ac4 to 023544e Compare October 15, 2024 18:24
@chickenn00dle chickenn00dle force-pushed the feat/site-kit-add-logging-on-disconnect branch from fc6877b to 81e290e Compare October 15, 2024 18:56
@chickenn00dle chickenn00dle marked this pull request as ready for review October 15, 2024 18:57
@chickenn00dle chickenn00dle requested a review from a team as a code owner October 15, 2024 18:57
@chickenn00dle chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Oct 16, 2024
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Beautiful.

I did a small change to bum the log level for the cron check so we can get more proactive alerts

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Oct 18, 2024
@chickenn00dle chickenn00dle merged commit 62bf98c into trunk Oct 18, 2024
8 checks passed
@chickenn00dle chickenn00dle deleted the feat/site-kit-add-logging-on-disconnect branch October 18, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants