Create an "activity-stream-newtab" source for Search Telemetry

[sarracini]

Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1381460
When we switched over our search component to recycle the existing one in about:newtab and about:home (https://github.com/mozilla/activity-stream/blob/master/system-addon/content-src/components/Search/Search.jsx#L26-L32) we see that we pass in "newtab" as the HealthReportKey. We did this in order to fix a failing test where we were originally passing in "activity" as a source, which wasn't recognized in BrowserUsageTelemetry.jsm as a valid source, and therefore we never added the appropriate Telemetry Probe, and failed some tests. The concern @dmose and I had was that by re-using the "newtab" source, we essentially mix up the data for searches that come in from tiles newtab with activity stream's newtab, since their sources are the same. At the time, @dmose and I weren't sure about how much engineering effort it would take to create a new source, so we decided to leave it as "newtab" for now.

After chatting with the folks in #fx-metrics on slack about this, we were told we probably need to add our own "activity-stream-newtab" source, since we definitely want to separate out the experience of activity stream vs current newtab. Georg Fritzsche is the guy we want to ask about what kind of engineering work is involved for adding a new source. At a minimum, we guess we have to add it to this list: https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUsageTelemetry.jsm#42, and possibly handle it differently here: https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUsageTelemetry.jsm#444, but we should ask Georg about what other work needs to be done to add it properly.

So, does this block preff-ing on in 56 for some users? Quoted by David Zeber on slack:

dzeber [4:55 PM] :
i’d recommend using the new source string prior to preffing on
that said, it’s not a huge issue for nightly
i’m not sure of how the engineering effort balances out on this
but if it’s only the strings that need to be changed/added to whitelists, it’s probably best
i would also claim that the scalars/events are lower priority (not sure if we’re using this data regularly at the moment)
so they could potentially wait until beta/release

cc @Mardak @emtwo @k88hudson

[emtwo]

Yea I would agree with dzeber that we probably don't want to mess with search telemetry for new tab since it could affect analysis being done on that.

On the other hand, do we need UT telemetry for Activity Stream search at all? If it's any easier to just not send UT for AS search perhaps that could be better since we are collecting metrics via onyx

[sarracini]

In that case (which is actually probably preferable) we could just add the source in BrowserUsageTelemetry so it gets picked up and then literally just do nothing when we get that action

[dmose]

It seems like we'd have to talk to the metrics folks before turning off that data flow -- I suspect they're comparing search sources across a whole variety of things, and they probably want to continue to do that even after tiles goes away.

[piatra]

There is now a bugzilla bug for this in order to get more info from people. I'm sure there will be a discussion around this.

[Mardak]

This will need to handle
Backport Bug 1381460 - Add telemetry search event for Activity Stream. f=florian, r=Dexter, data-r=bsmedberg #2954
https://bugzilla.mozilla.org/show_bug.cgi?id=1381460
https://hg.mozilla.org/integration/mozilla-inbound/rev/839d8a60a2ea

[piatra]

I've had a new idea for this where I would disable preloading in the test setup so that we don't have to close the tab to open a new one to ensure we don't get a preloaded Activity Stream tab.
Unfortunately that still did not work with 1 test failure on Linux debug (out of 5).
Now I'm disabling Activity Stream in the test setup phase and restoring the tests to what they were originally doing and see how that works out https://treeherder.mozilla.org/#/jobs?repo=try&revision=8985169ff8481ce101318a1b52e9a1a9ea308fa7.

Update: that second approach also doesn't work.

[Mardak]

I'm assuming this was closed because this is no longer desired?