-
Notifications
You must be signed in to change notification settings - Fork 47
Query Monitor support #212
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
base: develop
Are you sure you want to change the base?
Conversation
81c83c2
to
4dfca33
Compare
2e03007
to
09cdd0f
Compare
09cdd0f
to
5c2265e
Compare
@@ -478,6 +487,35 @@ public function query( $query ) { | |||
$return_val = $num_rows; | |||
} | |||
|
|||
// Query monitor integration: | |||
if ( $query_monitor_active && class_exists( 'QM_Backtrace' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way we could use the QM_DB class directly without recreating its logic here? I see it's class QM_DB extends wpdb
. Perhaps we could provide it with a version of wpdb
that uses SQLite?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamziel Inspired by what WP CLI does with wp-config.php
, I guess we could try to read the QB_DB
contents, change the extends ...
clause, and then eval
it 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nono, that will break on plugin update. I've meant not including the root wpdb
and providing our own class with that name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamziel Oh, but we're extending wpdb
, we need to load it, at least under a different name — and I guess that's not really doable without actually rewriting it at some point.
What I meant is adjusting QM_DB
on the fly. This could be fine, as when QM is active, we don't expect performance to be critical anyway. Something like:
$contents = file_get_contents( '.../QM_DB.php' );
$contents = str_replace( '<?php', '', $contents );
$contents = str_replace( 'extends wpdb', 'extends WP_SQLite_DB', $contents );
eval( $contents );
$wpdb = new QM_DB( ... );
But maybe it's better to suggest some improvements upstream so we can reuse some code without the hacks? For example, I think that making the inheritance configurable can be done pretty simply:
$wpdb_class = defined( 'WPDB_CLASS' ) && class_exists( WPDB_CLASS ) ? WPDB_CLASS : 'wpdb';
class_alias( $wpdb_class, 'QM_WPDB' );
class QM_DB extends QM_WPDB { ... }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upstream improvement sound like a great idea. Until then, this PR looks solid. Thank you 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one idea but otherwise I think it's a good solution. Thank you @JanJakes! Is there any way we could add this to our test suite? Ideally not via Playground, just to separate the tool from its testing infrastructure.
This PR integrates Query Monitor so that it can be used together with the SQLite Database Integration plugin, and it extends the Query Monitor panel to include SQLite query information.
This implementation uses some tricks and workarounds to achieve this functionality. We could improve the integration by proposing upstream changes to the Query Monitor plugin.
The
wp-content/db.php
issueThe main issue to overcome was that both plugins rely on the usage of the
wp-content/db.php
database drop-in file. To resolve this, I’ve enabled the SQLite plugin to override thedb.php
file created by Query Monitor while also ensuring it eagerly boots Query Monitor when needed. The behavior depends on which plugin is activated first:db.php
file will never be replaced by Query Monitor. The SQLite plugin will detect when Query Monitor is active and boot it eagerly while also storing detailed query information for Query Monitor to consume.db.php
file, and it will retain the eager boot logic for Query Monitor. The user will be notified about thedb.php
file replacement.Extended query information for Query Monitor
The SQLite plugin now stores enhanced query information for Query Monitor when it’s active, effectively mirroring a tiny portion of the Query Monitor code. To reduce duplication, we can propose changes to the Query Monitor codebase that would allow this logic to be reused rather than reimplemented.
Extending the Query Monitor panel
The Query Monitor panel was extended to provide information about the executed SQLite queries for each MySQL query. This makes it easy to inspect and debug which SQLite queries correspond to each MySQL statement.
Currently, Query Monitor doesn’t offer a way to customize the rendered query information. To overcome this limitation, I implemented this by capturing the generated HTML and modifying it before it is sent to the browser. This is another area where we could improve integration by proposing upstream changes to the Query Monitor plugin.
Playground support
Playground loads the SQLite Database Integration differently, without creating a
db.php
file. As a result, third-party plugins can still inject theirdb.php
drop-in. For Query Monitor, I’ve added a simple fix. Setting theQM_DB_SYMLINK
constant tofalse
prevents it from using thedb.php
drop-in, and the SQLite plugin will boot it as needed.