Ping @phuedx

We can probably decouple them so that they can be used on third parties.

Sure. Use mw.track (along with mw.experiments for bucketing) in favour of ext.eventLogging.Schema.

I've updated the description and pinged @phuedx on irc to flesh out how to approach this.

There should be a third party-support tag /cc @Aklapper

[offtopic]
@phuedx: https://www.mediawiki.org/wiki/Phabricator/Creating_and_renaming_projects

Proposal from @phuedx sounds good to me. 3 points?

Sorry, I should've been clearer in T158999#3057921):

In the context of Popups (and other Reading Web maintained simple extensions, e.g. RelatedArticles) mw.eventLog.Schema#log can be replaced by something like the following.

As @bmansurov remarked, this approach comes at the cost of duplicating the sampling logic (ish) in the Schema class but it:

• Doesn't require you to check whether EventLogging extension is loaded, if it is, then [the event. topic subscriber is guaranteed to be loaded](https://github.com/wikimedia/mediawiki-extensions-EventLogging/blob/master/includes/EventLoggingHooks.php#L37-L43).
• Doesn't require your RL modules to depend on EventLogging RL modules.

/**
 * Makes a weighted choice between `true` and `false`.
 *
 * @param {Number} trueWeight The weight for the `true` choice, expressed as a number between 0 and 1. The weight for the `false` is defined as `1 - trueWeight`
 * @param {String} token
 */
function weightedBoolean( trueWeight, token ) {
  return mw.experiments.getBucket( {
    name: 'weightedBoolean',
    enabled: true,
    buckets: {
      A: trueWeight,
      control: 1 - trueWeight,
    }
  }, token ) === 'A';
}

/**
 * Creates a logger for the schema, given a sampling rate and a token unique to the user's session. If the user isn't in the sample, then the logger will NOOP.
 *
 * @param {String} schemaName
 * @param {Number} samplingRate The sampling rate expressed a number between 0 and 1.
 * @param {String} token A token unique to the user's current session, e.g. mw.user.sessionId()
 */
function createLogger( schemaName, samplingRate, token ) {
  var topic;

  if ( !weightedBoolean( samplingRate, token ) ) {
    // return $.noop();
    return function () {};
  }

  topic = 'event.' + schemaName;

  return function ( data ) {
    mw.track( topic, data );
  }
}

Thanks @phuedx 👏

Change 343127 had a related patch set uploaded (by Jdlrobson):
[mediawiki/extensions/Popups] Popups doesnt need to depend on EventLogging

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

I might be misunderstanding something but I can't see why we need to make use of mw.experiments and mw.track. The only thing that breaks when EventLogging is absent is the logging of events. If it's not there.. why do we need to log anything? So my current version is far simpler and just uses NOPs. Let me know if I've missed the point here... :)

So re-reading after some helpful feedback from @Krinkle I think the motivation for the suggested solution was to avoid using mw.loader and avoid having to add any resource loader dependencies?

TBH the reliance of using mw.track based on the knowledge that any topics prefixed with 'event.' will be logged using EventLogging seems a little too magical for my liking and I'd prefer to be explicit.
I've amended the patch and I add a mw.loader.using call to the startup script.

In T158999#3108137, @Jdlrobson wrote:

So my current version is far simpler and just uses NOPs.

"Just" trigger word detected. I realize that you comment was written a few patch sets ago but the current version also:

• Uses mw.loader to load the ext.eventLogging.Schema RL module.
• Tests for the existence of the mw.eventLog.Schema constructor function inside the createSchema factory function.

In T158999#3108620, @Jdlrobson wrote:

TBH the reliance of using mw.track based on the knowledge that any topics prefixed with 'event.' will be logged using EventLogging seems a little too magical for my liking and I'd prefer to be explicit.
I've amended the patch and I add a mw.loader.using call to the startup script.

Firstly, thanks for being proactive by uploading your change and seeking outside review – thanks, as always, to @Krinkle! I hope the following doesn't sound too condescending and helps to clarify my position:

I can sympathize with your preference to invoke function directly (to be explicit) rather than use pub-sub (mw.track). However, the use of the latter does have its advantages and it certainly isn't magical.

When you say "any topics prefixed with 'event.' will be logged...", you're describing the protocol defined by the WikimediaEvents extension. While the protocol isn't as rigorously defined as, say, HTTP, it's still well defined and we can rely on it – we do in other extensions that we maintain and we rely on another similar protocol in the Popups extension already (see rEPOP9a94300858d8: Log events to statsv for monitoring PagePreviews performance). Protocols only define standard requests and responses for a client and a service, e.g. a request that the PP instrumentation can make of the EventLogging extension. Protocols don't define how those requests and responses are made (see https://en.wikipedia.org/wiki/IP_over_Avian_Carriers for a rather extreme example). Therefore, the client and the service are bound to the protocol but not to each other's implementations.

APIs define ways in which a client can consume the service and bind the two together. A relevant, concrete example of this is following line in PS5 of rEPOP1455ea2789ac: Popups doesnt need to depend on EventLogging:

mw.eventLog && mw.eventLog.Schema ? new mw.eventLog.Schema( /* ... */ ) : nullLogger; // The nullLogger bit is mine, I'll admit...

PP remains very tightly coupled to the EventLogging implementation despite not explicitly depending on it.

This task is about decoupling the Popups and EventLogging extensions – allowing one to change independently from the other – which, given the above, is why I favor the protocol over the API.

My new patch folds in concerns from Baha and Sam. The coupling was not right.
After understanding registerChangeListener a little more it seemed obvious that we could just load this conditionally without delaying the loading of Popups code and without having to touch the contract of Schema - i.e. only load and run the schema code when you need to.

This avoids hacky behaviour, keeps the approach explicit but untangles the dependency.

Of course, this doesn't recover the 3kb that Sam pointed out we could save by migrating away from EventLogging - but if we really care about that we should look at why EventLogging JS is so big in the first place and rectify that issue.

I have a few minor nitpicks with rEPOP8b3be51bc9b2: Popups doesnt need to depend on EventLogging's commit message but that's all.

Commit message has now been updated :)

Change 343127 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Popups doesn't need to depend on EventLogging

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

This is a technical task so over to you @bmansurov!