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.

Sign up for GitHub

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

DB Query Monitor #96

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

DB Query Monitor #96

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e9a622f
Remove unnecessary setup() calls
felipeelia Nov 8, 2021
6da59ce
Initial version of DBQueryMonitor
felipeelia Nov 8, 2021
0bec10e
Escape query to avoid breaking json_decode later
felipeelia Nov 10, 2021
07cd952
New queries page, reduce Support Monitor message + docs
felipeelia Nov 11, 2021
e2a0f9b
Merge branch 'develop' into feature/db-query-monitor
felipeelia Dec 9, 2021
21f2ab6
Change DB Query Monitor to be Opt In
felipeelia Dec 9, 2021
91df409
Escape INSERT and UPDATE queries
felipeelia Dec 10, 2021
ef6bea0
Apply data size restrictions and move to its own message
felipeelia Jan 31, 2022
f2b84cc
Add query categorization
felipeelia Feb 8, 2022
5037c4a
Merge branch 'develop' into feature/db-query-monitor
felipeelia Feb 8, 2022
301f689
Merge branch 'develop' into feature/db-query-monitor
felipeelia Mar 23, 2022
83da9c1
Merge branch 'develop' into feature/db-query-monitor
felipeelia Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions 10up-experience.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,27 +70,27 @@ function( $plugin_info, $http_response = null ) {
define( 'TENUP_EXPERIENCE_IS_NETWORK', (bool) $network_activated );

if ( ! defined( 'TENUP_DISABLE_BRANDING' ) || ! TENUP_DISABLE_BRANDING ) {
AdminCustomizations\Customizations::instance()->setup();
AdminCustomizations\Customizations::instance();
}

API\API::instance()->setup();
Authentication\Usernames::instance()->setup();
Authors\Authors::instance()->setup();
Gutenberg\Gutenberg::instance()->setup();
Headers\Headers::instance()->setup();
Plugins\Plugins::instance()->setup();
PostPasswords\PostPasswords::instance()->setup();
SupportMonitor\Monitor::instance()->setup();
SupportMonitor\Debug::instance()->setup();
Notifications\Welcome::instance()->setup();
API\API::instance();
Authentication\Usernames::instance();
Authors\Authors::instance();
Gutenberg\Gutenberg::instance();
Headers\Headers::instance();
Plugins\Plugins::instance();
PostPasswords\PostPasswords::instance();
SupportMonitor\Monitor::instance();
SupportMonitor\Debug::instance();
Notifications\Welcome::instance();

/**
* We load this later to make sure there are no conflicts with other plugins.
*/
add_action(
'plugins_loaded',
function() {
Authentication\Passwords::instance()->setup();
Authentication\Passwords::instance();
}
);

Expand Down
173 changes: 173 additions & 0 deletions includes/classes/SupportMonitor/DBQueryMonitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
<?php
/**
* Database Queries Monitor. A submodule of Support Monitor to report heavy SQL queries executed on staging.
*
* @since x.x
* @package 10up-experience
*/

namespace TenUpExperience\SupportMonitor;

/**
* DBQueryMonitor class
bengreeley marked this conversation as resolved.
Show resolved Hide resolved
*/
class DBQueryMonitor {

/**
* The transient name
*
* This transient stores all the queries logged.
*/
const TRANSIENT_NAME = 'tenup_experience_db_queries';

/**
* Setup module
*/
public function setup() {
$production_environment = Monitor::instance()->get_setting( 'production_environment' );
if ( 'no' === $production_environment || apply_filters( 'tenup_experience_log_heavy_queries', false ) ) {
add_filter( 'query', [ $this, 'maybe_log_query' ] );
}
}

/**
* Conditionally log a query
*
* Get all potential heavy queries (CREATE, ALTER, etc.) and store it,
* ignoring transients and options by default.
*
* @param string $query The SQL query.
* @return string
*/
public function maybe_log_query( $query ) {
global $wpdb;

if ( ! preg_match( '/^\s*(create|alter|truncate|drop|insert|delete|update|replace)\s/i', $query ) ) {
bengreeley marked this conversation as resolved.
Show resolved Hide resolved
return $query;
}

if ( false !== strpos( $query, 'transient_' ) ) {
return $query;
}

if ( false !== strpos( $query, $wpdb->options ) ) {
return $query;
}

if ( apply_filters( 'tenup_experience_log_query', true, $query ) ) {
bengreeley marked this conversation as resolved.
Show resolved Hide resolved
$this->log_query( $query );
}

return $query;
}

/**
* Get the logged queries. Also removes from the transient all queries logged for more than 7 days.
*
* @return array
*/
public function get_report() {
$date_limit = strtotime( '7 days ago' );
bengreeley marked this conversation as resolved.
Show resolved Hide resolved

$stored_queries = array_filter(
(array) get_transient( self::TRANSIENT_NAME ),
function ( $query_date ) use ( $date_limit ) {
return strtotime( $query_date ) > $date_limit;
},
ARRAY_FILTER_USE_KEY
);

set_transient( self::TRANSIENT_NAME, $stored_queries );

return $stored_queries;
}

/**
* Log/store a SQL query
*
* Queries are stored like:
*
* 'YYYY-MM-DD' => [
* 'a5be998feee8968155052c4d332a7223' => [ // md5 of file:line:query
* 'query' => 'ALTER TABLE wp_my_db_heavy_plugin CHANGE COLUMN `id` id BIGINT(20) UNSIGNED NOT NULL AUTO_INCREMENT',
* 'file' => '/var/www/html/wp-content/plugins/my-db-heavy-plugin/my-db-heavy-plugin.php',
* 'line' => 53,
* 'count' => 1,
bengreeley marked this conversation as resolved.
Show resolved Hide resolved
* ]
* ]
*
* @param string $query The SQL query.
*/
protected function log_query( $query ) {
static $stored_queries;
if ( empty( $stored_queries ) ) {
$stored_queries = get_transient( self::TRANSIENT_NAME ) ?? [];
}

$current_date = date_i18n( 'Y-m-d' );

if ( ! isset( $stored_queries[ $current_date ] ) ) {
$stored_queries[ $current_date ] = [];
}

$main_caller = $this->find_main_caller();

$key = md5(
$main_caller['file'] . ':' .
$main_caller['line'] . ':' .
$query
);

if ( isset( $stored_queries[ $current_date ][ $key ] ) ) {
$stored_queries[ $current_date ][ $key ]['count']++;
} else {
$stored_queries[ $current_date ][ $key ] = [
'query' => stripslashes( $query ),
Copy link

Choose a reason for hiding this comment

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

@felipeelia How will we be making sure $query doesn't contain any sensitive information such as user e-mail addresses or other information? We'll want to be really careful about sending that sort of sensitive information to Support Monitor where we could be potentially liable for storing that information if we were ever hacked. I'd recommend we do some sort of cleanup or see if you can get the version of the query that has placeholders like %s in place of actual data.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bengreeley my bad on this one. Trying to get the version of the query with placeholders can be hard (or near impossible depending on the plugin), so cleaning it up would be probably easier. I'll work on that, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@bengreeley with the change I made, I'm curious to know your opinion about this topic. We are not sending the query anymore but only storing it in a transient. As the entire feature is only available in non-production environments, do you think we still need to escape the queries? (I don't want to make it insecure but also don't want to over-engineer it). If we decide to escape them, would you be okay with using a lib like this?

Copy link

Choose a reason for hiding this comment

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

I wonder if you create a regex or search/replace to remove anything between '' and "" if that would be sufficient? I'm not as worried if we're storing it on the server, but we'll want to be aware that if we have queries that have sensitive user information that's available to anybody or any plugin, we'll want to think of a way to clean it up.As long as it's information that is freely available in any other WP table it shouldn't be a bit deal, but should be given adequate testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bengreeley I've pushed a commit that does that. When I save a long post these are the queries being saved (the post update and the revision created):

Query: UPDATE `wp_posts` SET `post_author` = 1, `post_date` = ?, `post_date_gmt` = ?, `post_content` = ?, `post_content_filtered` = ?, `post_title` = ?, `post_excerpt` = ?, `post_status` = ?, `post_type` = ?, `comment_status` = ?, `ping_status` = ?, `post_password` = ?, `post_name` = ?, `to_ping` = ?, `pinged` = ?, `post_modified` = ?, `post_modified_gmt` = ?, `post_parent` = 0, `menu_order` = 0, `post_mime_type` = ?, `guid` = ? WHERE `ID` = 7
File: /var/www/html/wp-includes/post.php
Line: 4196
Count: 1

Query: INSERT INTO `wp_posts` (`post_author`, `post_date`, `post_date_gmt`, `post_content`, `post_content_filtered`, `post_title`, `post_excerpt`, `post_status`, `post_type`, `comment_status`, `ping_status`, `post_password`, `post_name`, `to_ping`, `pinged`, `post_modified`, `post_modified_gmt`, `post_parent`, `menu_order`, `post_mime_type`, `guid`) VALUES (1, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 7, 0, ?, ?)
File: /var/www/html/wp-includes/post.php
Line: 4219
Count: 1

Does that look okay?

'file' => $main_caller['file'],
'line' => $main_caller['line'],
'count' => 1,
];
}

set_transient( self::TRANSIENT_NAME, $stored_queries );
}

/**
* Based on the debug backtrace, try to find the main caller, i.e., the plugin/theme
* that fired the query.
*
* Simple SQL queries generally come from wp-db.php. dbDelta calls come from upgrade.php.
* We are usually interested in the caller immediately before those.
*
* @return array
*/
protected function find_main_caller() {
$debug_backtrace = debug_backtrace(); // phpcs:ignore

// Remove this plugin references of the backtrace.
array_shift( $debug_backtrace );
array_shift( $debug_backtrace );

$main_caller = null;

$wp_db_found = false;
foreach ( $debug_backtrace as $caller ) {
$is_wp_db_file = ( false !== strpos( $caller['file'], 'wp-db.php' ) || false !== strpos( $caller['file'], 'upgrade.php' ) );
if ( $is_wp_db_file ) {
$wp_db_found = true;
}
if ( ! $wp_db_found || $is_wp_db_file ) {
continue;
}
$main_caller = $caller;
break;
}

// If a caller was not found simply get the first item of the backtrace.
if ( ! $main_caller ) {
$main_caller = array_shift( $debug_backtrace );
}

return $main_caller;
}
}
11 changes: 11 additions & 0 deletions includes/classes/SupportMonitor/Monitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,22 @@
* Monitor class
*/
class Monitor extends Singleton {
/**
* The DBQueryMonitor instance.
*
* @since x.x
* @var DBQueryMonitor
*/
protected $db_query_monitor;

/**
* Setup module
*
* @since 1.7
*/
public function setup() {
$this->db_query_monitor = new DBQueryMonitor();
$this->db_query_monitor->setup();

if ( TENUP_EXPERIENCE_IS_NETWORK ) {
add_action( 'wpmu_options', [ $this, 'ms_settings' ] );
Expand Down Expand Up @@ -461,6 +471,7 @@ public function send_daily_report() {
$this->format_message(
[
'php_version' => $this->get_php_version(),
'db_queries' => $this->db_query_monitor->get_report(),
bengreeley marked this conversation as resolved.
Show resolved Hide resolved
],
'notice',
'system'
Expand Down