Page MenuHomePhabricator

Newcomer tasks: code for instrumentation
Closed, ResolvedPublic

Description

This task is for adding instrumentation to the application code for newcomer tasks, based on the schema(s) being developed in T235930, and based on the measurement specifications that can be found in this document. Engineers should feel free to make subtasks if helpful.

Details

Related Gerrit Patches:
mediawiki/extensions/GrowthExperiments : masterTaskExplanationWidget: Fix action name for opening/closing explanation popup
mediawiki/extensions/GrowthExperiments : masterNewcomer Tasks: Add maintenance template information to tasks
mediawiki/extensions/GrowthExperiments : masterSuggested Edits: Instrumentation

Event Timeline

LGoto triaged this task as Medium priority.Oct 21 2019, 5:04 PM
LGoto edited projects, added Product-Analytics; removed Product-Analytics (Kanban).
LGoto moved this task from Triage to Tracking on the Product-Analytics board.

Change 547204 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] (wip) Suggested Edits: Instrumentation

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

Tgr claimed this task.Nov 1 2019, 6:56 PM

Stealing this temporarily while Kosta is OoO.

Tgr added a comment.Nov 1 2019, 11:10 PM

specific maintenance template (we don't track these currently, would require some changes to the API or the code calling it)

Currently we do a search like hastemplate:tpl1|tpl2|tpl3 and we don't know which of those were matched by the search engine. The options I can think of:

  • do a separate search for every template, instead of every task type - would probably seriously degrade performance.
  • include prop=templates in the client-side API request - probably some performance impact, and for pages with many templates where the template is not on the top of the page, we might need to deal with query continuations, which we can't as long as the search results are random.
  • do a templatelinks query on the server side (something along the lines of SELECT tl_from, tl_title FORM templatelinks WHERE tl_from IN (<recommended task IDs>)

The last one seems to be the only sane solution.

do a templatelinks query on the server side (something along the lines of SELECT tl_from, tl_title FORM templatelinks WHERE tl_from IN (<recommended task IDs>)

Yes, agreed.

Change 549243 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] Newcomer Tasks: Add maintenance template information to tasks

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

Change 547204 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Instrumentation

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

Change 549243 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Newcomer Tasks: Add maintenance template information to tasks

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

@Catrope -- I finished testing the instrumentation for desktop today (and only the parts that go into HomepageModule -- not the parts that happen once the user has selected an article). In general, things look good. But I found four issues:

  • When the user loads the homepage, they get an impression event for each module, including the suggested edits module. That impression's action_data is supposed to contain the task types selected in the SE module and the number of articles in the queue (which is usually 200). But all these impression events have taskCount=null in action_data.
  • When the user sees a suggestion, they get an se-task-impression event. The action_data is supposed to contain the maintenance template that caused the article to be suggested. Many of the articles end up with this recorded in action_data, but many also have maintenanceTemplates=null. In my dataset of 210 se-task-impression events, 19 have nulls. Potentially notable is that all of the nulls have taskType=copyedit, but not all of the taskType=copyedit have nulls.
  • In se-task-impression events, most articles have pageviews=null. This is probably related to T238178: Newcomer tasks: pageview count not appearing or inconsistent.
  • Although the browser console records events for interactions with the TaskExplanationWidget, open and close events did not show up in the database. Maybe there is some validation error somewhere? Those include events where action_data is se-task-explanation-open and se-task-explanation-close. However, events where action_data is se-explanation-link-click do show up in the database.

Going back to Ready for Development.

@Catrope -- I finished testing the instrumentation for desktop today (and only the parts that go into HomepageModule -- not the parts that happen once the user has selected an article). In general, things look good. But I found four issues:

  • When the user loads the homepage, they get an impression event for each module, including the suggested edits module. That impression's action_data is supposed to contain the task types selected in the SE module and the number of articles in the queue (which is usually 200). But all these impression events have taskCount=null in action_data.

Right now it's impossible to know this information at impression time, because the module is still loading then. Subsequent events should have the correct taskCount and taskTypes in action_data, and those things should also update as the user changes the state of the module through the filters dialog. To remedy this, we could try to delay the impression event for the suggested edits module until its contents have loaded; right now, we fire impression events for all modules on the page at the same time.

  • When the user sees a suggestion, they get an se-task-impression event. The action_data is supposed to contain the maintenance template that caused the article to be suggested. Many of the articles end up with this recorded in action_data, but many also have maintenanceTemplates=null. In my dataset of 210 se-task-impression events, 19 have nulls. Potentially notable is that all of the nulls have taskType=copyedit, but not all of the taskType=copyedit have nulls.

I discovered why this is: the Czech template definitions use things like "Kdy\?" while the template name is Kdy?. Presumably the backslash escaping the question mark is needed for the search code we use, but that should really be handled by our code, not put in the JSON file. Now our code that tries to recognize copyedit templates on articles looks for "Kdy\?" with a literal backslash, and doesn't find it.

Yes, almost certainly.

  • Although the browser console records events for interactions with the TaskExplanationWidget, open and close events did not show up in the database. Maybe there is some validation error somewhere? Those include events where action_data is se-task-explanation-open and se-task-explanation-close. However, events where action_data is se-explanation-link-click do show up in the database.

Yes, this is a validation error. The schema expects se-explanation-open (and -close) rather than se-task-explanation-open. I will change the code to emit se-explanation-open instead.

Change 550989 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] TaskExplanationWidget: Fix action name for opening/closing explanation popup

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

I discovered why this is: the Czech template definitions use things like "Kdy\?" while the template name is Kdy?. Presumably the backslash escaping the question mark is needed for the search code we use, but that should really be handled by our code, not put in the JSON file. Now our code that tries to recognize copyedit templates on articles looks for "Kdy\?" with a literal backslash, and doesn't find it.

It appears that these backslashes are not needed by the search code, so I've removed them from the JSON page. This change should take effect immediately (no deployment needed), and you should no longer see nulls. Hopefully you will also see tasks whose template name ends in a question mark (e.g. Kdy?); if not, we may have broken the search for those templates.

  • When the user loads the homepage, they get an impression event for each module, including the suggested edits module. That impression's action_data is supposed to contain the task types selected in the SE module and the number of articles in the queue (which is usually 200). But all these impression events have taskCount=null in action_data.

Right now it's impossible to know this information at impression time, because the module is still loading then. Subsequent events should have the correct taskCount and taskTypes in action_data, and those things should also update as the user changes the state of the module through the filters dialog. To remedy this, we could try to delay the impression event for the suggested edits module until its contents have loaded; right now, we fire impression events for all modules on the page at the same time.

Reading the code, it appears that every suggested edits impression event should always be followed by an se-fetch-tasks event once the module finishes loading. Is this the case, and does that event have the correct/complete task count data? If so, all the information you need is in the event stream, just not in the precise way you asked for, and I suggest leaving this unfixed.

@Catrope -- sounds good to me about leaving the "impression" situation unfixed.

Change 550989 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] TaskExplanationWidget: Fix action name for opening/closing explanation popup

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

I tested mobile instrumentation today and found no additional issues beyond the ones I listed yesterday.

This should be QAed by @Etonkovidova, including the bugs left in the comments.

Hopefully you will also see tasks whose template name ends in a question mark (e.g. Kdy?); if not, we may have broken the search for those templates.

I just wanted to note the following, maybe we need to look into it a bit more. On Czech wiki, this request for "Kdy?" yields 49 results.

curl 'https://cs.wikipedia.org/w/api.php?action=query&format=json&list=search&srlimit=max&srnamespace=0&srqiprofile=classic_noboostlinks&origin=*&srsearch=hastemplate:%22Kdy?%22' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:71.0) Gecko/20100101 Firefox/71.0' -H 'Accept: */*' -H 'Accept-Language: en-US,en;q=0.7,el;q=0.3' --compressed -H 'Origin: https://newcomertasks-prototype.netlify.com' -H 'Connection: keep-alive' -H 'Referer: https://newcomertasks-prototype.netlify.com/' -H 'TE: Trailers' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache'

But if I escape the question mark, then I get 1296 results

curl 'https://cs.wikipedia.org/w/api.php?action=query&format=json&list=search&srlimit=max&srnamespace=0&srqiprofile=classic_noboostlinks&origin=*&srsearch=hastemplate:%22Kdy\?%22' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:71.0) Gecko/20100101 Firefox/71.0' -H 'Accept: */*' -H 'Accept-Language: en-US,en;q=0.7,el;q=0.3' --compressed -H 'Origin: https://newcomertasks-prototype.netlify.com' -H 'Connection: keep-alive' -H 'Referer: https://newcomertasks-prototype.netlify.com/' -H 'TE: Trailers' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache'

I guess it's seen more easily in the web interface: https://cs.wikipedia.org/w/index.php?sort=relevance&search=hastemplate%3A%22Kdy%5C%3F%22&title=Speci%C3%A1ln%C3%AD:Hled%C3%A1n%C3%AD&profile=advanced&fulltext=1&advancedSearch-current=%7B%7D&ns0=1&ns100=1&ns102=1&searchToken=ch4l99ls5o2jkk547z4r3e5lv

note how "Kdy\?" results in https://cs.wikipedia.org/wiki/CAPTCHA being returned, and its source has {{kdy?}}, whereas Kdy? does not yield that title.

So, are we sure about that change? It seems to me that we need to escape the question mark. I'll try to verify that in my dev environment after I'm done with my backlog of emails

Tgr added a comment.Nov 22 2019, 10:02 PM

Seems like hastemplate:"Kdy?" only returns pages which have the Kdy template (a little-used redirect to Kdy?) - the templatecount numbers match the search ones within a reasonably small margin of error. Which is very different from what the documentation says, but then the documentation never explicitly says if the same escaping rules apply to filters as anything else :-/

In any case, Roan has a point that any escaping should be applied in the code, not in the configuration.

Tgr added a comment.Nov 22 2019, 10:27 PM

Looking at the code, it seems [[https://gerrit.wikimedia.org/g/mediawiki/extensions/CirrusSearch/+/60637db866e21ccf3d1b458163e6a1762de68303/includes/Util.php#351|stripQuestionMarks ]] is invoked on the search string before all else, and it completely ignores quotes and only cares about backlash-escaping. At some point we might want to look into building our own ES query to have a little more predictable behavior; for now we should escape question marks. Thanks for catching it, Kosta.

Etonkovidova added a comment.EditedDec 4 2019, 12:30 AM

Checked in betalabs eventlogging db and it seems that several issues noticed by @MMiller_WMF

Many of the articles end up with this recorded in action_data, but many also have maintenanceTemplates=null. In my dataset of 210 se-task-impression >events, 19 have nulls. Potentially notable is that all of the nulls have taskType=copyedit, but not all of the taskType=copyedit have nulls.

Now nulls are not present, e.g.

action: "se-fetch-tasks"
action_data: "taskTypes=copyedit,links;taskCount=200"

action: "se-task-impression"
action_data: "taskTypes=copyedit,links;taskCount=200;taskType=copyedit;maintenanceTemplates=Pravopis,Sloh;hasImage=false;ordinalPosition=0;pageviews=338;pageTitle=Mien Ruys;pageId=199470;revisionId=402083"

The schema expects se-explanation-open (and -close) rather than se-task-explanation-open. I will change the code to emit se-explanation-open instead.

It was impossible to see in betalabs, but I saw that such events were successfully recorded in production (in updates provided by @MMiller_WMF).

MMiller_WMF closed this task as Resolved.Dec 10 2019, 11:26 PM

Thank you!