Bug 1572500 - Count internal @search keyword searches separately from other urlbar searches in the SEARCH_COUNTS histogram. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D42569

--HG--
extra : moz-landing-system : lando

browser/modules/BrowserUsageTelemetry.jsm

@@ -525,10 +525,11 @@ let BrowserUsageTelemetry = {
       return;
     }
 
-    const isOneOff = !!details.isOneOff;
-    const countId = getSearchEngineId(engine) + "." + source;
+    const countIdPrefix = getSearchEngineId(engine) + ".";
+    const countIdSource = countIdPrefix + source;
+    let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
 
-    if (isOneOff) {
+    if (details.isOneOff) {
       if (!KNOWN_ONEOFF_SOURCES.includes(source)) {
         // Silently drop the error if this bogus call
         // came from 'urlbar' or 'searchbar'. They're
@@ -536,9 +537,7 @@ let BrowserUsageTelemetry = {
         // code paths because they want to record the search
         // in SEARCH_COUNTS.
         if (["urlbar", "searchbar"].includes(source)) {
-          Services.telemetry
-            .getKeyedHistogramById("SEARCH_COUNTS")
-            .add(countId);
+          histogram.add(countIdSource);
           return;
         }
         throw new Error("Unknown source for one-off search: " + source);
@@ -547,15 +546,15 @@ let BrowserUsageTelemetry = {
       if (!KNOWN_SEARCH_SOURCES.includes(source)) {
         throw new Error("Unknown source for search: " + source);
       }
-      let histogram = Services.telemetry.getKeyedHistogramById("SEARCH_COUNTS");
-      histogram.add(countId);
-
       if (
         details.alias &&
         engine.wrappedJSObject._internalAliases.includes(details.alias)
       ) {
-        let aliasCountId = getSearchEngineId(engine) + ".alias";
-        histogram.add(aliasCountId);
+        // This search uses an internal @search keyword.  Record the source as
+        // "alias", not "urlbar".
+        histogram.add(countIdPrefix + "alias");
+      } else {
+        histogram.add(countIdSource);
       }
     }
 

browser/modules/test/browser/browser_UsageTelemetry_urlbar.js

@@ -191,7 +191,8 @@ add_task(async function test_simpleQuery() {
     "This search must only increment one entry in the scalar."
   );
 
-  // Make sure SEARCH_COUNTS contains identical values.
+  // SEARCH_COUNTS should be incremented, but only the urlbar source since an
+  // internal @search keyword was not used.
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
     "other-MozSearch.urlbar",
@@ -287,7 +288,8 @@ add_task(async function test_searchAlias() {
     "This search must only increment one entry in the scalar."
   );
 
-  // Make sure SEARCH_COUNTS contains identical values.
+  // SEARCH_COUNTS should be incremented, but only the urlbar source since an
+  // internal @search keyword was not used.
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
     "other-MozSearch.urlbar",
@@ -354,10 +356,12 @@ add_task(async function test_internalSearchAlias() {
   EventUtils.synthesizeKey("KEY_Enter");
   await p;
 
+  // SEARCH_COUNTS should be incremented, but only the alias source since an
+  // internal @search keyword was used.
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
     "other-MozSearch.urlbar",
-    1
+    undefined
   );
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
@@ -374,7 +378,7 @@ add_task(async function test_internalSearchAlias() {
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
     "other-MozSearch.urlbar",
-    2
+    undefined
   );
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
@@ -435,7 +439,8 @@ add_task(async function test_oneOff_enter() {
     "This search must only increment one entry in the scalar."
   );
 
-  // Make sure SEARCH_COUNTS contains identical values.
+  // SEARCH_COUNTS should be incremented, but only the urlbar source since an
+  // internal @search keyword was not used.
   TelemetryTestUtils.assertKeyedHistogramSum(
     search_hist,
     "other-MozSearch.urlbar",
@@ -604,7 +609,7 @@ add_task(async function test_suggestion_click() {
       "This search must only increment one entry in the scalar."
     );
 
-    // Make sure SEARCH_COUNTS contains identical values.
+    // SEARCH_COUNTS should be incremented.
     let searchEngineId = "other-" + engine.name;
     TelemetryTestUtils.assertKeyedHistogramSum(
       search_hist,

toolkit/components/telemetry/Histograms.json

@@ -9186,8 +9186,8 @@
     "keyed": true,
     "releaseChannelCollection": "opt-out",
     "alert_emails": ["adw@mozilla.com", "loines@mozilla.com"],
-    "bug_numbers": [1089670, 1475571, 1482158, 1499193, 1545172],
-    "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>.alias For in content searches, the format is <provider>.in-content:[sap|sap-follow-on|organic]:[code|none]",
+    "bug_numbers": [1089670, 1475571, 1482158, 1499193, 1545172, 1572500],
+    "description": "Records search counts for search access points and in-content searches. For search access points in general, the format is: <engine-name>.<search-access-point> For the urlbar when an internal @engine shortcut is used, the format is: <engine-name>.alias For in-content searches, the format is <provider>.in-content:[sap|sap-follow-on|organic]:[code|none]",
     "record_into_store": ["main", "pre-account"]
   },
   "SEARCH_SERVICE_INIT_MS": {