Page MenuHomePhabricator

Remove non-VirtualPageView EventLogging code from the Page Previews codebase
Closed, ResolvedPublic3 Estimated Story Points

Description

Ronseal. This will shave at least 1kb of JS off the Popups bundle.


The Page Previews codebase includes instruments and associated code for:

The instrument for the latter has long since been disabled but the associated code is still present in the build product that we ship to the user on very nearly all of mainspace pageviews on the Wikipedias.

Acceptance Criteria

  • The Popups schema instrument is removed
  • Associated code (e.g. configuration variable reading) is removed
  • The documentation is updated
  • The production configuration is updated
  • The VirtualPageView and ReferencePreviewsPopups instruments MUST still work

Developer notes

Quick analysis, suggests the code is not being utilised and nicely organized: pageviews.js takes care of the VirtualPageView and is completely separate from statsv (For performance timing metrics) . eventLogging.js is the one we want to remove.

When implementing this, we'll need to work out if changes can also be made to changeListeners/syncUserSettings.js which seems to support the event logging.

POC: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/655953

Event Timeline

Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.
ovasileva lowered the priority of this task from High to Medium.Jan 13 2021, 6:28 PM

Change 655953 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] POC: Remove instrumentation from page previews

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

Change 655953 abandoned by Jdlrobson:
[mediawiki/extensions/Popups@master] POC: Remove instrumentation from page previews

Reason:

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

Change 667897 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/Popups@master] WIP: Remove Popups instrumentation

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

Bumping to backlog. We'll focus on this as part of T284095 .

@awight: I've +1'd https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/667897/. I'd really appreciate it if you could confirm that the ReferencePreviewsPopups instrumentation is still works as expected.

Lowering priority compared to other things in flight. Will merge once we've got an OKAY from a Wikimedia Deutschland representative.

@awight: I've +1'd https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Popups/+/667897/. I'd really appreciate it if you could confirm that the ReferencePreviewsPopups instrumentation is still works as expected.

Thanks for the consideration! I've confirmed that WMDE's metrics are still emitted for Reference Previews popups, with your patch in place.

Jdlrobson claimed this task.
Jdlrobson moved this task from Code Review to QA on the Web-Team-Backlog (Kanbanana-FY-2020-21) board.

As part of QA we need to verify that VirtualPageView logging is still working as expected. Will think about how to do this around Tuesday 3rd next week. Ping me Edward if you get to this sooner.

Change 667897 merged by jenkins-bot:

[mediawiki/extensions/Popups@master] Remove Popups instrumentation

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

@Jdlrobson ...Ping! need some steps/direction on how to test this one.

@Edtadros I can QA this one.

We're basically going to look at this graph later today and make sure it doesn't do anything crazy compared to normal:
https://grafana.wikimedia.org/d/000000566/overview?viewPanel=6&orgId=1&from=now-30d&to=now

This looks fine to me. Nothing unusual in our graphs.

Data collection seems to have stopped around the 5th/6th August which coincides with the above change.