Closed Bug 1201653 Opened 5 years ago Closed 5 years ago

Provide a way to self-select into a specific experiment [Switchboard]

Categories

(Firefox for Android :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed
fennec 45+ ---

People

(Reporter: mfinkle, Assigned: liuche)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Switchboard controls the user segmenting. A user can't force themselves to be in or out of a particular experiment. It would be nice for QA and L10N if we could find a way to allow self selecting.
Is there a meta bug this could block so it doesn't get lost?
Flags: needinfo?(liuche)
Blocks: ab-v1
Flags: needinfo?(liuche)
Duplicate of this bug: 1210044
We might have two different goals here:
1. Use a predetermined UUID so a tester can force a specific A/B segment.
2. Ignore A/B altogether and fallback to a default code flow.

For both, I think the simplest way forward is passing params to Fennec launched via ADB.

#1: Pass an ignore flag, which would be used to effectively disable SwitchBoard for that session. We currently check AppConstants.MOZ_SWITCHBOARD to see if SwitchBoard is enabled. We could use Snorp's ExperimentHelper idea to add an isEnabled check that uses AppConstants _and_ a secondary flag.

#2: Pass a UUID as a commandline param when starting Fennec via ADB. The UUID would then be passed to the SwitchBoard config loader and used as the session's UUID instead of using the generated UUID.

Places that accept a UUID:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/thirdparty/com/keepsafe/switchboard/SwitchBoard.java#186
http://mxr.mozilla.org/mozilla-central/source/mobile/android/thirdparty/com/keepsafe/switchboard/AsyncConfigLoader.java#62

We call one of those here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#750
tracking-fennec: --- → ?
Going to optimistically track for 43. Chenxia, let us know if you think that's unreasonable.
Assignee: nobody → liuche
tracking-fennec: ? → 43+
Sounds like this isn't something we're going to get for 43. Chenxia, do you have any ideas for a simple solution here?
Flags: needinfo?(liuche)
Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=mfinkle
Attachment #8680995 - Flags: review?(mark.finkle)
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

This is a first pass of a patch to use flags - if you don't pass a flag value in, it defaults to the usual switchboard behavior.

(I actually just had a thought that I should add capability to pass in a 0, to turn off switchboard entirely, while not having it defined just runs everything normally).

I'll test the different values (because clobbering) tomorrow, but right now the sanity check of "does it build and run switchboard normally" seem fine.

I looked up how to pass in a param from adb, and to do that we start an intent and we can pass in extras. Is that what we want?
http://developer.android.com/tools/help/shell.html#am (under the "<INTENT> Arguments" part). I don't think that'd be more complicated, and maybe that's a way that we can just cover all the bases (without using configure flags) because we can pass in any extra we want.
Flags: needinfo?(liuche)
Attachment #8680995 - Flags: review?(mark.finkle) → feedback?(mark.finkle)
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=mfinkle
Attachment #8680995 - Flags: feedback?(mark.finkle) → review?(mark.finkle)
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

Hm, so the first version of this had a typo in it so I wasn't hitting the code where I was using the param value in preprocessing - I fixed this and then hit a snag.

Specifically, when the build hits the switch flag for setting A/B-variants in AppConstants, the value it substitutes in is empty, which causes the compiler to complain. It's defined, but empty. I know I'm missing a step - but what is it?
Attachment #8680995 - Flags: review?(mark.finkle) → feedback?(nalexander)
https://reviewboard.mozilla.org/r/23717/#review21309

I think I see the issue.

I'd like to step back, however, and ask why we want to bake this into the build system.  Can we do this with a specially crafted intent?  "Start first-run, with this configuration?"  It'll be a *lot* easier for testing, and let folks take a Nightly build and just ... try the scenarios.

If that's not reasonable, I see no reason to map from ints to your special constants.  Why not include the constant strings in the build configuration?

::: configure.in:3763
(Diff revision 2)
> +MOZ_AB_TESTING_VARIANT_ONBOARDING=

You'll want something in your `mozconfig` to set this -- `mk_add_options`, IIRC.

::: mobile/android/base/moz.build:877
(Diff revision 2)
>      DEFINES[var] = CONFIG[var]

You're defining the value unconditionally, so that you /always/ get the substitution in `AppConstants.java.in`.

Ths list above checks `if CONFIG[var]`, so it'll be undefined in `AppConstants.java.in` if it's undefined in the build system.
(In reply to Nick Alexander :nalexander from comment #10)

> I'd like to step back, however, and ask why we want to bake this into the
> build system.  Can we do this with a specially crafted intent?  "Start
> first-run, with this configuration?"  It'll be a *lot* easier for testing,
> and let folks take a Nightly build and just ... try the scenarios.

Yes! We only need a way to pass an extra, or some other flag, in a startup intent. See comment 3. Let's just make a way to pass in some args. I think we'd need two args:
1. disable switchboard - just a simple on/off
2. force a UUID  - Switchboard uses a UUID to control user segmentation. The right UUID will force Firefox into certain A/B code paths.
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

I commented on the patch, and I think we will try to do this without build system hackery.
Attachment #8680995 - Flags: feedback?(nalexander)
Any update here? It would be good to be able to do this if we start using Switchboard for more features.
Flags: needinfo?(liuche)
I'm going on PTO, and I'll pick this up again when I get back, but if we need it in the meantime, someone should take it over.
Flags: needinfo?(liuche) → needinfo?(margaret.leibovic)
(In reply to Chenxia Liu [:liuche] (PTO until 1/9) from comment #14)
> I'm going on PTO, and I'll pick this up again when I get back, but if we
> need it in the meantime, someone should take it over.

I didn't have time to work on this. You can pick this back up when you're back next week.
Flags: needinfo?(margaret.leibovic)
The ship has sailed on 43, but we really should get around to doing this sooner rather than later.
tracking-fennec: 43+ → 45+
Poking this bug... as we start rolling out more experiments, this will be more important to help testers.
Flags: needinfo?(liuche)
Attachment #8680995 - Attachment description: MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=mfinkle → MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret
Attachment #8680995 - Flags: review?(margaret.leibovic)
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23717/diff/2-3/
To pass in a switchboard uuid, start fennec as follows:

adb shell am start <package-name> --es switchboard-uuid <uuid>

where package name is something like org.mozilla.fennec_liuche

These are also some uuids that I've pulled from first runs:
onboarding2-a (old): 1
onboarding2-b (static): 4f6dd32e-5a5f-45db-9219-40f7c6cb4cd0
Onboarding2-c (static + dynamic): 79693e2a-d3ea-44ca-94f3-04f0887eaeb3
Flags: needinfo?(liuche)
Attachment #8680995 - Flags: feedback?(nalexander)
Attachment #8680995 - Flags: review?(margaret.leibovic)
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

https://reviewboard.mozilla.org/r/23717/#review30447

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:592
(Diff revision 3)
> +            final String switchboardUUID = intent.getStringExtra(INTENT_KEY_SWITCHBOARD_UUID);

We should probably be using ContextUtils.getStringExtra here. Also, I wonder if it's a threat vector for another app to pass in a UUID with an intent. Is there any way to only handle this if it's coming from a trusted source?
> Also, I wonder
> if it's a threat vector for another app to pass in a UUID with an intent. Is
> there any way to only handle this if it's coming from a trusted source?

Hm, I wouldn't say that starting the app from commandline or adb is really a "trusted source" anyways so I'm not sure what we could do alternatively. This also doesn't seem like a huge threat model - we don't overwrite the bucket, so if Fennec is started without this extra, we just default to whatever bucket we stored in SharedPreferences, or if there isn't one, we just make a new UUID (and then store it).
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23717/diff/3-4/
Attachment #8680995 - Flags: feedback?(nalexander) → review?(margaret.leibovic)
(In reply to Chenxia Liu [:liuche] from comment #21)
> > Also, I wonder
> > if it's a threat vector for another app to pass in a UUID with an intent. Is
> > there any way to only handle this if it's coming from a trusted source?
> 
> Hm, I wouldn't say that starting the app from commandline or adb is really a
> "trusted source" anyways so I'm not sure what we could do alternatively.
> This also doesn't seem like a huge threat model - we don't overwrite the
> bucket, so if Fennec is started without this extra, we just default to
> whatever bucket we stored in SharedPreferences, or if there isn't one, we
> just make a new UUID (and then store it).

It is a bit worrisome that another app could force a certain A/B version on a user, but then again, all these versions are things we're putting in front of users, so it's not like it causes data loss or anything. The main problems are that the UI could change on the user and our A/B test data could be affected, but there's not much incentive for a malicious app to do either of these things, so I think it's worth it to make testing easier.
Comment on attachment 8680995 [details]
MozReview Request: Bug 1201653 - Provide a way to self-select into a specific experiment [Switchboard]. r=margaret

https://reviewboard.mozilla.org/r/23717/#review30673
Attachment #8680995 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/a6c1554af2a7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Can you update the README in the switchboard-experiments repo to document this?
Flags: needinfo?(liuche)
Blocks: 1249384
You need to log in before you can comment on or make changes to this bug.