Bug 1954500: Fix tab group telemetry to be in line with spec r=sthompson,tabbrowser-reviewers

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

browser/components/tabbrowser/metrics.yaml

@@ -79,91 +79,6 @@ browser.engagement:
         type: quantity
     expires: never
 
-  tab_group_create:
-    type: event
-    description: >
-      Recorded when the user creates a new tab group via
-      the tab context menu or through drag and drop.
-    notification_emails:
-      - dao@mozilla.com
-      - jswinarton@mozilla.com
-    bugs:
-      - https://bugzil.la/1938423
-    data_reviews:
-      - https://bugzil.la/1938423
-    data_sensitivity:
-      - interaction
-    extra_keys:
-      source:
-        description: The means by which the tab group was created
-        type: string
-      tabs:
-        description: The number of tabs in the group when it was created
-        type: quantity
-      layout:
-        description: The layout of the tab strip when the group was created (either "horizontal" or "vertical")
-        type: string
-      id:
-        description: The ID of the created tab group
-        type: string
-    expires: never
-
-  tab_group_expand_or_collapse:
-    type: event
-    description: >
-      Recorded when a tab group is expanded or collapsed.
-    notification_emails:
-      - dao@mozilla.com
-      - jswinarton@mozilla.com
-    bugs:
-      - https://bugzil.la/1938397
-    data_reviews:
-      - https://bugzil.la/1938397
-    data_sensitivity:
-      - interaction
-    extra_keys:
-      total_collapsed:
-        description: The total number of active groups in all windows that are collapsed
-        type: quantity
-      total_expanded:
-        description: The total number of active groups in all windows that are expanded
-        type: quantity
-    expires: never
-
-  tab_group_modify:
-    type: event
-    description: >
-      Recorded when the number of tabs in a tab group are modified.
-    notification_emails:
-      - dao@mozilla.com
-      - jswinarton@mozilla.com
-    bugs:
-      - https://bugzil.la/1938397
-    data_reviews:
-      - https://bugzil.la/1938397
-    data_sensitivity:
-      - interaction
-    extra_keys:
-      tabs_per_active_group_max:
-        description: The number of open tabs in the largest tab group in any window
-        type: quantity
-      tabs_per_active_group_min:
-        description: The number of open tabs in the smallest tab group in any window
-        type: quantity
-      tabs_per_active_group_median:
-        description: The median number of open tabs in tab groups in all windows
-        type: quantity
-      tabs_per_active_group_average:
-        description: The average number of open tabs in tab groups in all windows
-        type: quantity
-      tabs_inside_groups:
-        description: The total number of tabs within tab groups in all windows
-        type: quantity
-      tabs_outside_groups:
-        description: The total number of tabs not in groups in all windows
-        type: quantity
-    expires: never
-
 browser.ui.interaction:
   all_tabs_panel_dragstart_tab_event_count:
     type: counter
@@ -288,3 +203,94 @@ browser.ml.interaction:
     expires: never
     no_lint:
       - COMMON_PREFIX
+
+tabgroup:
+  create_group:
+    type: event
+    description: >
+      Recorded when the user creates a new tab group via
+      the tab context menu or through drag and drop.
+    notification_emails:
+      - dao@mozilla.com
+      - jswinarton@mozilla.com
+    bugs:
+      - https://bugzil.la/1938423
+      - https://bugzil.la/1954500
+    data_reviews:
+      - https://bugzil.la/1938423
+      - https://bugzil.la/1954500
+    data_sensitivity:
+      - interaction
+    extra_keys:
+      source:
+        description: The means by which the tab group was created
+        type: string
+      tabs:
+        description: The number of tabs in the group when it was created
+        type: quantity
+      layout:
+        description: The layout of the tab strip when the group was created (either "horizontal" or "vertical")
+        type: string
+      id:
+        description: The ID of the created tab group
+        type: string
+    expires: never
+
+  active_groups:
+    type: labeled_quantity
+    description: >
+      Records the number of groups present in the tab bar, split by expanded or collapsed.
+    notification_emails:
+      - dao@mozilla.com
+      - jswinarton@mozilla.com
+    bugs:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    data_reviews:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    expires: never
+    unit: tab groups
+    labels:
+      - expanded
+      - collapsed
+
+  tabs_per_active_group:
+    type: labeled_quantity
+    description: >
+      Records statistics about the number of tabs per active group: max, median, average and min.
+    notification_emails:
+      - dao@mozilla.com
+      - jswinarton@mozilla.com
+    bugs:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    data_reviews:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    expires: never
+    unit: tabs
+    labels:
+      - max
+      - median
+      - average
+      - min
+
+  tab_count_in_groups:
+    type: labeled_quantity
+    description: >
+      Records the latest number of tabs in the tab bar, split by being inside a group or outside.
+    notification_emails:
+      - dao@mozilla.com
+      - jswinarton@mozilla.com
+    bugs:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    data_reviews:
+      - https://bugzil.la/1938397
+      - https://bugzil.la/1954500
+    expires: never
+    unit: tabs
+    labels:
+      - inside
+      - outside

browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_telemetry.js

@@ -10,9 +10,7 @@ let resetTelemetry = async () => {
 add_task(async function test_tabGroupTelemetry() {
   await resetTelemetry();
 
-  let tabGroupCreateTelemetry,
+  let tabGroupCreateTelemetry;
-    tabGroupModifyTelemetry,
-    tabGroupCollapseTelemetry;
 
   let group1tab = BrowserTestUtils.addTab(gBrowser, "https://example.com");
   await BrowserTestUtils.browserLoaded(group1tab.linkedBrowser);
@@ -24,15 +22,13 @@ add_task(async function test_tabGroupTelemetry() {
   gBrowser.tabGroupMenu.close();
 
   await BrowserTestUtils.waitForCondition(() => {
-    tabGroupCreateTelemetry =
+    tabGroupCreateTelemetry = Glean.tabgroup.createGroup.testGetValue();
-      Glean.browserEngagement.tabGroupCreate.testGetValue();
-    tabGroupModifyTelemetry =
-      Glean.browserEngagement.tabGroupModify.testGetValue();
     return (
       tabGroupCreateTelemetry?.length == 1 &&
-      tabGroupModifyTelemetry?.length == 1
+      Glean.tabgroup.tabCountInGroups.inside.testGetValue() !== null &&
+      Glean.tabgroup.tabsPerActiveGroup.average.testGetValue() !== null
     );
-  }, "Wait for tabGroupCreate and tabGroupModify events after creating a single tab group");
+  }, "Wait for createGroup and at least one metric from the tabCountInGroups and tabsPerActiveGroup to be set");
 
   Assert.deepEqual(
     tabGroupCreateTelemetry[0].extra,
@@ -44,17 +40,36 @@ add_task(async function test_tabGroupTelemetry() {
     },
     "tabGroupCreate event extra_keys has correct values after tab group create"
   );
-  Assert.deepEqual(
+
-    tabGroupModifyTelemetry[0].extra,
+  Assert.equal(
-    {
+    Glean.tabgroup.tabCountInGroups.inside.testGetValue(),
-      tabs_per_active_group_min: "1",
+    1,
-      tabs_per_active_group_max: "1",
+    "tabCountInGroups.inside has correct value"
-      tabs_inside_groups: "1",
+  );
-      tabs_per_active_group_median: "1",
+  Assert.equal(
-      tabs_outside_groups: "1",
+    Glean.tabgroup.tabCountInGroups.outside.testGetValue(),
-      tabs_per_active_group_average: "1",
+    1,
-    },
+    "tabCountInGroups.outside has correct value"
-    "tabGroupModify event extra_keys has correct values after tab group create"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.median.testGetValue(),
+    1,
+    "tabsPerActiveGroup.median has correct value"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.average.testGetValue(),
+    1,
+    "tabsPerActiveGroup.average has correct value"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.max.testGetValue(),
+    1,
+    "tabsPerActiveGroup.max has correct value"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.min.testGetValue(),
+    1,
+    "tabsPerActiveGroup.min has correct value"
   );
 
   await resetTelemetry();
@@ -75,23 +90,43 @@ add_task(async function test_tabGroupTelemetry() {
   gBrowser.tabGroupMenu.close();
 
   await BrowserTestUtils.waitForCondition(() => {
-    tabGroupModifyTelemetry =
+    return (
-      Glean.browserEngagement.tabGroupModify.testGetValue();
+      Glean.tabgroup.tabCountInGroups.inside.testGetValue() !== null &&
-    return tabGroupModifyTelemetry?.length == 1;
+      Glean.tabgroup.tabsPerActiveGroup.average.testGetValue() !== null
-  }, "Wait for tabGroupModify event after adding a new tab group");
+    );
+  }, "Wait for at least one metric from the tabCountInGroups and tabsPerActiveGroup to be set after adding a new tab group");
 
-  Assert.deepEqual(
+  Assert.equal(
-    tabGroupModifyTelemetry[0].extra,
+    Glean.tabgroup.tabCountInGroups.inside.testGetValue(),
-    {
+    4,
-      tabs_per_active_group_max: "3",
+    "tabCountInGroups.inside has correct value after adding a new tab group"
-      tabs_per_active_group_min: "1",
-      tabs_per_active_group_median: "2",
-      tabs_per_active_group_average: "2",
-      tabs_inside_groups: "4",
-      tabs_outside_groups: "1",
-    },
-    "tabGroupModify event extra_keys has correct values after adding a new tab group"
   );
+  Assert.equal(
+    Glean.tabgroup.tabCountInGroups.outside.testGetValue(),
+    1,
+    "tabCountInGroups.outside has correct value after adding a new tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.median.testGetValue(),
+    2,
+    "tabsPerActiveGroup.median has correct value after adding a new tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.average.testGetValue(),
+    2,
+    "tabsPerActiveGroup.average has correct value after adding a new tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.max.testGetValue(),
+    3,
+    "tabsPerActiveGroup.max has correct value after adding a new tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.min.testGetValue(),
+    1,
+    "tabsPerActiveGroup.min has correct value after adding a new tab group"
+  );
+
   await resetTelemetry();
 
   let newTabInGroup2 = BrowserTestUtils.addTab(gBrowser, "https://example.com");
@@ -100,42 +135,62 @@ add_task(async function test_tabGroupTelemetry() {
   group2.addTabs([newTabInGroup2]);
 
   await BrowserTestUtils.waitForCondition(() => {
-    tabGroupModifyTelemetry =
+    return (
-      Glean.browserEngagement.tabGroupModify.testGetValue();
+      Glean.tabgroup.tabCountInGroups.inside.testGetValue() !== null &&
-    return tabGroupModifyTelemetry?.length == 1;
+      Glean.tabgroup.tabsPerActiveGroup.average.testGetValue() !== null
-  }, "Wait for tabGroupModify event after modifying a tab group");
+    );
+  }, "Wait for at least one metric from the tabCountInGroups and tabsPerActiveGroup to be set after modifying a tab group");
 
-  Assert.deepEqual(
+  Assert.equal(
-    tabGroupModifyTelemetry[0].extra,
+    Glean.tabgroup.tabCountInGroups.inside.testGetValue(),
-    {
+    5,
-      tabs_per_active_group_max: "4",
+    "tabCountInGroups.inside has correct value after modifying a tab group"
-      tabs_per_active_group_min: "1",
-      tabs_per_active_group_median: "2.5",
-      tabs_per_active_group_average: "2.5",
-      tabs_inside_groups: "5",
-      tabs_outside_groups: "1",
-    },
-    "tabGroupModify event extra_keys has correct values after changing the number of tabs in groups"
   );
-  await Services.fog.testFlushAllChildren();
+  Assert.equal(
-  Services.fog.testResetFOG();
+    Glean.tabgroup.tabCountInGroups.outside.testGetValue(),
+    1,
+    "tabCountInGroups.outside has correct value after modifying a tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.median.testGetValue(),
+    2,
+    "tabsPerActiveGroup.median has correct value after modifying a tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.average.testGetValue(),
+    2,
+    "tabsPerActiveGroup.average has correct value after modifying a tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.max.testGetValue(),
+    4,
+    "tabsPerActiveGroup.max has correct value after modifying a tab group"
+  );
+  Assert.equal(
+    Glean.tabgroup.tabsPerActiveGroup.min.testGetValue(),
+    1,
+    "tabsPerActiveGroup.min has correct value after modifying a tab group"
+  );
+
+  await resetTelemetry();
 
   group2.collapsed = true;
 
   await BrowserTestUtils.waitForCondition(() => {
-    tabGroupCollapseTelemetry =
+    return Glean.tabgroup.activeGroups.collapsed.testGetValue() !== null;
-      Glean.browserEngagement.tabGroupExpandOrCollapse.testGetValue();
+  }, "Wait for the activeGroups metric to be set after collapsing a tab group");
-    return tabGroupCollapseTelemetry?.length;
-  }, "Wait for tabGroupExpandOrCollapseEvent after tab group collapse");
 
-  Assert.deepEqual(
+  Assert.equal(
-    tabGroupCollapseTelemetry[0].extra,
+    Glean.tabgroup.activeGroups.collapsed.testGetValue(),
-    {
+    1,
-      total_collapsed: "1",
+    "activeGroups.collapsed has correct value after collapsing a tab group"
-      total_expanded: "1",
-    },
-    "tabGroupExpandOrCollapse event extra_keys has correct values"
   );
+  Assert.equal(
+    Glean.tabgroup.activeGroups.expanded.testGetValue(),
+    1,
+    "activeGroups.collapsed has correct value after collapsing a tab group"
+  );
+
   await resetTelemetry();
 
   await removeTabGroup(group1);

browser/modules/BrowserUsageTelemetry.sys.mjs

@@ -588,7 +588,6 @@ export let BrowserUsageTelemetry = {
         break;
       case "TabGroupCreate":
         this._onTabGroupCreate(event);
-        this._onTabGroupChange();
         break;
       case "TabGrouped":
       case "TabUngrouped":
@@ -1220,7 +1219,7 @@ export let BrowserUsageTelemetry = {
 
   _onTabGroupCreate(event) {
     if (event.detail.isUserCreated) {
-      Glean.browserEngagement.tabGroupCreate.record({
+      Glean.tabgroup.createGroup.record({
         id: event.target.id,
         layout: lazy.sidebarVerticalTabs ? "vertical" : "horizontal",
         source: event.detail.telemetryUserCreateSource,
@@ -1240,6 +1239,10 @@ export let BrowserUsageTelemetry = {
   _doOnTabGroupChange() {
     let totalTabs = 0;
     let totalTabsInGroups = 0;
+    let max = 0;
+    let min = 0;
+    let average = 0;
+    let median = 0;
 
     // Used for calculation of average and median
     let tabGroupLengths = [];
@@ -1253,29 +1256,27 @@ export let BrowserUsageTelemetry = {
     }
 
     const tabGroupCount = tabGroupLengths.length;
-    if (!tabGroupCount) {
+    if (tabGroupCount) {
-      // If this event was fired because the last tab group was closed, do not
+      tabGroupLengths.sort((a, b) => a - b);
-      // fire a metric.
+      const middleIndex = Math.floor(tabGroupCount / 2);
-      return;
-    }
 
-    tabGroupLengths.sort((a, b) => a - b);
+      max = Math.max(...tabGroupLengths);
-    const middleIndex = Math.floor(tabGroupCount / 2);
+      min = Math.min(...tabGroupLengths);
-
+      median =
-    const data = {
-      tabs_per_active_group_median:
         tabGroupCount % 2 == 0
           ? (tabGroupLengths[middleIndex - 1] + tabGroupLengths[middleIndex]) /
             2
-          : tabGroupLengths[middleIndex],
+          : tabGroupLengths[middleIndex];
-      tabs_per_active_group_average:
+      average = tabGroupLengths.reduce((a, b) => a + b, 0) / tabGroupCount;
-        tabGroupLengths.reduce((a, b) => a + b, 0) / tabGroupCount,
+    }
-      tabs_per_active_group_min: Math.min(...tabGroupLengths),
+
-      tabs_per_active_group_max: Math.max(...tabGroupLengths),
+    Glean.tabgroup.tabCountInGroups.inside.set(totalTabsInGroups);
-      tabs_inside_groups: totalTabsInGroups,
+    Glean.tabgroup.tabCountInGroups.outside.set(totalTabs - totalTabsInGroups);
-      tabs_outside_groups: totalTabs - totalTabsInGroups,
+
-    };
+    Glean.tabgroup.tabsPerActiveGroup.median.set(median);
-    Glean.browserEngagement.tabGroupModify.record(data);
+    Glean.tabgroup.tabsPerActiveGroup.average.set(average);
+    Glean.tabgroup.tabsPerActiveGroup.max.set(max);
+    Glean.tabgroup.tabsPerActiveGroup.min.set(min);
   },
 
   _onTabGroupExpandOrCollapse() {
@@ -1297,10 +1298,8 @@ export let BrowserUsageTelemetry = {
       }
     }
 
-    Glean.browserEngagement.tabGroupExpandOrCollapse.record({
+    Glean.tabgroup.activeGroups.collapsed.set(collapsed);
-      total_collapsed: collapsed,
+    Glean.tabgroup.activeGroups.expanded.set(expanded);
-      total_expanded: expanded,
-    });
   },
 
   /**