Page MenuHomePhabricator

performer struct fields NULL in event_sanitized.mediawiki_revision_tags_change
Open, Needs TriagePublicBUG REPORT

Description

Data Platform Engineering Bug Report or Data Problem Form.

Please fill out the following

What kind of problem are you reporting?

  • Access related problem
  • Service related problem
  • Data related problem
For a data related problem:
  • Is this a data quality issue? Yes.
  • What datasets and/or dashboards are affected? event_sanitized.mediawiki_revision_tags_change
  • What are the observed vs expected results? Please include information such as location of data, any initial assessments, sql statements, screenshots.

Querying data about tagged edits in event_sanitized.mediawiki_revision_tags_change results in the performer struct having no information about the user making the edit, the fields are instead NULL. When querying this data historically (see output below), it appears that some edits have always been this way but the proportion dramatically increased in October 2023 and now accounts for almost all edits. This problem is current and ongoing.

Expected results: the performer struct correctly reflects the information about the user who made the given edit.

Historical counts:

WITH tagged_edits AS (
    SELECT
        rev_id,
        FIRST_VALUE(substr(rev_timestamp, 1, 7)) AS log_month,
        MAX(IF(performer.user_id IS NULL, 1, 0)) AS had_null_performer
    FROM event_sanitized.mediawiki_revision_tags_change AS mert
    WHERE year = 2023
    GROUP BY rev_id
)
SELECT
    log_month,
    count(1) AS num_tagged_edits,
    SUM(had_null_performer) AS num_tag_edits_with_null_performer,
    SUM(had_null_performer) / count(1) AS prop_null
FROM tagged_edits
GROUP BY log_month

Output table from Pandas with non-2023 edits removed and sorted by log_month:

log_month	num_tagged_edits	num_tag_edits_with_null_performer	prop_null
2023-01	33133594	2264876	0.068356
2023-02	27028430	1939196	0.071747
2023-03	29962527	2158956	0.072055
2023-04	29423416	2021438	0.068702
2023-05	27712184	1997860	0.072093
2023-06	28597216	1989085	0.069555
2023-07	27921145	2111404	0.075620
2023-08	30272521	1993681	0.065858
2023-09	26427299	1999313	0.075653
2023-10	28243140	8352344	0.295730
2023-11	29146770	29146625	0.999995
2023-12	5696883	5696840	0.999992
For the DE Team to fill out
Which systems does this effect?
  • Hive
  • Druid
  • Superset
  • Turnilo
  • WikiDumps
  • Wikistats
  • Airflow
  • HDFS
  • Goblin
  • Scqoop
  • Dashiki
  • DataHub
  • Spark
  • Jupyter
  • Modern Event Platform
  • Event Logging
  • Other
Impact Assessment:

Does this problem qualify as an incident?

  • Yes
  • No

Does this violate an SLO?

  • Yes
  • No
Value CalculatorRank
Will this improve the efficiency of a teams workflow?1-3
Does this have an effect of our Core Metrics?1-3
Does this align with our strategic goals?1-3
Is this a blocker for another team?1-3

Event Timeline

Aklapper renamed this task from NEW BUG REPORT performer struct fields NULL in event_sanitized.mediawiki_revision_tags_change to performer struct fields NULL in event_sanitized.mediawiki_revision_tags_change.Dec 6 2023, 6:13 PM

As far as I can tell, EventBus just forwards whatever is given as the $user param of the ChangeTagsAfterUpdateTags.

If something has changed, it is likely to be in the MediaWiki hook.

Out of curiosity, and in hoping to make this easier to fix, I grabbed daily aggregates for October 2023. Looks like something was deployed the week of Oct 23 as that's when the proportion increases from the expected 6–7%:

log_date	num_tagged_edits	num_tag_edits_with_null_performer	prop_null
2023-10-23	985895	64943	0.065872
2023-10-24	961813	65499	0.068100
2023-10-25	1133200	275178	0.242833
2023-10-26	1118374	926144	0.828117
2023-10-27	1241023	1241020	0.999998
2023-10-28	1157623	1157620	0.999997
2023-10-29	1151495	1151490	0.999996

Including MW-Interfaces-Team who (I think) support hooks. Let me know if not.

The ChangeTagsAfterUpdateTags hook is fired by ChangeTagsStore:.updateTags():

	public function updateTags( $tagsToAdd, $tagsToRemove, &$rc_id = null,
		&$rev_id = null, &$log_id = null, $params = null, RecentChange $rc = null,
		UserIdentity $user = null
	)

As you can see, the $user parameter is optional, some callers just don't supply it. The documentation for the parameter says: Tagging user, in case the tagging is subsequent to the tagged action. I don't know what that means, exactly, but it's the same in the documentation of the hook itself: the parameter is optional.

Looking at callers, I find that ChangeTagStore::addTags() and ChangeTag::addTags() never supply a user, the $user parameter is just absent. They are used in quite a few places, including ApiTag. I suppose that's where the nulls come from. But I don't see what changed, it seems like these methods never supported passing a user. The last time the method sigature was touched was in 2014.

Maybe it used to fall back to the user from the main RequestContext? But then, why is the $user parameter in the hook signature declared as optional?

We'll take a look at this issue during our Offsite, week of March 18th.

The relevant code was moved from ChangeTags into ChangeTagStore by @Ladsgroup in June 2023, see https://gerrit.wikimedia.org/r/c/mediawiki/core/+/926469

But the old code was just:

		$services = MediaWikiServices::getInstance();
		$userObj = $user ? $services->getUserFactory()->newFromUserIdentity( $user ) : null;
		( new HookRunner( $services->getHookContainer() ) )->onChangeTagsAfterUpdateTags(
			$tagsToAdd, $tagsToRemove, $prevTags, $rc_id, $rev_id, $log_id, $params, $rc, $userObj );

That would also indicate that the user would be null if not provided.

Change 1011383 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/EventBus@master] EXPERIMENT: Add fallback to onChangeTagsAfterUpdateTags

https://gerrit.wikimedia.org/r/1011383

I can't fiogure out what triggered the change in behavior. I made an experimental patch that may address the issue. However, I am not sure it is semantically correct in al lcases.

FJoseph-WMF raised the priority of this task from Medium to Needs Triage.Mar 19 2024, 6:20 PM