WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 3 months ago

#42151 new enhancement

Create a filter to add information to query data saved when SAVEQUERIES is true

Reported by: CrazyJaco Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch needs-testing
Focuses: Cc:

Description

When SAVEQUERIES is set to true in wp-config.php, an array gets created and all queries to the database get logged to that array with the query, elapsed time, and stack trace.

It would be nice if there was a filter here to add other elements to this array on a per query basis.

For instance, if I were using a plugin like ElasticPress, I would like to use this filter to add in a value that might say "Query served by ElasticPress" or something to that affect.

Attachments (4)

42151.diff (681 bytes) - added by CrazyJaco 21 months ago.
First pass.
42151.2.diff (826 bytes) - added by CrazyJaco 21 months ago.
Second pass
42151.3.diff (1.2 KB) - added by CrazyJaco 17 months ago.
3rd pass. Added public helper method
42151.4.diff (1.2 KB) - added by CrazyJaco 17 months ago.
Fixing a bug.

Download all attachments as: .zip

Change History (21)

#1 @CrazyJaco
21 months ago

I will have a patch for this shortly.

@CrazyJaco
21 months ago

First pass.

#2 @CrazyJaco
21 months ago

  • Keywords has-patch added

#3 @johnbillion
21 months ago

  • Component changed from Query to Database
  • Keywords 2nd-opinion added
  • Version trunk deleted

Thanks for the patch, @CrazyJaco.

I think this is a bit of a risky change. The idea behind the saved queries and the associated debugging data is that a debugging plugin can make assumptions about what data is present in the wpdb::$queries property. If a filter is added here, then that (somewhat weak) contract is broken.

I wonder if it would be better to add an extra element to the stored query data, the value of which is a filterable array of "extra" and arbitrary information relating to the query. That way a debugging plugin can use an expose that information as it wishes, but without the need to worry about which data is present in the stored queries and in what order.

Related: #41956

#4 @CrazyJaco
21 months ago

@johnbillion Thank you for the reply! I was thinking about this last night and came to a similar realization. If two plugins added an element each to the array, they wouldn't know which element in the array was necessarily their piece of data since it is not an associative array. Changing it to one would be a breaking change so that is out of the question.

Are you suggestion something like this:

<?php
QueryData[0] => query syntax
QueryData[1] => elapsed time,
QueryData[2] => stacktrace,
Querydata[3] => apply_filter( Array( 
      custom_meta_key_1 => custom_meta_value_1,
      custom_meta_key_2 => custom_meta_value_2
    )
  )

so that QueryData[3] is just set to an empty filterable associative array?

#5 @johnbillion
21 months ago

  • Keywords needs-patch added; has-patch removed

Yeah I think that makes the most sense. A note in the documentation for the filter could be added which recommends that associative keys are used in the array.

@CrazyJaco
21 months ago

Second pass

#6 @CrazyJaco
21 months ago

  • Keywords has-patch added; needs-patch removed

@johnbillion Added an updated patch. Let me know what you think.

Testing this with a test plugin works as I would expect, except for the first query that gets run, but I think that query might fire before plugins are loaded. All other queries show the new meta data added by my test plugin.

This is the first query:
SELECT option_name, option_value FROM wp_options WHERE autoload = 'yes'

#7 @johnbillion
21 months ago

A plugin in mu-plugins should be able to catch every query. Patch looks good, I'll review properly a bit later.

#8 @CrazyJaco
21 months ago

Thank you for your time and attention!

#9 @CrazyJaco
20 months ago

Hey @johnbillion, just checking in to see if you have had a moment to review this patch/ticket properly.

Thanks!

#10 @CrazyJaco
17 months ago

  • Keywords needs-refresh added; 2nd-opinion has-patch removed

This ticket has sat for a while and after some thought I think the call to log a query should be extracted into a method in the class so it can be called elsewhere.

The way it currently works, you can't really log a query that goes to an external source (ElasticSearch) outside of the wordpress database and never makes it this far in the query call stack.

I will update the patch for feedback.

@CrazyJaco
17 months ago

3rd pass. Added public helper method

#11 @CrazyJaco
17 months ago

  • Keywords has-patch added; needs-refresh removed

Uploaded new patch.

@CrazyJaco
17 months ago

Fixing a bug.

This ticket was mentioned in Slack in #core by crazyjaco. View the logs.


15 months ago

#13 @SergeyBiryukov
15 months ago

  • Milestone changed from Awaiting Review to 5.0

#14 @pento
8 months ago

  • Milestone changed from 5.0 to 5.1

#15 @pento
5 months ago

  • Milestone changed from 5.1 to 5.2

This ticket was mentioned in Slack in #core by crazyjaco. View the logs.


3 months ago

#17 @desrosj
3 months ago

  • Keywords needs-testing added
  • Milestone changed from 5.2 to 5.3

This still needs feedback, testing, and a proper review. Punting to 5.3.

Note: See TracTickets for help on using tickets.