1743885 - Add latency telemetry for search suggestions

Status: VERIFIED FIXED

(Firefox :: Search, task, P2)

Description

[Drew Willcoxon :adw]

We recently added latency telemetry for the Firefox Suggest server, Merino, in bug 1727799. We'd like to see how Merino's latency compares against Google's search suggestions (or whichever engine the user uses), but we don't currently measure search suggestion latency. And aside from Merino and Firefox Suggest, search suggestion latency seems like potentially useful data anyway.

I think it makes sense to instrument SearchSuggestionController so that all consumers of suggestions in Firefox trigger the telemetry, not only the urlbar, even though AFAICT the urlbar is the only current consumer now that newtab handoff is enabled. The other obvious place is UrlbarProviderSearchSuggestions, but of course that would be urlbar only.

Mark, do you see any problems with adding a new probe to SearchSuggestionController?

Comment 1

[Mark Banner (:standard8)]

I've no problem with using SearchSuggestionController. We already have bandwidth telemetry there, and I don't see an issue with adding more.

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1743885 - Add latency telemetry for search suggestions.

This adds a SEARCH_SUGGESTIONS_LATENCY_MS keyed exponential histogram. Keys
are search engine IDs. There are 50 buckets in the range 0s to 30s, which is the
same as the Merino latency histogram and other HTTP latency histograms. Latency
is always recorded except when the remote fetch is aborted, meaning it's
recorded on success, timeout, and error so that we can measure the true latency
even on timeout and error. That's also the same as the Merino histogram.

Comment 3

[Drew Willcoxon :adw]

Attachment: data-request.md

Data-review request for the new SEARCH_SUGGESTIONS_LATENCY_MS histogram

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 9255403 [details]
data-request.md

Clearing data review request for now. Megan says we should get approval from Trust/legal for Category 3, which makes sense. The reason I considered this as Category 3 is because it records search engine IDs. I started a discussion with Mark about it in the Phabricator revision, and I'll wait to hear back from him before continuing with this.

Comment 5

[Drew Willcoxon :adw]

Attachment: data-request.md v2

This is an updated request doc. After talking with Mark, I see I misunderstood the data recorded by my patch. It's actually category 2 data, not category 3. I had thought my patch records the IDs of non-built-in search engines the user installed, but it actually records "other" for those engines. Only the IDs for built-in engines are recorded.

Comment 6

[Megan McCorquodale]

Comment on attachment 9256276 [details]
data-request.md v2

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?
   Yes, this will be documented in the probe dictionary.

2. Is there a control mechanism that allows the user to turn the data collection on and off?
   Yes, users can opt out of telemetry collection.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?
   Drew will monitor.

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
   Category 2, Interaction Data.

5. Is the data collection request for default-on or default-off?
   Default on.

6. Does the instrumentation include the addition of any new identifiers?
   No new identifiers.

7. Is the data collection covered by the existing Firefox privacy notice?
   Yes.

8. Does there need to be a check-in in the future to determine whether to renew the data?
   No.

9. Does the data collection use a third-party collection tool?
   No.

---

data-review +

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5d1f7579011
Add latency telemetry for search suggestions. r=Standard8

Comment 8

[Sandor Molnar[:smolnar]]

Backed out for causing xpc failures in test_searchSuggest

Backout link: https://hg.mozilla.org/integration/autoland/rev/1148b5b8b76e57dee2fd80b346b1331b854d5f8f

Push with failures

Failure log

INFO -  TEST-START | xpcshell.ini:toolkit/components/search/tests/xpcshell/test_searchSuggest.js
[task 2021-12-22T20:08:31.312Z] 20:08:31  WARNING -  TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/search/tests/xpcshell/test_searchSuggest.js | xpcshell return code: 0
[task 2021-12-22T20:08:31.313Z] 20:08:31     INFO -  TEST-INFO took 8761ms
[task 2021-12-22T20:08:31.313Z] 20:08:31     INFO -  >>>>>>>
[task 2021-12-22T20:08:31.313Z] 20:08:31     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2021-12-22T20:08:31.314Z] 20:08:31     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2021-12-22T20:08:31.314Z] 20:08:31     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2021-12-22T20:08:31.314Z] 20:08:31     INFO -  running event loop
[task 2021-12-22T20:08:31.315Z] 20:08:31     INFO -  xpcshell.ini:toolkit/components/search/tests/xpcshell/test_searchSuggest.js | Starting setup
[task 2021-12-22T20:08:31.315Z] 20:08:31     INFO -  (xpcshell/head.js) | test setup pending (2)
[task 2021-12-22T20:08:31.315Z] 20:08:31     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2021-12-22T20:08:31.316Z] 20:08:31     INFO -  (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2021-12-22T20:08:31.316Z] 20:08:31     INFO -  (xpcshell/head.js) | test setup finished (2)
[task 2021-12-22T20:08:31.317Z] 20:08:31     INFO -  xpcshell.ini:toolkit/components/search/tests/xpcshell/test_searchSuggest.js | Starting add_test_engines
[task 2021-12-22T20:08:31.317Z] 20:08:31     INFO -  (xpcshell/head.js) | test add_test_engines pending (2)
[task 2021-12-22T20:08:31.318Z] 20:08:31     INFO -  PID 12848 | console.debug: SearchService: "addEngine: Adding" "http://localhost:54810/data/engineMaker.sjs?{\"baseURL\":\"http://localhost:54810/data/\",\"name\":\"GET suggestion engine\",\"method\":\"GET\"}"
[task 2021-12-22T20:08:31.318Z] 20:08:31     INFO -  PID 12848 | console.debug: SearchService: "init"

Comment 9

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/jobs?repo=try&revision=2cbd10dd1efc4bc5506c58b186c1ed272ce8e5d0

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9aaf0e3b285a
Add latency telemetry for search suggestions. r=Standard8

Comment 11

[Drew Willcoxon :adw]

STR for QA

1. Type a few things in the urlbar that trigger Google search suggestions
2. Open about:telemetry#keyed-histograms-tab_search=SEARCH_SUGGESTIONS_LATENCY_MS and verify there are values recorded for the SEARCH_SUGGESTIONS_LATENCY_MS keyed histogram under the key "google-b-1-d" (your key may be slightly different, I'm not sure, but it should include "google")
3. Open about:preferences#search and change the default search engine to Bing (or something else)
4. Type a few things in the urlbar that trigger Bing search suggestions
5. Open (or refresh) about:telemetry#keyed-histograms-tab_search=SEARCH_SUGGESTIONS_LATENCY_MS and verify there are values recorded under the key "bing"

Comment 12

[Drew Willcoxon :adw]

Comment on attachment 9255401 [details]
Bug 1743885 - Add latency telemetry for search suggestions.

Beta/Release Uplift Approval Request

• User impact if declined: This would be very useful for the next Firefox Suggest rollout on 96. It will allow us to compare the latency of our Merino server to the latencies of built-in engines like Google and Bing.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see comment 11
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only adds telemetry for measuring the latency of search suggestions. No user-facing changes.
• String changes made/needed:

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/9aaf0e3b285a

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9255401 [details]
Bug 1743885 - Add latency telemetry for search suggestions.

Approved for 96.0b9

Comment 15

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/903476cc874f

Comment 16

[Dianna Smith [:diannaS]]

I would like to note that this was temporarily backed out during build failure investigation and reintroduced for 96.0b9.
Backout link

Re-introduced in:
https://hg.mozilla.org/releases/mozilla-beta/rev/5d7d18dc07f6

Comment 17

[Hani Yacoub, Desktop QA]

Verified as fixed on Windows 10 x64, macOS 11.6 and on Ubuntu 20.04 x64.