Context
In T376266: Allow to assign an experiment variant to existing account holders, the Growth-Team switched to assigning A/B testing variants on the fly (via conditional defaults) based on the user ID, rather than by assigning variants. The relevant code is this:
public function shouldAssignBucket( UserIdentity $userIdentity, string $experimentName, array $args ): bool { $centralIdLookupCallback = $this->centralIdLookupCallback; /** @var CentralIdLookup */ $centralIdLookup = $centralIdLookupCallback(); $userCentralId = $centralIdLookup->centralIdFromLocalUser( $userIdentity ); $sample = $this->getSample( $userCentralId, $experimentName ); if ( $this->isInSample( $args[0], $sample ) ) { return true; } return false; }
This worked fine in practice. However, we also have code in onLocalUserCreated that reports number of users in each variant to Grafana. Those counts were not matching the reality, as discovered in T379682: Growth KPI Grafana dashboard claims no A/B testing happens at pilot wikis when using global assignment.
This was happening because the shouldAssignBucket() method was failing silently when executed from within onLocalUserCreated(): in there, CentralIdLookup is not guaranteed to work, because the central user is also created in onLocalUserCreated (by MediaWiki-extensions-CentralAuth), and it is not guaranteed that CentralAuth's handler runs before Growth's (or the other way around).
Problem
As far as I can tell, the only place where we can run code on account creation is onLocalUserCreated. However, the CentralAuth part is not guaranteed to be available at that point. Together with the "failing silently" part, this caused a lot of confusion.
I think there are aspects CentralAuth might consider doing in a more friendly way, such as:
- CentralIdLookup could fail if called on an user that does not have a central ID, rather than silently returning zero
- A hook to finalise account creation could be added, central account creation moved there
For now, Growth workarounded the problem by wrapping the monitoring call in a deferred update (so it runs at the end of the request, when the central user exists), but this is not really a nice solution. Curious to hear what CentralAuth maintainers think about this.