Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#58584 closed enhancement (fixed)

Telemetry / Stats: Send the raw DB version identifier, and/or server type.

Reported by: dd32's profile dd32 Owned by: dd32's profile dd32
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description

Currently wordpress.org/about/stats lists MySQL version stats based on what WordPress sends with it's update calls.

This is what is sent to WordPress.org:
https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/update.php#L76-L82
which is generated from this:
https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/class-wpdb.php#L4101-L4111

However, $wpdb->db_version() is not always 100% accurate, and can return the "incorrect" data for some servers.
For example:

$wpdb->db_server_info();
string(25) "5.5.5-10.2.44-MariaDB-log"

wp> $wpdb->db_version();
string(5) "5.5.5"

It would be ideal if we could either/and/or:
a) Parse the proper version number out core side
b) Report the full version string ( 5.5.5-10.2.44-MariaDB-log )
c) Report the full identifier ([ 'MariaDB', '10.2.44' ] )
d) Report the value of $wpdb->is_mysql
e) All of the above

For cases where a DB drop-in is used, is it possible to determine the "type" from that possibly? I guess the final classname of $wpdb might specify it?

We can do some parsing on the WordPress API side to trim the data back to a single valid identifier, such as MariaDB 10.2.44 or MySQL 5.5.5 but not with what's currently being reported.

For some background, see https://meta.trac.wordpress.org/ticket/7080

Change History (17)

#1 @afragen
11 months ago

@costdev, anything transferable from Report a Bug?

#2 follow-up: @costdev
11 months ago

  • Keywords reporter-feedback added

I was just wondering that.

@dd32 if you install WordPress Beta Tester and click the Report a Bug adminbar link, does the database string in the "Environment" section reflect any of your preferred data in the various scenarios, such as MariaDB, dropins, etc?

Last edited 11 months ago by costdev (previous) (diff)

This ticket was mentioned in PR #4653 on WordPress/wordpress-develop by @dd32.


11 months ago
#3

  • Keywords has-patch added
  • Sends the raw *SQL version field from WPDB if available
  • Lifts the 'interface' code from Site Health and sends that too.

For example:

Current After
MariaDB mysql_verison: 5.5.5 mysql_version: 5.5.5-10.2.44-MariaDB <br> mysql_interface: mysqli
WordPress Playground <br> w/ SQLite driver mysql_verison: 5.5 mysql_version: 3.40.1 <br> mysql_interface: WP_SQLite_Translator

Unlike Site Health, this doesn't use SELECT VERSION(), as it seems unlikely to be needed.

Trac ticket: https://core.trac.wordpress.org/ticket/58584

#4 in reply to: ↑ 2 @dd32
11 months ago

Replying to costdev:

if you install WordPress Beta Tester and click the Report a Bug adminbar link, does the database string in the "Environment" section reflect any of your preferred data

Yes, it (and Site Health) list the data I proposed above.

The main difference is that Site Health (And I assume the Beta Tester) don't use the WPDB methods, I guess as it's unreliable?

Instead of $wpdb->db_version() it returns something similar to $wpdb->db_server_info() but via a SQL query of SELECT VERSION(); to workaround the above 5.5.5-10.2.44-MariaDB-log "bug":

wp> $wpdb->db_server_info();
string(25) "5.5.5-10.2.44-MariaDB"

wp> $wpdb->get_var( "SELECT VERSION();" );
string(19) "10.2.44-MariaDB"

Site Health does prefix it with the connection type, mostly through get_class( $wpdb->dbh ) which might work, but I'm not sure what that would return for all dropins.

Changing the reported value from $wpdb->db_version() to $wpdb->db_server_info(), and using the extension usage would probably suffice.

I've submitted a PR for that: https://github.com/WordPress/wordpress-develop/pull/4653

Notably, I don't really see the point in using SELECT VERSION() here, but I wouldn't be against seeing the customisations made in Site Health being merged with WPDB..

#5 @l1nuxjedi
11 months ago

You can also use the information from SHOW VARIABLES LIKE 'VERSION', which will have the correct version number in there for <=10.2. I think offhand the VERSION_COMMENT variable will identify the server type.

#6 @costdev
11 months ago

  • Keywords reporter-feedback removed

@spacedmonkey commented on PR #4653:


11 months ago
#7

@dd32 Have you tested with hyperdb and ludicrousdb?

This ticket was mentioned in Slack in #hosting by crixu. View the logs.


11 months ago

#9 @johnbillion
11 months ago

In my Query Monitor plugin I switched to using $wpdb->db_server_info() instead of $wpdb->db_version() a long time ago because of the parsing problems with some version numbers. SELECT VERSION() is used as a fallback if the value is empty, which I did see once.

I also use the get_class( $dbh ) method to get a name that's generally going to be representative of the database driver. It operates on the handler class so it shouldn't matter if there's a database dropin class in use.

This ticket was mentioned in Slack in #hosting by jadonn. View the logs.


11 months ago

@dd32 commented on PR #4653:


11 months ago
#11

Have you tested with hyperdb and ludicrousdb?

  • Fresh install: Yes.
  • HyperDB: Yes (It's actually the MariaDB example above)
  • LudicrousDB: Not directly, but I did review the code.
    • db_version() + db_server_info() are close enough to core to behave the same as core: https://github.com/stuttter/ludicrousdb/blob/master/ludicrousdb/includes/class-ludicrousdb.php#L1688-L1722
    • If a dropin doesn't return the same data-types here as core, it might record incorrect in stats, but I don't think it would be possible to break the update requests.
    • The mysql_interface might return falsey though, as if a query hasn't been made $wpdb->dbh might not be setup correctly (This equally applies to HyperDB I believe). This is something that would need to be handled api-side, and would only affect a reasonably small number of sites, in these cases db_version() is also going to return '' though.

@dd32 commented on PR #4653:


11 months ago
#12

Unlike Site Health, this doesn't use SELECT VERSION(), as it seems unlikely to be needed.

So, I've tracked down this double-version, 5.5.5-10.x.x-MariaDB.

It turns out that MariaDB return that to resolve issues with compat with MySQL during the initial connection - which is where the PHP function is pulling it from _in some versions_.

WordPress then picks up that compat and uses the 5.5.5 as the version as a result.

#13 @mukesh27
11 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 6.3

Thanks @dd32 for the ticket and PR. The PR got enough approval for merge.

This one looks good to go in 6.3. Adding for commit consideration.

Feel free to punt it if it did not commit today.

#14 @dd32
11 months ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 56050:

Upgrade/Install: Pass the full database version string to WordPress.org for parsing.

This aims to make the provided statistical data more complete, by correctly identifying the version for a wider selection of database types and servers.

Props mukesh27, costdev, spacedmonkey, johnbillion.
Fixes #58584.

This ticket was mentioned in Slack in #hosting by javier. View the logs.


11 months ago

This ticket was mentioned in Slack in #hosting by amykamala. View the logs.


11 months ago

Note: See TracTickets for help on using tickets.