Bug 1499193 - Add new SEARCH_COUNTS telemetry for internal search engine alias usage. r=mkaply a=jcristau
authorDrew Willcoxon <adw@mozilla.com>
Sat, 03 Nov 2018 16:47:16 +0000
changeset 501244 d3724480adee4f9b6a1a0d1bcbe3c4608fa57dbe
parent 501243 f4a229e163c6ab284c471afda36f8e381c79270c
child 501245 f3625d08b4c0e1ca4dd8c182a398afb9009b639a
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkaply, jcristau
bugs1499193
milestone64.0
Bug 1499193 - Add new SEARCH_COUNTS telemetry for internal search engine alias usage. r=mkaply a=jcristau Modify `BrowserUsageTelemetry.recordSearch` to take an alias instead of the bool `isAlias`. If an alias is given and it's an internal alias of the given engine, then increment a new `"engineName.@engine.source"` key under the `SEARCH_COUNTS` histogram -- in addition to incrementing the usual `"engineName.source"` key under that same histogram. Differential Revision: https://phabricator.services.mozilla.com/D10633 * * * Bug 1499193 - Follow-up: Update metadata for the SEARCH_COUNTS keyed histogram. r=mkaply Differential Revision: https://phabricator.services.mozilla.com/D11382
browser/base/content/urlbarBindings.xml
browser/modules/BrowserUsageTelemetry.jsm
browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
browser/modules/test/browser/head.js
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/histogram-whitelists.json
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -750,17 +750,17 @@ file, You can obtain one at http://mozil
                 break;
               case "searchengine":
                 if (selectedOneOff && selectedOneOff.engine) {
                   // Replace the engine with the selected one-off engine.
                   action.params.engineName = selectedOneOff.engine.name;
                 }
                 const actionDetails = {
                   isSuggestion: !!action.params.searchSuggestion,
-                  isAlias: !!action.params.alias,
+                  alias: action.params.alias,
                 };
                 [url, postData] = this._parseAndRecordSearchEngineLoad(
                   action.params.engineName,
                   action.params.searchSuggestion || action.params.searchQuery,
                   event,
                   where,
                   openUILinkParams,
                   actionDetails
--- a/browser/modules/BrowserUsageTelemetry.jsm
+++ b/browser/modules/BrowserUsageTelemetry.jsm
@@ -414,18 +414,18 @@ let BrowserUsageTelemetry = {
    * @param {String} source
    *        Where the search originated from. See KNOWN_SEARCH_SOURCES for allowed
    *        values.
    * @param {Object} [details] Options object.
    * @param {Boolean} [details.isOneOff=false]
    *        true if this event was generated by a one-off search.
    * @param {Boolean} [details.isSuggestion=false]
    *        true if this event was generated by a suggested search.
-   * @param {Boolean} [details.isAlias=false]
-   *        true if this event was generated by a search using an alias.
+   * @param {Boolean} [details.alias=null]
+   *        The search engine alias used in the search, if any.
    * @param {Object} [details.type=null]
    *        The object describing the event that triggered the search.
    * @throws if source is not in the known sources list.
    */
   recordSearch(tabbrowser, engine, source, details = {}) {
     if (!shouldRecordSearchCount(tabbrowser)) {
       return;
     }
@@ -445,17 +445,25 @@ let BrowserUsageTelemetry = {
           return;
         }
         throw new Error("Unknown source for one-off search: " + source);
       }
     } else {
       if (!KNOWN_SEARCH_SOURCES.includes(source)) {
         throw new Error("Unknown source for search: " + source);
       }
-      Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS").add(countId);
+      let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
+      histogram.add(countId);
+
+      if (details.alias &&
+          engine.wrappedJSObject._internalAliases.includes(details.alias)) {
+        let aliasCountId =
+          [getSearchEngineId(engine), details.alias, source].join(".");
+        histogram.add(aliasCountId);
+      }
     }
 
     // Dispatch the search signal to other handlers.
     this._handleSearchAction(engine, source, details);
   },
 
   _recordSearch(engine, source, action = null) {
     let scalarKey = action ? "search_" + action : "search";
@@ -517,17 +525,17 @@ let BrowserUsageTelemetry = {
       return;
     }
 
     // The search was not a one-off. It was a search with the default search engine.
     if (details.isSuggestion) {
       // It came from a suggested search, so count it as such.
       this._recordSearch(engine, sourceName, "suggestion");
       return;
-    } else if (details.isAlias) {
+    } else if (details.alias) {
       // This one came from a search that used an alias.
       this._recordSearch(engine, sourceName, "alias");
       return;
     }
 
     // The search signal was generated by typing something and pressing enter.
     this._recordSearch(engine, sourceName, "enter");
   },
--- a/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
+++ b/browser/modules/test/browser/browser_UsageTelemetry_urlbar.js
@@ -104,16 +104,19 @@ add_task(async function setup() {
   Services.search.addEngineWithDetails("MozSearch", "", "mozalias", "", "GET",
                                        "http://example.com/?q={searchTerms}");
 
   // Make it the default search engine.
   let engine = Services.search.getEngineByName("MozSearch");
   let originalEngine = Services.search.currentEngine;
   Services.search.currentEngine = engine;
 
+  // Give it some mock internal aliases.
+  engine.wrappedJSObject.__internalAliases = ["@mozaliasfoo", "@mozaliasbar"];
+
   // And the first one-off engine.
   Services.search.moveEngine(engine, 0);
 
   // Enable search suggestions in the urlbar.
   let suggestionsEnabled = Services.prefs.getBoolPref(SUGGEST_URLBAR_PREF);
   Services.prefs.setBoolPref(SUGGEST_URLBAR_PREF, true);
 
   // Enable the urlbar one-off buttons.
@@ -170,16 +173,19 @@ add_task(async function test_simpleQuery
   // Check if the scalars contain the expected values.
   const scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true, false);
   checkKeyedScalar(scalars, SCALAR_URLBAR, "search_enter", 1);
   Assert.equal(Object.keys(scalars[SCALAR_URLBAR]).length, 1,
                "This search must only increment one entry in the scalar.");
 
   // Make sure SEARCH_COUNTS contains identical values.
   checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.mozalias.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasfoo.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasbar.urlbar", undefined);
 
   // Also check events.
   let events = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false);
   events = (events.parent || []).filter(e => e[1] == "navigation" && e[2] == "search");
   checkEvents(events, [["navigation", "search", "urlbar", "enter", {engine: "other-MozSearch"}]]);
 
   // Check the histograms as well.
   let resultIndexes = resultIndexHist.snapshot();
@@ -224,16 +230,19 @@ add_task(async function test_searchAlias
   // Check if the scalars contain the expected values.
   const scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true, false);
   checkKeyedScalar(scalars, SCALAR_URLBAR, "search_alias", 1);
   Assert.equal(Object.keys(scalars[SCALAR_URLBAR]).length, 1,
                "This search must only increment one entry in the scalar.");
 
   // Make sure SEARCH_COUNTS contains identical values.
   checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.mozalias.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasfoo.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasbar.urlbar", undefined);
 
   // Also check events.
   let events = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false);
   events = (events.parent || []).filter(e => e[1] == "navigation" && e[2] == "search");
   checkEvents(events, [["navigation", "search", "urlbar", "alias", {engine: "other-MozSearch"}]]);
 
   // Check the histograms as well.
   let resultIndexes = resultIndexHist.snapshot();
@@ -252,16 +261,46 @@ add_task(async function test_searchAlias
   let resultMethods = resultMethodHist.snapshot();
   checkHistogramResults(resultMethods,
     URLBAR_SELECTED_RESULT_METHODS.enter,
     "FX_URLBAR_SELECTED_RESULT_METHOD");
 
   BrowserTestUtils.removeTab(tab);
 });
 
+add_task(async function test_internalSearchAlias() {
+  let search_hist = getAndClearKeyedHistogram("SEARCH_COUNTS");
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank");
+
+  info("Search using an internal search alias.");
+  let p = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  await searchInAwesomebar("@mozaliasfoo query");
+  EventUtils.synthesizeKey("KEY_Enter");
+  await p;
+
+  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.mozalias.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasfoo.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasbar.urlbar", undefined);
+
+  info("Search using the other internal search alias.");
+  p = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  await searchInAwesomebar("@mozaliasbar query");
+  EventUtils.synthesizeKey("KEY_Enter");
+  await p;
+
+  checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 2);
+  checkKeyedHistogram(search_hist, "other-MozSearch.mozalias.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasfoo.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasbar.urlbar", 1);
+
+  BrowserTestUtils.removeTab(tab);
+});
+
 // Performs a search using the first result, a one-off button, and the Return
 // (Enter) key.
 add_task(async function test_oneOff_enter() {
   Services.telemetry.clearScalars();
   Services.telemetry.clearEvents();
 
   let resultIndexHist = getAndClearHistogram("FX_URLBAR_SELECTED_RESULT_INDEX");
   let resultTypeHist = getAndClearHistogram("FX_URLBAR_SELECTED_RESULT_TYPE");
@@ -283,16 +322,19 @@ add_task(async function test_oneOff_ente
   // Check if the scalars contain the expected values.
   const scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true, false);
   checkKeyedScalar(scalars, SCALAR_URLBAR, "search_oneoff", 1);
   Assert.equal(Object.keys(scalars[SCALAR_URLBAR]).length, 1,
                "This search must only increment one entry in the scalar.");
 
   // Make sure SEARCH_COUNTS contains identical values.
   checkKeyedHistogram(search_hist, "other-MozSearch.urlbar", 1);
+  checkKeyedHistogram(search_hist, "other-MozSearch.mozalias.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasfoo.urlbar", undefined);
+  checkKeyedHistogram(search_hist, "other-MozSearch.@mozaliasbar.urlbar", undefined);
 
   // Also check events.
   let events = Services.telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false);
   events = (events.parent || []).filter(e => e[1] == "navigation" && e[2] == "search");
   checkEvents(events, [["navigation", "search", "urlbar", "oneoff", {engine: "other-MozSearch"}]]);
 
   // Check the histograms as well.
   let resultIndexes = resultIndexHist.snapshot();
--- a/browser/modules/test/browser/head.js
+++ b/browser/modules/test/browser/head.js
@@ -122,16 +122,20 @@ function getAndClearKeyedHistogram(name)
 }
 
 
 /**
  * Check that the keyed histogram contains the right value.
  */
 function checkKeyedHistogram(h, key, expectedValue) {
   const snapshot = h.snapshot();
+  if (expectedValue === undefined) {
+    Assert.ok(!(key in snapshot), `The histogram must not contain ${key}.`);
+    return;
+  }
   Assert.ok(key in snapshot, `The histogram must contain ${key}.`);
   Assert.equal(snapshot[key].sum, expectedValue, `The key ${key} must contain ${expectedValue}.`);
 }
 
 /**
  * Return the scalars from the parent-process.
  */
 function getParentProcessScalars(aChannel, aKeyed = false, aClear = false) {
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7958,17 +7958,19 @@
     "description": "Time elapsed between before responding to Slow Add-on Warning UI (ms). Not updated if the user doesn't respond at all."
   },
   "SEARCH_COUNTS": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "count",
     "keyed": true,
     "releaseChannelCollection": "opt-out",
-    "description": "Records search counts for search access points and in content searches. For search access points, the format is: <engine-name>.<search-access-point> For in content searches, the format is <provider>.in-content:[sap|sap-follow-on|organic]:[code|none]"
+    "alert_emails": ["adw@mozilla.com"],
+    "bug_numbers": [1089670, 1482158, 1499193],
+    "description": "Records search counts for search access points and in content searches. For search access points in general and for the urlbar when an internal @engine shorcut is not used, the format is: <engine-name>.<search-access-point> For the urlbar when an internal @engine shortcut is used, the format is: <engine-name>.<@engine>.urlbar For in content searches, the format is <provider>.in-content:[sap|sap-follow-on|organic]:[code|none]"
   },
   "SEARCH_RESET_RESULT": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["fqueze@mozilla.com"],
     "bug_numbers": [1203168],
     "expires_in_version": "65",
     "kind": "enumerated",
     "n_values": 5,
--- a/toolkit/components/telemetry/histogram-whitelists.json
+++ b/toolkit/components/telemetry/histogram-whitelists.json
@@ -392,17 +392,16 @@
     "PROCESS_CRASH_SUBMIT_SUCCESS",
     "PWMGR_MANAGE_DELETED_ALL",
     "PWMGR_MANAGE_SORTED",
     "PWMGR_NUM_HTTPAUTH_PASSWORDS",
     "PWMGR_PASSWORD_INPUT_IN_FORM",
     "PWMGR_USERNAME_PRESENT",
     "REQUESTS_OF_ORIGINAL_CONTENT",
     "SAFE_MODE_USAGE",
-    "SEARCH_COUNTS",
     "SERVICE_WORKER_CONTROLLED_DOCUMENTS",
     "SERVICE_WORKER_LIFE_TIME",
     "SERVICE_WORKER_REGISTRATIONS",
     "SERVICE_WORKER_REGISTRATION_LOADING",
     "SERVICE_WORKER_REQUEST_PASSTHROUGH",
     "SERVICE_WORKER_SPAWN_ATTEMPTS",
     "SERVICE_WORKER_UPDATED",
     "SERVICE_WORKER_WAS_SPAWNED",
@@ -978,17 +977,16 @@
     "PWMGR_PASSWORD_INPUT_IN_FORM",
     "PWMGR_USERNAME_PRESENT",
     "RANGE_CHECKSUM_ERRORS",
     "READER_MODE_DOWNLOAD_RESULT",
     "READER_MODE_PARSE_RESULT",
     "REFRESH_DRIVER_TICK",
     "REQUESTS_OF_ORIGINAL_CONTENT",
     "SAFE_MODE_USAGE",
-    "SEARCH_COUNTS",
     "SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT",
     "SEARCH_SERVICE_COUNTRY_FETCH_RESULT",
     "SEARCH_SERVICE_COUNTRY_FETCH_TIME_MS",
     "SEARCH_SERVICE_COUNTRY_TIMEOUT",
     "SEARCH_SERVICE_INIT_SYNC",
     "SEARCH_SERVICE_NONUS_COUNTRY_MISMATCHED_PLATFORM_OSX",
     "SEARCH_SERVICE_NONUS_COUNTRY_MISMATCHED_PLATFORM_WIN",
     "SEARCH_SERVICE_US_COUNTRY_MISMATCHED_PLATFORM_OSX",