Page MenuHomePhabricator

Simplify WebClientError code
Closed, DeclinedPublic

Description

EventLogging provides a simple API to log events to a schema.
the existing usage of

mw.track( 'wikimedia.event.WebClientError' )

seems risky as it shows knowledge of EventLogging's internals. It also only works if WikimediaEvents is installed.

I propose we use mw.eventLog schema to do EventLogging for this schema going forward.

This will switch sampling from "per user" to "per page", but given we are only ever going to use this at 100% on the beta cluster that's not a problem.

Event Timeline

Jdlrobson created this task.Oct 4 2018, 8:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 4 2018, 8:01 PM

Change 464640 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Simplify WebClientError code

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

Jdlrobson triaged this task as Normal priority.Oct 4 2018, 11:02 PM
Jdlrobson updated the task description. (Show Details)

Change 464640 abandoned by Jdlrobson:
Simplify WebClientError code

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

Jdlrobson closed this task as Declined.Oct 5 2018, 7:21 PM
Jdlrobson added a subscriber: Krinkle.

Per @Krinkle "The Schema class will soon be deprecated. The abstraction has no added value."
No extra value here so let's not do this.

mw.track( 'wikimedia.event.WebClientError' ) seems risky as it shows knowledge of EventLogging's internals. It also only works if WikimediaEvents is installed.

This track API is designed with the opposite intent, to not require a dependency on EventLogging. Extensions call track(), and if EventLogging is installed, it itself will load the subscriber module and process these. The subscriber module should not be loaded explicitly.

Using mw.track( 'wikimedia.event.WebClientError' ) indeed means it only works when WikimediaEvents is installed. Based on this task and the above patch, I assume that was unintentional. Using mw.track( 'event.WebClientError' ) instead would resolve this issue, and is what most other extensions do as well.

I'm hoping to better document this as part of T187207. Your feedback would be appreciated on the draft at T205744: Deprecation Information for EventLogging ResourceLoader modules.

Krinkle updated the task description. (Show Details)Oct 10 2018, 7:03 PM