Page MenuHomePhabricator

Prepare feature flagging gateway for mobile issues work
Closed, ResolvedPublic5 Story Points

Description

To prepare for the mobile issues work, we will need to setup some feature flagging which separates 2 different treatments for page issues. We will not worry about cached HTML right or FOUC right now, nor will we worry about the new design treatment (tracked in T191303)

In group control or A (which should show for 100% of users by default):

In group B (which should be set to 0):

Acceptance criteria

  • Add a feature flag with control, A and B groups. When group control or A is present show the existing design. When group B is present show the page issue without the design treatment
  • The client should decide whether to enter group A or B
  • It should be possible to enter group B on reading web staging or a local machine, but impossible to enter the group on production.

Developer notes

Do not worry about flash of unstyled contents. The purpose of this task is to get a sense of how much of a problem these are and provide us a space to maintain 2 versions of page issues.

Make use of mediawiki.experiments to take care of the bucketing.

We will bucket across all pages for the time being, but this may change later.

Note: Tilman has asked for all events that we log to parallel those for ReadingWebDepth, thus session id and sampling rate must be consistent for these. This information shouldn't be necessary to complete implementation for this task, but we should bear it in mind in our decision making here.

QA

  • Make sure page issues is working correctly on the beta cluster without any visible change from before.
  • Ask a developer to help test the A/B test switching and verify that it's possible to load the second treatment.

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterA/B test bucketing wrapper for page issues AB.
mediawiki/extensions/WikimediaEvents : masterAllow extensions to explicitly opt into the ReadingDepthSchema

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 1 2018, 10:35 PM
Jdlrobson updated the task description. (Show Details)
ovasileva triaged this task as High priority.May 2 2018, 1:31 PM
ovasileva updated the task description. (Show Details)May 2 2018, 5:03 PM
Jdlrobson updated the task description. (Show Details)May 2 2018, 5:03 PM
Jdlrobson updated the task description. (Show Details)May 2 2018, 5:07 PM
Jdlrobson updated the task description. (Show Details)May 2 2018, 5:12 PM
ovasileva set the point value for this task to 5.May 2 2018, 5:13 PM
Jdrewniak claimed this task.May 3 2018, 2:41 PM
Jdrewniak moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.
Jdrewniak added a comment.EditedMay 7 2018, 2:21 PM

This feature feels like it should live in the WikimediaEvents repo to me:

  1. It's temporary
  2. It will result in an A/B test with event-logging
  3. It relates to other event-logging experiments (reading depth)

The way I see this working is that we create a new module in WikimediaEvents and put the feature-switcher in there. For performance reasons, I see this feature switcher as doing nothing more than adding a css class to the document body.

Loading modules via JS has a pretty high performance/FOUC cost, so instead, I think we could load all the old & new styles up-front, and reveal them depending on which class is on the body element.

@Jdlrobson what do you think about this approach? Did you have something more generic in mind?

After some discussion it was agreed that loading as much code upfront for the test would reduce FOUC, but that placing the code in Minerva would give us the most flexibility as to when the code is executed.

Change 432097 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Page issues feature switching for A/B test

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

Change 437686 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/WikimediaEvents@master] Allow extensions to explicitly opt into the ReadingDepthSchema

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

Change 437686 merged by jenkins-bot:
[mediawiki/extensions/WikimediaEvents@master] Allow extensions to explicitly opt into the ReadingDepthSchema

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

We're almost there :) Will likely get merged tomorrow.

Change 432097 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] A/B test bucketing wrapper for page issues AB.

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

Jdlrobson added a comment.EditedJun 14 2018, 5:22 PM

This initial code is merged but disabled. We're planning to iterate on this code some more and we have a meeting planned on Wednesday to discuss in detail with Tilman.

This task should not be signed off until that has happened.

We chatted with @Tbayer about this. The plan is to add an additional field to ReadingWebDepth to show where bucketing comes from. This will be done as a separate task. Are there any other follow ups we need to do here based on our conversation (other than keep an eye on the FOUC from a design POV)?

phuedx claimed this task.Jun 25 2018, 5:11 PM
phuedx added subscribers: Jdrewniak, phuedx.

As discussed in today's standup meeting, I'll create a follow on task to capture the follow-on change to the ReadingDepth instrumentation to mark events as "oversampled" (using the terminology from the NavigationTiming schema).

I'm getting the following behavior and just wanted to check if that's expected, it strikes as a bit odd:

  1. go to http://reading-web-staging.wmflabs.org/wiki/Demolition_Doll_Rods
  2. Page issues appear with old design (link)
  3. close tab (not closing the browser)
  4. open again
  5. page issues appear with new design

This is not happening on refresh.

I'm getting the following behavior and just wanted to check if that's expected.

The behaviour you describe is expected. A browser session is limited to the lifetime of the browser tab or window and the die that we roll on your behalf will stick for the duration of your session, i.e. we'll roll a die in #1 and in #4 but we won't roll one when you refresh.

…I'll create a follow on task to capture the follow-on change to the ReadingDepth instrumentation to mark events as "oversampled" (using the terminology from the NavigationTiming schema).

This was already captured in the last AC for T191532: Mobile page issues - instrument page issues, which is the follow-on task to this task 💪

phuedx updated the task description. (Show Details)Jun 27 2018, 10:56 AM
phuedx added a comment.EditedJun 27 2018, 11:10 AM

For T193584#4318754:

Add a feature flag with control, A and B groups. When group control or A is present show the existing design. When group B is present show the page issue without the design treatment.

rSMIN45a4fda37980: Mobile page issues - visual styling changes actually implements this as:

BucketInstrumentation Enabled?New Treatment?
control
A
B

which is fine as the A and B buckets are the same size.

phuedx added a comment.EditedJun 27 2018, 11:22 AM

It should be possible to enter group B on reading web staging or a local machine, but impossible to enter the group on production.

ssh readingwebstaging.reading-web-staging.eqiad.wmflabs
grep ABSamplingRate /srv/mediawiki-vagrant/Staging.php

yields

$wgMinervaABSamplingRate = 1;

i.e. on the staging server, the size of the B bucket is 0.5.

I can confirm that no such configuration has been added to the operations/mediawiki-config repo.

phuedx updated the task description. (Show Details)Jun 27 2018, 12:08 PM
phuedx closed this task as Resolved.Jun 27 2018, 12:12 PM

Alright. This LGTM 👍

Vvjjkkii renamed this task from Prepare feature flagging gateway for mobile issues work to ztdaaaaaaa.Jul 1 2018, 1:13 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed phuedx as the assignee of this task.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed the point value for this task.
Vvjjkkii removed subscribers: gerritbot, Aklapper.
phuedx renamed this task from ztdaaaaaaa to Prepare feature flagging gateway for mobile issues work.Jul 2 2018, 4:36 AM
phuedx closed this task as Resolved.
phuedx claimed this task.
phuedx updated the task description. (Show Details)
phuedx set the point value for this task to 5.
phuedx added a subscriber: Aklapper.