rdbms: Restore debug toolbar "Queries" feature
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 5 Oct 2019 01:56:12 +0000 (02:56 +0100)
committerKrinkle <krinklemail@gmail.com>
Tue, 15 Oct 2019 18:35:37 +0000 (18:35 +0000)
commit88e3a888cc6823f11df79ac53eb963a7e4d2871a
tree339b4722961b9203e9e70d4fec30b813dbfd8cf4
parent824ecd9b75bcbcbda0d9330656ea0f06ce288c3a
rdbms: Restore debug toolbar "Queries" feature

This broke after e0cc49ce3971e19, due to the field 'master'
being removed from the log context. The LegacyLogger logic
forwarding these messages to MWDebug (for the debug toolbar)
however, was dependant on.

Users of debug toolbar experienced a silent failure because the
logic in question is very tolerant of missing fields. This is
because it uses those fields to distinguish the 'sql' messages
from channel=DBQuery from other messages in the same channel.
Making that less fragile is outside the scope of this commit.

This commit:

* Restore the basic functionality by making sure MWDebug::query()
  gets called again for DBQuery messages.

* Remove the code relating to the 'master' field as this no longer
  exists in RDBMS. It also wasn't used anywhere (to be used,
  it would need to be read by mediawiki.debug/debug.js).

* Remove unexpanded "{method}" and "{runtime}" noise in the debug
  toolbar text. This was introduced by he conversion to PSR-3
  logging.. These fields are already rendered separately by
  the toolbar and should not be part of the "SQL" column.
  To do this, we need to log the $sql bit as its own key, so
  I've made this a context field as well.

* Reduce the condition logic in LegacyLogger to only looking for
  'DBQuery' and 'sql'. This way, if it breaks again it will
  still call the logic within and emit E_NOTICE instead, which
  would help detect the issue (and still fallback to at least
  showing the queries). Unlike before this commit where it took
  quite some time to figure out why it wasn't working.

* The above fixes still weren't enough to get queries to show
  up in the Debug toolbar for me. Turns out, this was because
  my local setup (mediawiki-docker-dev) uses a master-replica
  set up. The setup doesn't use any custom LBFactory config,
  just plain $wgDBservers. The logic for turning these plain
  settings into LBFactory (in MWLBFactory.php) does kick in,
  and does run (unlike if I had custom wgLBFactoryConf).
  But, the DBO_DEBUG flag didn't make it through because of
  the += operator preferring any pre-existing value my setup
  has, which is just `DBO_DEFAULT`.
  Merging 'flags' keys seems unsafe in general, but adding
  DBO_DEBUG based on $wgDebugDumpSql seems innocent and doesn't
  affect other behaviour (it's a case of DWIM).

Bug: T231742
Change-Id: I122bb1a65620a7ae4e1943136c975b63524a5111
(cherry picked from commit 2e404a25828c713c0926b8f9e2003ad1e7dbe667)
includes/DefaultSettings.php
includes/db/MWLBFactory.php
includes/debug/MWDebug.php
includes/debug/logger/LegacyLogger.php
includes/libs/rdbms/database/Database.php