Page MenuHomePhabricator

Update event debug logging in EventLogging extension
Closed, ResolvedPublic5 Estimated Story Points

Description

With the deprecation of the old EventLogging pipeline, we need to update the code to remove dead debugging logic and do something that we like going forward. There's a discussion about this in slack, and this task can be used to continue it and record the outcomes.

(note, this task was re-purposed, see the history for the original attempt to clarify odd behavior in Chrome Dev tools and how it disconnected before pagehide fires. This updated task will still consider that and try to explain the problem to devs monitoring the console)

Relevant links:


In the Slack thread linked above, a handful of WMF SE's reached consensus on the next steps for the EventLogging debug modes:

  1. Remove the visual debug mode
  2. Remove eventlogging-display-web rows from the user_properties table
  3. Keep the console debug mode
  4. Allow all users to enable the console debug mode
    • If the user is logged in, then set a user preference; otherwise, set a value in sessionStorage
  5. Allow all users to enable the console debug mode for one or more streams

I propose the following API to enable users to manage their preferences:

namespace mw {
  interface eventLog {
  
    /**
     * Enable debugging for all streams or the specified stream.
     * 
     * @remarks
     *
     * When debugging is enabled, the event is logged to the console
     * immediately after it is enqueued for submission.
     *
     * If `persist` is truthy and you are logged-in, then debugging will
     * persist across browsing sessions; otherwise debugging will only be
     * enabled for the duration of the browsing session.
     * 
     * @param streamName
     */
    enableDebugging( streamName: ?string, persist: ?boolean );

    /**
     * Disable debugging for all streams or the specified stream.
     *
     * @see {@link mw.eventLog.enableDebugging}
     */
    disableDebugging( streamName: ?string, persist: ?boolean );
  }
}

Details

Related Changes in Gerrit:
Related Changes in GitLab:
TitleReferenceAuthorSource BranchDest Branch
Shows a warning message when stream is not registered or is out of samplerepos/data-engineering/metrics-platform!111sfacicheck-out-of-sample-streamsmain
Customize query in GitLab

Event Timeline

Milimetric claimed this task.
Milimetric triaged this task as Low priority.
Milimetric set the point value for this task to 2.

People have shared opinions, so we are processing and will continue to be in review and conversation for a couple weeks.

Milimetric renamed this task from Notify developers that pagehide event handlers might not be logged in the console to Update event debug logging in EventLogging extension.Feb 10 2025, 5:00 PM
Milimetric updated the task description. (Show Details)

Change #1116449 had a related patch set uploaded (by Milimetric; author: Milimetric):

[mediawiki/extensions/EventLogging@master] Improve EventLogging's debugging tools

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

phuedx raised the priority of this task from Low to Needs Triage.

Change #1131644 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/EventLogging@master] Remove visual debugging mode

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

Change #1131644 merged by jenkins-bot:

[mediawiki/extensions/EventLogging@master] Remove visual debugging mode

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

Following the merge and deployment of Remove visual debugging mode, we should write a query/maintenance script to unset the eventlogging-display-web user preference.

delete
  from user_properties
 where up_property = 'eventlogging-display-web';

There are only a handful of folks that set this.

phuedx triaged this task as High priority.Sep 12 2025, 12:14 PM
phuedx moved this task from Incoming to READY TO GROOM on the Test Kitchen board.

Being bold. This is related to OKR work.

Change #1116449 abandoned by Milimetric:

[mediawiki/extensions/EventLogging@master] Improve EventLogging's debugging tools

Reason:

superseded by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventLogging/+/1131644

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

JVanderhoop-WMF changed the point value for this task from 2 to 5.
JVanderhoop-WMF subscribed.

Definition of Done here:

  • Legacy EventLogging has a visual debug mode that is very rarely used. We killed it but we didn't tell anyone (including ourselves!). Tell people we killed it.
  • Replace all instances of mw.log() with console.log()
  • We don't log if a stream is out of sample, so we silently lose events. TK SDK should alert to say "hey this is overridden, we won't send an event" or "hey this config variable might be wrong" -- we want to tell people why events are not being sent. EventLogging just quietly discards. We should tell people when a stream is out of sample.

Two points remaining from here

Definition of Done here:

  • Legacy EventLogging has a visual debug mode that is very rarely used. We killed it but we didn't tell anyone (including ourselves!). Tell people we killed it.

. . . .

Regarding that first item I would also add "Update related documentation". I have just found https://www.mediawiki.org/wiki/Extension:EventLogging/Programming#Monitoring_events where eventlogging-display-web property is mentioned as a way to activate the visual debug mode that was already removed.

Definition of Done here:
. . .

  • Replace all instances of mw.log() with console.log()

. . .

@phuedx @Milimetric Can I assume that the replacement is the following?

  • mw.log() -> console.log
  • mw.log.warn()-> console.warn()
  • mw.log.deprecate() -> console.warn()
  • mw.log.error() -> console.error()

@phuedx @Milimetric Can I assume that the replacement is the following?

  • mw.log() -> console.log
  • mw.log.warn()-> console.warn()
  • mw.log.deprecate() -> console.warn()
  • mw.log.error() -> console.error()

Only mw.log() needs to be replaced. mw.log.warn() and .error() call the equivalent methods on the console object. We shouldn't replace mw.log.deprecate() as it's for marking methods as deprecated / tracking deprecations via a dashboard.

@phuedx @Milimetric Can I assume that the replacement is the following?

  • mw.log() -> console.log
  • mw.log.warn()-> console.warn()
  • mw.log.deprecate() -> console.warn()
  • mw.log.error() -> console.error()

Only mw.log() needs to be replaced. mw.log.warn() and .error() call the equivalent methods on the console object. We shouldn't replace mw.log.deprecate() as it's for marking methods as deprecated / tracking deprecations via a dashboard.

Understood! Thanks!
So there is nothing to replace then, no usages of mw.log() remains in EventLogging (and the last one we found in MetricsPlatform extension was already replaced via https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MetricsPlatform/+/1204251/11/modules/ext.xLab/index.js and there is none in the JS client library

Regarding the last definition of done a few comments above:

Definition of Done here:

  • Legacy EventLogging has a visual debug mode that is very rarely used. We killed it but we didn't tell anyone (including ourselves!). Tell people we killed it.
  • Replace all instances of mw.log() with console.log()
  • We don't log if a stream is out of sample, so we silently lose events. TK SDK should alert to say "hey this is overridden, we won't send an event" or "hey this config variable might be wrong" -- we want to tell people why events are not being sent. EventLogging just quietly discards. We should tell people when a stream is out of sample.

Two points remaining from here

The following has been done already:

  • Replace all instances of mw.log() with console.log()

After taking a look, we can confirm that there are no usages of mw.log() in EventLogging at this time. We don't need to do anything about this

  • We don't log if a stream is out of sample, so we silently lose events. TK SDK should alert to say "hey this is overridden, we won't send an event" or "hey this config variable might be wrong" -- we want to tell people why events are not being sent. EventLogging just quietly discards. We should tell people when a stream is out of sample.

A patch have been pushed for JS client library (https://gitlab.wikimedia.org/repos/data-engineering/metrics-platform/-/merge_requests/111) to show a warning console message when an event is sent for a stream that is not configured and also when the stream is out of sample. Once the change is merge we will have to deploy a new version of the JS client library to make this change a reality for EventLogging

And regarding

  • Legacy EventLogging has a visual debug mode that is very rarely used. We killed it but we didn't tell anyone (including ourselves!). Tell people we killed it.

After reading the thread that causes this ticket, I have seen that some approaches were discussed to work on something to have a way to debug for logged-out users. As far as I understand, that thing wasn't done. For example, there is no a new way that allows developers to debug when logged-out but something about that was mentioned. I haven't found any ticket about it. Should we file a new one about that to work on it later? I would like to have this clear, just in case I'm missing something, before announcing the related work that was done. Maybe @phuedx @Milimetric you can provide clarity

After reading the thread that causes this ticket, I have seen that some approaches were discussed to work on something to have a way to debug for logged-out users. As far as I understand, that thing wasn't done. For example, there is no a new way that allows developers to debug when logged-out but something about that was mentioned. I haven't found any ticket about it. Should we file a new one about that to work on it later? I would like to have this clear, just in case I'm missing something, before announcing the related work that was done. Maybe @phuedx @Milimetric you can provide clarity

I think our latest thinking around EventLogging is to divorce TK from it completely. It's still ours to maintain, but ideally nobody should really use it, they should migrate to TK. If they have a compelling reason they prefer EventLogging, we should implement something equivalent in TK and still aim to kill EventLogging.

If everyone agrees with those aesthetics, then the logical course of action is to not implement any kind of debugging of EventLogging. Just delete all of the UI or console based debugging and focus only on TK debugging.