1877788 - Record reach for all messaging experiment features

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P1)

Description

[Shane Hughes [:aminomancer]]

Currently, we only record reach for 5 feature ids:

const REACH_EVENT_GROUPS = [
  "cfr",
  "moments-page",
  "infobar",
  "spotlight",
  "featureCallout",
];

But nowadays, there are many more messaging experiment feature ids that we'd want reach for. So, instead of using REACH_EVENT_GROUPS (which can be removed), we should use MESSAGING_EXPERIMENTS_DEFAULT_FEATURES. This will extend reach to the generic feature ids that we use when the named feature ids are already occupied.

The reason to do this is to reduce the risk of running an experiment where a precise exposure basis cannot be calculated. We need exposure or reach from the control branch to do this properly. But exposure is only recorded for control if the control message is a complete message (without content or template, to avoid actually showing something). If it's just an empty object {} exposure won't be recorded. And if that happens, reach is the only way to salvage the basis. But if the experiment is also on one of the fxms-message-* features, then we won't have that either, leaving DS with no option but to estimate exposure using past knowledge or some highbrow hackery beyond the comprehension of mere mortals.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Performance concerns related to reach came up when we did a couple of reach experiments like this one. The concern was mostly about sending reach for many different branches per user at once and the larger lists involved. Ultimately, we went ahead and never became aware of any problems.

For the normal use cases, we've been doing this for a small number of branches in our experiments forever and have not surfaced any problems of this sort, it's hard for me to imagine that the normal usage patterns would somehow magically be different for the same surfaces on what are just different fxms-* feature ids. I think we should do it for the principle of least surprise, in addition to the reasons Shane mentioned.

That said, I think pbNewtab warrants a discussion with legal making the change there, and that could happen either as part of this bug or it could just stay in bug 1855401.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Part of the scope of this bug should be verifying/updating existing docs on firefox-source-docs and experiment.info to make sure they reflect the new state of reality w.r.t. reach (eg all feature-ids or all except pbNewtab with a link to that bug).

Comment 3

[Shane Hughes [:aminomancer]]

It does seem reasonable to handle pbNewtab separately. So for this bug, we can add a special exclusion for pbNewtab, and potentially remove it later in bug 1855401.

(In reply to Dan Mosedale (:dmosedale, :dmose) from comment #2)

Part of the scope of this bug should be verifying/updating existing docs on firefox-source-docs and experiment.info to make sure they reflect the new state of reality w.r.t. reach (eg all feature-ids or all except pbNewtab with a link to that bug).

Also our reach documentation has moved to the Glean dictionary, so we may wanna update that too. There'll be an edit button at the bottom of the annotations, leading to a github edit prompt.

Comment 4

[Hanna Alemu [:hanna_a]]

Attachment: Bug 1877788 - Record reach for all messaging experiment features

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/757f786bddde
Record reach for all messaging experiment features r=omc-reviewers,aminomancer

Comment 6

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/757f786bddde