Impact module: Instrument loading times
Closed, ResolvedPublic
Actions

Description

Description
We have a dashboard that provides us information about loading times for the suggested edits module on Special:Homepage.

We will want to implement something similar for the Impact module, so that we can 1) understand the overall load time for desktop/mobile users per wiki and 2) catch performance related regressions after we've deployed the new module to production.

Acceptance Criteria

Should be able to see time-to-interactive for the Impact module on a Grafana chart on the Special:Homepage grafana dashboard
...

Completion checklist

Functionality

The patches have been code reviewed and merged
The task passes its acceptance criteria

Engineering

There are existing and passing unit/integration tests
Tests for every involved patch should pass
Coverage for every involved project should have improved or stayed the same

Design & QA

If the task is UX/Design related: it must be reviewed and approved by the UX/Design team
Must be reviewed and approved by Quality Assurance.

Documentation

Related and updated documentation done where necessary
- Internal technical changes: internal repository documentation must be updated (README.md, JSDoc, PHPDoc)
- Infrastructure technical changes: technical changes that reflect on environment, infrastructure, endpoints or any other area of interest for technical contributors should be reflected on Extension:GrowthExperiments or Extension:GrowthExperiments/Technical documentation pages.

Details

Subject	Repo	Branch	Lines +/-
NewImpact: Instrument app loading	mediawiki/extensions/GrowthExperiments	master	+13 -2
UserImpactHandler: Instrument cache hit and misses	mediawiki/extensions/GrowthExperiments	master	+9 -0
UserImpactLookup: Instrument getExpensiveUserImpact	mediawiki/extensions/GrowthExperiments	master	+15 -3
UserImpactHandler: Instrument API response time	mediawiki/extensions/GrowthExperiments	master	+12 -2

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	KStoller-WMF	T248330 [EPIC] Growth: Positive reinforcement
Resolved	KStoller-WMF	T222310 [EPIC] Positive reinforcement: New Impact Module
Resolved	kostajh	T310779 Impact module: Instrument loading times
Resolved	kostajh	T324198 Special:Homepage: Add instrumentation for monitoring transfer size and firstPaint

Event Timeline

kostajh created this task.Jun 16 2022, 9:58 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 16 2022, 9:58 AM

kostajh renamed this task from Instrument impact module loading to Impact module: Instrument loading times.Jun 16 2022, 9:59 AM

kostajh updated the task description. (Show Details)Jun 16 2022, 10:02 AM

MShilova_WMF edited projects, added Growth-Team; removed Growth-Team (Sprint 0 (Growth Team)).Jul 21 2022, 4:03 PM

MShilova_WMF moved this task from Inbox to Upcoming Work on the Growth-Team board.

KStoller-WMF triaged this task as Medium priority.Oct 16 2022, 7:47 PM

KStoller-WMF edited projects, added Growth-Team (Sprint 0 (Growth Team)); removed Growth-Team.

KStoller-WMF moved this task from Incoming to Ready for Development on the Growth-Team (Sprint 0 (Growth Team)) board.

We can also instrument UserImpactHandler for measuring API response times and UserImpactCompute service (for calculating impact data)

Change 856010 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] UserImpactLookup: Instrument getExpensiveUserImpact

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

gerritbot added a project: Patch-For-Review.Nov 11 2022, 7:40 PM

Change 856011 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] UserImpactHandler: Instrument API response time

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

Change 856011 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] UserImpactHandler: Instrument API response time

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.10; 2022-11-14).Nov 15 2022, 1:01 AM

Change 856010 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] UserImpactLookup: Instrument getExpensiveUserImpact

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

Maintenance_bot removed a project: Patch-For-Review.Nov 15 2022, 1:30 AM

Change 858456 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] WIP: NewImpact: Instrument app loading

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

gerritbot added a project: Patch-For-Review.Nov 18 2022, 7:44 AM

MShilova_WMF mentioned this in T222310: [EPIC] Positive reinforcement: New Impact Module.Nov 18 2022, 3:10 PM

Things other than loading times that might be worth keeping track of:

Cache hit ratio - it's reflected in UserImpactHandler.run but still might help with diagnostics to track directly.
Maybe how often we run into the ping limiter in UserImpactHandler?
Job queue characteristics like average wait time? (Probably exists somewhere, just need to find it.) One potential performance complication could be when the job queue gets clogged up (I don't think we use a dedicated queue, maybe we should? not sure about the policies around that) so RefreshUserImpactJob writes get delayed. (Alternatively, maybe the refresh in the API handler should be a deferred.)
Reliability of PageViewInfo (which sadly does not have any monitoring of its own) - how often do we get truncated pageview data because of quota overruns?

Change 860016 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] UserImpactHandler: Instrument cache hit and misses

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

In T310779#8413743, @Tgr wrote:

Things other than loading times that might be worth keeping track of:

Cache hit ratio - it's reflected in UserImpactHandler.run but still might help with diagnostics to track directly.

Maybe how often we run into the ping limiter in UserImpactHandler?

Those should be accounted for in https://gerrit.wikimedia.org/r/860016.

Job queue characteristics like average wait time? (Probably exists somewhere, just need to find it.) One potential performance complication could be when the job queue gets clogged up (I don't think we use a dedicated queue, maybe we should? not sure about the policies around that) so RefreshUserImpactJob writes get delayed.

I think we have that data here? https://grafana.wikimedia.org/d/LSeAShkGz/jobqueue?orgId=1

(Alternatively, maybe the refresh in the API handler should be a deferred.)

That's probably a good idea, though, it used to be the case that we were not supposed to do writes in a GET request, even in a deferred, due to the multi-DC work. I think that changed, though?

Reliability of PageViewInfo (which sadly does not have any monitoring of its own) - how often do we get truncated pageview data because of quota overruns?

Maybe we should add that in PageViewInfo? In either case I think that should be a separate task, if we want to instrument this.

kostajh changed the task status from Open to In Progress.Nov 23 2022, 1:40 PM

kostajh moved this task from In Progress to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.

In T310779#8417101, @kostajh wrote:

Those should be accounted for in https://gerrit.wikimedia.org/r/860016.

Thanks!

I think we have that data here? https://grafana.wikimedia.org/d/LSeAShkGz/jobqueue?orgId=1

Thanks for finding. Might be worth duplicating some of it IMO because individual Grafana metrics cannot be deeplinked and manually selecting the right job out of dozens is very time-consuming.

That's probably a good idea, though, it used to be the case that we were not supposed to do writes in a GET request, even in a deferred, due to the multi-DC work. I think that changed, though?

I think it's not strictly forbidden but discouraged, as deferreds are less time-sensitive but you still end up with a cross-datacenter DB connection. Plus we'd have to silence the logspam somehow.

Maybe we should add that in PageViewInfo?

It's hard to tell at that level if it's a problem. E.g. the action API wrapper around it will just stop when it encounters the first title with no data, and the client can use standard API continuation to get more data, so with most MediaWiki-specific API libraries the developer doesn't even need to pay extra attention and will get all the data eventually.

Change 860016 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] UserImpactHandler: Instrument cache hit and misses

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

ReleaseTaggerBot edited projects, added MW-1.40-notes (1.40.0-wmf.12; 2022-11-28); removed MW-1.40-notes (1.40.0-wmf.10; 2022-11-14).Nov 24 2022, 6:01 AM

In T310779#8419001, @Tgr wrote:

That's probably a good idea, though, it used to be the case that we were not supposed to do writes in a GET request, even in a deferred, due to the multi-DC work. I think that changed, though?

I think it's not strictly forbidden but discouraged, as deferreds are less time-sensitive but you still end up with a cross-datacenter DB connection. Plus we'd have to silence the logspam somehow.

I filed T323747: NewImpact: Use POST request for fetching impact data for this.

Change 858456 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] NewImpact: Instrument app loading

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

In T310779#8419001, @Tgr wrote:

In T310779#8417101, @kostajh wrote:

Those should be accounted for in https://gerrit.wikimedia.org/r/860016.

Thanks!

I think we have that data here? https://grafana.wikimedia.org/d/LSeAShkGz/jobqueue?orgId=1

Thanks for finding. Might be worth duplicating some of it IMO because individual Grafana metrics cannot be deeplinked and manually selecting the right job out of dozens is very time-consuming.

That's probably a good idea, though, it used to be the case that we were not supposed to do writes in a GET request, even in a deferred, due to the multi-DC work. I think that changed, though?

I think it's not strictly forbidden but discouraged, as deferreds are less time-sensitive but you still end up with a cross-datacenter DB connection. Plus we'd have to silence the logspam somehow.

Maybe we should add that in PageViewInfo?

It's hard to tell at that level if it's a problem. E.g. the action API wrapper around it will just stop when it encounters the first title with no data, and the client can use standard API continuation to get more data, so with most MediaWiki-specific API libraries the developer doesn't even need to pay extra attention and will get all the data eventually.