1761171 - Implement telemetry for the new Suggest opt-in modals in 100

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Drew Willcoxon :adw]

There are two kinds of telemetry we need to add/change:

Nimbus exposure event

For users in a treatment branch, the exposure event should be recorded when they first encounter the new UI. In this case, that means when they are shown the modal. For users in the control branch, it should be recorded when they would have been shown the modal.

Currently the exposure event is recorded when users are first shown a Suggest suggestion (source). For previous modal experiments, we have not recorded the exposure event when users were shown the modal but we really should have.

Right now the event is recorded by UrlbarProviderQuickSuggest._ensureExposureEventRecorded(), but I think we should move that method to UrlbarQuickSuggest. To detect when the user is enrolled in a modal experiment, I think we should add a new enum Nimbus variable called experimentType. For the best match experiment, we added a isBestMatchExperiment boolean, but we should have added experimentType so we can use the same variable for each experiment. The values for experimentType should be "best-match" and "modal". Then ensureExposureEventRecorded() can use UrlbarPrefs.get("experimentType") to determine if the user is in a modal experiment and UrlbarPrefs.get("quickSuggestOnboardingDialogVariation") to determine if the user is in a treatment branch; if they are, quickSuggestOnboardingDialogVariation will have a value, and if they aren't, it won't have a value.

In the onboarding dialog test, we can use QuickSuggestTestUtils.assertExposureEvent() and clearExposureEvent().

Other telemetry

We already have telemetry for the modal variations so there's not much we need to add. The only thing it looks like we need is a learn_more_1 since we now have a learn more link in the first intro pane in addition to the existing one in the second main pane.

Since treatment B has only one pane, it's debatable whether we should add accept_1, reject_1, etc. since we only have accept_2, reject_2, etc. right now. It's probably OK to use the 2 versions for treatment B but I'll check with Rebecca to make sure.

Comment 1

[Daisuke Akatsuka (:daisuke)]

I have one question. Assuming we introduce experimentType and set it as "best-match".
If we change this evaluation to like below, the behavior is as same as the explanation in FeatureManifest.yaml.

if (
  UrlbarPrefs.get("experimentType") !== "best-match" ||
  (isSuggestionBestMatch &&
    (!UrlbarPrefs.get("bestMatchEnabled") ||
      UrlbarPrefs.get("suggest.bestmatch")))
) {
  this._ensureExposureEventRecorded();
}

On the other hand, assuming experimentType is "modal". As experimentType is not "best-match", it will work as same as isBestMatchExperiment: false, the exposure event will be recorded. To avoid it, should we do like below that the case of no exmerimentType is as same as the behavior of isBestMatchExperiment: false?

if (
  !UrlbarPrefs.get("experimentType") ||
  (isSuggestionBestMatch &&
    UrlbarPrefs.get("experimentType") === "best-match" &&
    (!UrlbarPrefs.get("bestMatchEnabled") ||
      UrlbarPrefs.get("suggest.bestmatch")))
) {
  this._ensureExposureEventRecorded();
}

Comment 2

[Daisuke Akatsuka (:daisuke)]

And one more question. Are there no cases in that we do multiple experiments at the same time?

Comment 3

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1761171: Add learn_more_1 telemetry for the onboarding dialog.

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1761171: Introduce experimentType nimbus variable.

Depends on D142031

Comment 5

[Drew Willcoxon :adw]

Ah, sorry for not explaining this a little better. There are a couple of things I should have mentioned:

• We need to keep the existing isBestMatchExperiment variable unfortunately. That's because we're already using it in the experiment recipe and in Firefox 99. Neither the experiment nor 99 has shipped yet, so it may be possible to replace it with experimentType, but IMO we should stick with it to avoid causing last-minute problems and work for people (Nan, release drivers, and QA in particular).
• With my suggested changes (and your patch), the description of isBestMatchExperiment is now incorrect. When isBestMatchExperiment is true, then yes, the exposure event should still be recorded when the user triggers a best match. When it's false, then the event will depend on which experiment the user is enrolled in. So could you update its description in the manifest please?

(In reply to Daisuke Akatsuka (:daisuke) from comment #2)

And one more question. Are there no cases in that we do multiple experiments at the same time?

A user will never be enrolled in multiple experiments at one time, so we don't need to worry about that. Of course there may be many experiments that are active at a time (there are right now), but a single user will only ever be enrolled in one.

So we need to change this logic that you mentioned to the following:

    // The user triggered a suggestion. Depending on the experiment the user is
    // enrolled in (if any), we may need to record the Nimbus exposure event.
    //
    // If the user is in a best match experiment:
    //   Record if the suggestion is itself a best match and either of the
    //   following are true:
    //   * The best match feature is enabled (i.e., the user is in a treatment
    //     branch), and the user has not disabled best match
    //   * The best match feature is disabled (i.e., the user is in the control
    //     branch)
    // Else if the user is not in a modal experiment:
    //   Record the event
    if (
      UrlbarPrefs.get("isBestMatchExperiment") ||
      UrlbarPrefs.get("experimentType") == "best-match"
    ) {
      if (
        isSuggestionBestMatch &&
        (!UrlbarPrefs.get("bestMatchEnabled") ||
          UrlbarPrefs.get("suggest.bestmatch"))
      ) {
        UrlbarQuickSuggest.ensureExposureEventRecorded();
      }
    } else if (UrlbarPrefs.get("experimentType") != "modal") {
      UrlbarQuickSuggest.ensureExposureEventRecorded();
    }

Comment 6

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #0)

Since treatment B has only one pane, it's debatable whether we should add accept_1, reject_1, etc. since we only have accept_2, reject_2, etc. right now. It's probably OK to use the 2 versions for treatment B but I'll check with Rebecca to make sure.

Rebecca says that using the _2 versions is OK.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca3baa5cc311
Add learn_more_1 telemetry for the onboarding dialog. r=adw
https://hg.mozilla.org/integration/autoland/rev/895885c5eb30
Introduce experimentType nimbus variable. r=adw

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ca3baa5cc311
https://hg.mozilla.org/mozilla-central/rev/895885c5eb30