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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 1

[:Margaret Leibovic]

Is there a meta bug this could block so it doesn't get lost?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 4

[:Margaret Leibovic]

Going to optimistically track for 43. Chenxia, let us know if you think that's unreasonable.

Comment 5

[:Margaret Leibovic]

Sounds like this isn't something we're going to get for 43. Chenxia, do you have any ideas for a simple solution here?

Comment 6

[Chenxia Liu [:liuche]]

Attachment: 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

Comment 7

[Chenxia Liu [:liuche]]

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.

Comment 8

[Chenxia Liu [:liuche]]

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

Comment 9

[Chenxia Liu [:liuche]]

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?

Comment 10

[Nick Alexander :nalexander [he/him]]

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.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(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 12

[Nick Alexander :nalexander [he/him]]

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.

Comment 13

[:Margaret Leibovic]

Any update here? It would be good to be able to do this if we start using Switchboard for more features.

Comment 14

[Chenxia Liu [: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.

Comment 15

[: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.

Comment 16

[:Margaret Leibovic]

The ship has sailed on 43, but we really should get around to doing this sooner rather than later.

Comment 17

[:Margaret Leibovic]

Poking this bug... as we start rolling out more experiments, this will be more important to help testers.

Comment 18

[Chenxia Liu [:liuche]]

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/

Comment 19

[Chenxia Liu [:liuche]]

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

Comment 20

[: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?

Comment 21

[Chenxia Liu [:liuche]]

> 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 22

[Chenxia Liu [:liuche]]

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/

Comment 23

[: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 24

[: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/#review30673

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/a6c1554af2a7

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a6c1554af2a7

Comment 27

[:Margaret Leibovic]

Can you update the README in the switchboard-experiments repo to document this?

Comment 28

[Chenxia Liu [:liuche]]

https://github.com/mozilla-services/switchboard-experiments/pull/7