⚓ T357050 editResponseTime's port to statslib is not actually backwards-compatible
Page MenuHomePhabricator
Create Task
Maniphest T357050

editResponseTime's port to statslib is not actually backwards-compatible
Closed, ResolvedPublic
Actions

Description

Pretty exactly synchronized with the rollout of wmf.17 to group1 -- and then with its rollback later -- we saw a decrease in the graphite metric MediaWiki.timing.editResponseTime.sample_rate.
https://grafana.wikimedia.org/d/O_OXJyTVk/home-w-wiki-status?orgId=1&from=1707408780000&to=1707419730000&viewPanel=10

This metric is used on the Edit Count dashboard, on the front page of grafana showing 5 high-level indicators of site functioning status, and is also displayed on our public site status page https://www.wikimediastatus.net.

So we should probably fix whatever issue with the transition patch causes it to not be backwards-compatible by the current clients.

Details

SubjectRepoBranchLines +/-
Stats: Amend tests to validate changing statsdDataFactorymediawiki/coremaster+17 -16
Migrate `editResponseTime` metric to Prometheus storemediawiki/extensions/WikimediaEventsmaster+37 -18
Stats: Ensure statsdDataFactory follows when setting componentmediawiki/coremaster+42 -2
Revert "Migrate `editResponseTime` metric to Prometheus store"mediawiki/extensions/WikimediaEventsmaster+11 -30
Revert "Migrate `editResponseTime` metric to Prometheus store"mediawiki/extensions/WikimediaEventswmf/1.42.0-wmf.17+11 -30
Customize query in gerrit

Related Objects

Event Timeline

CDanis triaged this task as High priority.Feb 8 2024, 7:03 PM
CDanis created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 8 2024, 7:03 PM
CDanis added a parent task: T354435: 1.42.0-wmf.17 deployment blockers.Feb 8 2024, 7:03 PM
brennen raised the priority of this task from High to Unbreak Now!.Feb 8 2024, 7:05 PM
brennen subscribed.

Raising to UBN per train blocker policy.

colewhite changed the task status from Open to In Progress.Feb 8 2024, 7:11 PM
colewhite claimed this task.
Reedy added a project: MediaWiki-extensions-WikimediaEvents.Feb 8 2024, 7:19 PM
Jdforrester-WMF subscribed.Feb 8 2024, 7:23 PM

My understanding is that sample_rate is generated by statsd rather than our direct code, so I don't understand how we could provide back-compat. with this migration; we could shift the monitoring to a different metric that is emitted in both, perhaps? But that's then measuring something else.

colewhite added a comment.EditedFeb 8 2024, 8:22 PM

I am unable to replicate this behavior in phpunit tests. It seems something more than a simple bug is occurring. I recommend we revert the transition patch to unblock the train and continue investigating.

	public function testCopyToStatsdAtArray() {
		$m = new StatsFactory( new StatsCache, new NullEmitter, new NullLogger );
		$statsdMock = $this->createMock( IBufferingStatsdDataFactory::class );

		$statsdMock->expects( $this->exactly( 2 ) )
			->method( 'timing' )
			->withConsecutive( [ 'test.timing.1', 1.0 ], [ 'test.timing.2', 1.0 ] );

		$m->withComponent('WikiMediaEvents')
			->withStatsdDataFactory( $statsdMock )
			->getTiming( 'testMetricTiming' )
			->copyToStatsdAt( ['test.timing.1', 'test.timing.2'] )
			->observe( 1 );
	}
gerritbot added a comment.Feb 8 2024, 8:36 PM

Change 998971 had a related patch set uploaded (by Brennen Bearnes; author: Brennen Bearnes):

[mediawiki/extensions/WikimediaEvents@master] Revert "Migrate `editResponseTime` metric to Prometheus store"

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

gerritbot added a project: Patch-For-Review.Feb 8 2024, 8:36 PM
brennen added a comment.Feb 8 2024, 8:36 PM

Created revert patch, please check me on that...

gerritbot added a comment.Feb 8 2024, 8:41 PM

Change 998972 had a related patch set uploaded (by Brennen Bearnes; author: Brennen Bearnes):

[mediawiki/extensions/WikimediaEvents@wmf/1.42.0-wmf.17] Revert "Migrate `editResponseTime` metric to Prometheus store"

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

gerritbot added a comment.Feb 8 2024, 8:45 PM

Change 998972 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@wmf/1.42.0-wmf.17] Revert "Migrate `editResponseTime` metric to Prometheus store"

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

Stashbot added a comment.Feb 8 2024, 8:45 PM

Mentioned in SAL (#wikimedia-operations) [2024-02-08T20:45:58Z] <brennen@deploy2002> Started scap: Backport for [[gerrit:998972|Revert "Migrate editResponseTime metric to Prometheus store" (T357050)]]

Stashbot added a comment.Feb 8 2024, 8:47 PM

Mentioned in SAL (#wikimedia-operations) [2024-02-08T20:47:30Z] <brennen@deploy2002> brennen: Backport for [[gerrit:998972|Revert "Migrate editResponseTime metric to Prometheus store" (T357050)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Stashbot added a comment.Feb 8 2024, 8:55 PM

Mentioned in SAL (#wikimedia-operations) [2024-02-08T20:55:15Z] <brennen@deploy2002> Finished scap: Backport for [[gerrit:998972|Revert "Migrate editResponseTime metric to Prometheus store" (T357050)]] (duration: 09m 17s)

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.17; 2024-02-06).Feb 8 2024, 9:00 PM
gerritbot added a comment.Feb 8 2024, 9:03 PM

Change 998971 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] Revert "Migrate `editResponseTime` metric to Prometheus store"

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

brennen lowered the priority of this task from Unbreak Now! to Needs Triage.Feb 8 2024, 9:14 PM
brennen removed a parent task: T354435: 1.42.0-wmf.17 deployment blockers.
Maintenance_bot removed a project: Patch-For-Review.Feb 8 2024, 9:30 PM
ReleaseTaggerBot edited projects, added MW-1.42-notes (1.42.0-wmf.18; 2024-02-13); removed MW-1.42-notes (1.42.0-wmf.17; 2024-02-06).Feb 8 2024, 10:00 PM
gerritbot added a comment.Feb 8 2024, 10:30 PM

Change 999012 had a related patch set uploaded (by Cwhite; author: Cwhite):

[mediawiki/core@master] stats: ensure statsdDataFactory follows when setting component

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

gerritbot added a project: Patch-For-Review.Feb 8 2024, 10:30 PM
colewhite triaged this task as High priority.EditedFeb 8 2024, 10:42 PM

Reproducing this issue locally was complicated because DeveloperSettings defaults $wgForceDeferredUpdatesPreSend = true;. After setting this to false, I was able to reproduce the problem locally.

The issue was that StatsFactory did not populate the $statsdDataFactory of the new StatsFactory instance returned from withComponent().

This was the first place we have tried using withComponent() and the bug appears limited to usage of that feature. Also affects wikibase.quality.constraints.* (T354909) - we'll probably want a backport

colewhite edited projects, added MediaWiki-libs-Stats; removed MediaWiki-extensions-WikimediaEvents.Feb 8 2024, 10:42 PM
gerritbot added a comment.Feb 8 2024, 10:48 PM

Change 998973 had a related patch set uploaded (by Cwhite; author: Cwhite):

[mediawiki/extensions/WikimediaEvents@master] Migrate `editResponseTime` metric to Prometheus store

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

gerritbot added a comment.Feb 9 2024, 6:38 PM

Change 999012 merged by jenkins-bot:

[mediawiki/core@master] Stats: Ensure statsdDataFactory follows when setting component

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

gerritbot added a comment.Feb 9 2024, 6:45 PM

Change 999022 had a related patch set uploaded (by Cwhite; author: Cwhite):

[mediawiki/core@master] stats: amend tests to validate changing statsdDataFactory

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

gerritbot added a comment.Feb 9 2024, 9:36 PM

Change 998973 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] Migrate `editResponseTime` metric to Prometheus store

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

gerritbot added a comment.Feb 12 2024, 10:48 AM

Change 999022 merged by jenkins-bot:

[mediawiki/core@master] Stats: Amend tests to validate changing statsdDataFactory

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

Maintenance_bot removed a project: Patch-For-Review.Feb 12 2024, 11:30 AM
colewhite closed this task as Resolved.Mar 1 2024, 5:40 PM

Fix was deployed on 2/15 and the graphs still look ok. Considering this resolved!

DAlangi_WMF awarded a token.Mar 1 2024, 5:47 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits