10up-experience
10up-experience copied to clipboard
DB Query Monitor
Description of the Change
This PR adds a "submodule" to Support Monitor, logging, and sending heavy SQL queries.
It also remove some instance()->setup()
calls, as instance()
already calls setup()
in its first execution (what was causing hooks to be added twice.)
Benefits
While updating plugins on staging we can detect any major changes in the database structure, so we can plan accordingly the same changes for production.
Verification Process
- Installed the plugin in a WordPress test installation
- Installed Support Monitor in another WP test install
- Checked messages in the "system" group coming
Changelog Entry
Added: DB Query Monitor
@bengreeley I've refactored the code a bit to display the queries in the site rather than sending them to Support Monitor and also added some docs. Do you mind giving it another look? Thank you very much for all the feedback so far!
ps.: I'm still considering this WIP as I have to test it with a multisite and also add a slack message in the case any heavy query was detected. Any thoughts on this, btw?
ps.: I'm still considering this WIP as I have to test it with a multisite and also add a slack message in the case any heavy query was detected. Any thoughts on this, btw?
@felipeelia To integrate with Slack, you'll want to look in the Support Monitor Dashboard repo and there will be checks performed during the message ingestion. I'd recommend to send this information with the 'systems' insight so you can piggy-back off that functionality and then in the message ingestion code you would want to check for the heavy query variable that was sent via the message.
Check out this code for reference: https://gitlab.10up.com/10up-internal/system-report/system-report-dashboard/-/blob/master/plugins/support-monitor/includes/functions/api/rest-api.php#L427
@bengreeley I've refactored the code to make the entire feature opt-in, rather than opt-out. You'll see that there are two core concepts: availability (the feature is only available in non-prod envs by default) and state (enabled/disabled). To have it working, it needs to be available AND enabled.
For the slack integration, I talked to @tott and we are seeing it as a "phase two", so let's ignore that for now.
Do you mind giving it yet another review and sharing your thoughts about it? When it looks okay, I need to change the @since x.x
strings to the actual version.
Thanks!
I think the changes look better, thanks for making those @felipeelia ! We'll want to do thorough testing on some internal environments before using this on production environments and also test with Support Monitor to make sure that data is brought over correctly. You'll likely need to make some changes to that code to accommodate for this new field/data. There's a staging environment we can test on as well if we want to test these changes on a staging version of Support Monitor.
I'll hold off on approving this until it's been tested, but so far so good!
Thanks, @bengreeley! I'll reach out internally about the best environments to use to test it. Regarding changes needed in the Support Monitor, I'm already testing it locally and as the field is just added to the message, it worked out of the box :)