Page MenuHomePhabricator

Decide how to deploy the AuthManagerStatsdHandler class
Open, Needs TriagePublic

Description

AuthManagerStatsdHandler is in WikimediaEvents, which is now not loaded on closed wikis, resulting in a missing class error. A quick & dirty fix is in https://gerrit.wikimedia.org/r/#/c/342778/. For a proper fix, the options that have been raised and their drawbacks:

  • disable AuthManager stats on closed wikis (ie. just keep the current fix). These are SUL wikis so they might still be abused in ways that the auth stats are intended to monitor (e.g. spambot registration).
  • re-enable WikimediaEvents on all wikis, use some config flag to disable the rest of its features. The main reason for disabling it was to not load unnecessary JS; just defining a ResourceLoader module results in JS being loaded (not the module itself, but the module definition); conditionally defining the module would require moving the module definition from JSON to PHP, which is the opposite of how we want our configuration in general.
  • move AuthManagerStatsdHandler to operations/mediawiki-config. That repo is not well prepared for holding classes (although it does contain a few): there are limitations to testing (both in CI and locally), the code update workflow becomes more awkward, and code outside the mediawiki/ repo tree is easier to miss when doing deprecations.
  • move the class to another extension, possibly a new one. The other existing WM extensions (WikimediaMessages and WikimediaMaintenance) don't really make sense, and creating a new extension is a lot of overhead.

Event Timeline

These are SUL wikis so they might still be abused in ways that the auth stats are intended to monitor (e.g. spambot registration).

On second thought I am not so sure about that. Are those wikis locked via $wgReadOnly? That should prevent registration and even login unless a local account already exists (which seems OK to ignore).

@Tgr They are not read-only from a database perspective (certain global users like stewards can still log-in and make edits). However auto-create does not happen in "closed" wikis. I've noticed this, for example, when running a maintenance script or bot over a wiki that has not previously been run on the wiki before it closed. It'll fail because the local account fails to auto-create.

@Tgr Perhaps these stats could be emitted from core directly? It's not entirely clear to me why we wouldn't want them by default.

The only part of the logger-to-statsd mapping that seems WMF-specific is checking of wikiid === loginwiki. I don't know if there's an existing primitive in AuthManager that relates to a "central" wiki that CentralAuth could map to wgCentralAuthLoginWiki.

Alternatively, rather than emitting from core/AuthManager, it could be a feature of CentralAuth's implementation of the relevant classes and emit the stats call from there, that would allow it to support wgCentralAuthLoginWiki directly.

EDIT: This relates to T157756, although my suggestion is to record the statistic explicitly instead of a generic means of counting log messages.

Krinkle renamed this task from Class undefined: AuthManagerStatsdHandler in /srv/mediawiki/php-1.29.0-wmf.15/includes/libs/ObjectFactory.php on line 71 to Decide how to deploy the AuthManagerStatsdHandler class.Sep 14 2018, 6:18 PM
Krinkle changed Risk Rating from N/A to default.