Page MenuHomePhabricator

MPIC: Update A/B test and instrument forms with new Sampling fields
Closed, ResolvedPublic3 Estimated Story Points

Assigned To
Authored By
Sfaci
Sep 17 2024, 2:12 PM
Referenced Files
Restricted File
Jan 13 2025, 7:06 PM
F58147249: Screenshot 2025-01-08 at 17.40.35.png
Jan 8 2025, 4:45 PM
F58147164: Screenshot 2025-01-08 at 17.26.52.png
Jan 8 2025, 4:45 PM
F57752952: Screenshot 2024-11-27 at 12.09.25.png
Nov 27 2024, 11:10 AM
F57752946: sample unit field.png
Nov 27 2024, 11:10 AM
F57656634: Screenshot 2024-10-29 at 20.51.31.png
Oct 29 2024, 7:51 PM
F57652676: Screenshot 2024-10-28 at 12.37.17.png
Oct 28 2024, 11:43 AM
F57652660: Screenshot 2024-10-28 at 12.31.56.png
Oct 28 2024, 11:32 AM

Description

Background

While working on the new A/B test forms, we took the opportunity to restyle the Location component we have to fill the sample_rate per wiki when registering/updating an instrument/experiment. But, in favour of prioritizing other work, we decided to work on this a bit later.

Suggested solution

The current "Sample unit" and "Locations and sampling rate" present a set of visual and copy issues that we could address to make it more user-friendly. In the new design proposal:

  • The section under which the fields are grouped could be called "Sampling"

Sample unit field:

  • The "Sample unit" field is now placed before the Location module (now called "Project and sample size") to attempt to make the relationship between parameters clearer
  • The "Sample unit" presents a new description and includes a "Learn more" link to https://wikitech.wikimedia.org/wiki/Metrics_Platform/Analytics_sampling
  • The select component's placeholder is updated to "Select the sample unit"
  • The "Sample unit" field options include descriptions (see designs below)

sample unit field.png (585×1 px, 118 KB)

Project and sample size:

  • The current "Location and sample rate" field is replaced by a new "Project and sample size" module
  • The label "Location" is replaced by "Wiki project"
  • The current custom multi-value lookup used to select locations/projects is replaced by the new LookupMultiselect component
  • The label "Sample rate" is replaced by "Sample size"
  • The "Sample rate" field allows users to enter values as percentages. Ideally, the field could be prefilled with a percentage symbol to simplify the input of the value
  • Icon-only buttons of the type quiet are used instead of normal buttons to allow the deletion of project entries

Screenshot 2024-11-27 at 12.09.25.png (810×1 px, 122 KB)

Considerations

  • Please consider breaking down this task and tackling the replacement of the custom chip input component separately, in case it adds too much effort.
  • It'd be ideal if we could validate the input of percentages higher than 100%. This effort could be captured in a separate ticket
  • This is an initial iteration on a part of the configuration that proved to be challenging for users during our recent exploration. It is assumed that the changes will suppose an improvement, but that removing the complexity attached to this specialized field would require further explorations and validation.
  • Please feel free to improve any copy in case inaccuracies are detected

Acceptance Criteria

  • The new "Sample size per project" module is used in the A/B test and baseline instrument forms
  • In both artifact forms, the adjusted "Sample unit "field is placed before the "Sample size per project"
  • The fields are grouped in their own section called "Sampling" right under the "Schema" section in the instrument form and the "Details" section in the A/B test form

Event Timeline

Sfaci set the point value for this task to 3.Sep 17 2024, 3:43 PM

@VirginiaPoundstone Based on product prioritization from https://docs.google.com/document/d/1B5OfJfBiSfa19IRm-dohZuzAHNYOqXdLw2mhHqJh7Dg/edit?tab=t.0#heading=h.b9g0gh3wsg1f, should this task still be worked on in this sprint? Or should we spin up whatever tickets are needed after the high + medium priority line items are left?
cc @Sfaci

Sarai-WMF renamed this task from MPIC: Update A/B test and baseline instrument forms to use the new look and feel for the Location component to MPIC: Update A/B test and instrument forms with new Sampling fields.Oct 18 2024, 3:23 PM
Sarai-WMF updated the task description. (Show Details)

The task was updated with an initial design iteration proposal. Please feel free to make adjustments and reach out in case details or clarifications are needed. Thank you!

Sarai-WMF updated the task description. (Show Details)

⚠ Important heads-up: If, following what's suggested on this ticket, we decided to replace the "Sample rate" field by a "Sample size" field that takes percentages, then the "Sample rate" section in https://wikitech.wikimedia.org/wiki/Metrics_Platform/Analytics_sampling would need to be updated

Att @apaskulin

@SGupta-WMF heads-up that the replacement of "sample unit" by "sampling unit", specified in this task, has been invalidated. We should only use the former (current) term. I'll update the task's description and the designs attached accordingly.

I'm being bold and taking this task off of @SGupta-WMF's hands while they are onboarding to the Wikimedia Enterprise team. The first thing I'll do is update the task description given the updates that have been made to the documentation (see @Sarai-WMF's comment above).

hey @phuedx. I did a first pass and updated the tasks' description given that the designs also needed to be rectified. There might be other adjustments needed. Happy to amend any specs if that's the case. Thank you!

Change #1105431 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Metrics Platform Instrument Configuration: Deploying to staging

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

Change #1105433 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Metrics Platform Instrument Configuration: Deploying to production

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

Change #1105431 merged by jenkins-bot:

[operations/deployment-charts@master] Metrics Platform Instrument Configuration: Deploying to staging

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

Change #1105433 merged by jenkins-bot:

[operations/deployment-charts@master] Metrics Platform Instrument Configuration: Deploying to production

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

Hey @VirginiaPoundstone and @phuedx 👋🏻 The new Sampling fields look and work amazing 💯 I just have a couple of comments:

  1. Copy: In this ticket, the suggestion was to use percentages as units for Sample size (there were perceived to be more aligned with users' expectations). I'm assuming that continuing to use decimals was a deliberate decision, so I just wonder if the field's description should be updated. The description currently says "Define the percentage of the selected sampling unit that should be tracked in each relevant project.". Maybe something more generic could prevent any potential misunderstandings/wrong inputs. E.g.,: "Define the proportion of the selected sample unit that should be tracked in each relevant project.". (Also correcting and using "sample unit"). What do you think?
  1. Spacing: The spacing between the elements within the "Project and sample size" module is a bit off in relation to the Figma specs. This is a minor fix that perhaps could be tackled in a separate design fixes type task.

{F58187197}

  1. Placement/existence of the A/B test concurrency warning: This might not be directly related to this ticket, but now the message is displayed under the "Variants" section instead of directly below the "Duration" module as before. Should the warning be moved back to its original position? Should we replace it with a more relevant solution, as we were debating?

Screenshot 2025-01-08 at 17.40.35.png (358×1 px, 46 KB)

Hi @Sarai-WMF! I will be bold and I will try to answer to your questions here. I was a bit involved in some of them:

  1. Regarding the percentage, you are right, it was a deliberate decision because using percentages here was more difficult that we thought at the beginning and, if I remember right, we also consider that the effect of that change wouldn't be much
  1. Definitely you are right. I missed that during review!
  1. And you are right again! I didn't realize that before but reviewing your designs right now I have found that, as you mentioned, the warning should be at the end of the Duration section. We should fix it.

I would say that all these issues are pretty straightforward and we should be able to prepare a MR for all of them. My only concern is the one related to styles but, if we see that it's a big deal, we can always fix it a bit later. I would say that's the less critical regarding MPIC Alpha which is our priority right now.

Items 1 and 3 are already included in https://gitlab.wikimedia.org/repos/data-engineering/mpic/-/merge_requests/162 that is ready for review.

According what we have discussed on Slack, we will create a separate ticket for the one about spacing. It seems there are other related and similar issues that we should fix as well

Change #1111318 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Experiment Platform Instrument Configuration: Deploying to staging

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

Change #1111321 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Experiment Platform Instrument Configuration: Deploying to production

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

Change #1111318 merged by jenkins-bot:

[operations/deployment-charts@master] Experiment Platform Instrument Configuration: Deploying to staging

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

Change #1111321 merged by jenkins-bot:

[operations/deployment-charts@master] Experiment Platform Instrument Configuration: Deploying to production

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

Thanks for the fixes! Copy and message placement were reviewed on mpic-next and everything looks correct. The problem with the concurrency message showing up in the Instrument form is already been tackled 🚀 and the issues with form modules styling were documented in a separate task as agreed: T383983: xLab: Correct form modules' style