Bug 1152733 - Convert all Loop keyed histograms to be enumerated types and opt-out, which helps reporting tremendously. r=vladan, r=dmose, a=lizzard
authorMike de Boer <mdeboer@mozilla.com>
Wed, 06 May 2015 12:55:31 +0200
changeset 267338 e84ae588f726a930872042634616fae409625e23
parent 267337 6d7f28e62afe371e501be4df6c1114de2e78c5a9
child 267339 3459985784d0a2bff504044ac2cdb2b43f97a443
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvladan, dmose, lizzard
bugs1152733
milestone39.0a2
Bug 1152733 - Convert all Loop keyed histograms to be enumerated types and opt-out, which helps reporting tremendously. r=vladan, r=dmose, a=lizzard
browser/components/loop/MozLoopAPI.jsm
browser/components/loop/MozLoopService.jsm
browser/components/loop/content/shared/js/otSdkDriver.js
browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js
browser/components/loop/test/shared/otSdkDriver_test.js
toolkit/components/telemetry/Histograms.json
--- a/browser/components/loop/MozLoopAPI.jsm
+++ b/browser/components/loop/MozLoopAPI.jsm
@@ -777,24 +777,24 @@ function injectLoopAPI(targetWindow) {
                         "&body=" + encodeURIComponent(body);
         extProtocolSvc.loadURI(CommonUtils.makeURI(mailtoURL));
       }
     },
 
     /**
      * Adds a value to a telemetry histogram.
      *
-     * @param  {string} histogramId Name of the telemetry histogram to update.
-     * @param  {string} value       Label of bucket to increment in the histogram.
+     * @param  {String}  histogramId Name of the telemetry histogram to update.
+     * @param  {String}  value       Label of bucket to increment in the histogram.
      */
-    telemetryAddKeyedValue: {
+    telemetryAddValue: {
       enumerable: true,
       writable: true,
       value: function(histogramId, value) {
-        Services.telemetry.getKeyedHistogramById(histogramId).add(value);
+        Services.telemetry.getHistogramById(histogramId).add(value);
       }
     },
 
     /**
      * Returns a new GUID (UUID) in curly braces format.
      */
     generateUUID: {
       enumerable: true,
--- a/browser/components/loop/MozLoopService.jsm
+++ b/browser/components/loop/MozLoopService.jsm
@@ -11,40 +11,40 @@ const { classes: Cc, interfaces: Ci, uti
 const INVALID_AUTH_TOKEN = 110;
 
 const LOOP_SESSION_TYPE = {
   GUEST: 1,
   FXA: 2,
 };
 
 /***
- * Buckets that we segment 2-way media connection length telemetry probes
+ * Values that we segment 2-way media connection length telemetry probes
  * into.
  *
- * @type {{SHORTER_THAN_10S: string, BETWEEN_10S_AND_30S: string,
- *   BETWEEN_30S_AND_5M: string, MORE_THAN_5M: string}}
+ * @type {{SHORTER_THAN_10S: Number, BETWEEN_10S_AND_30S: Number,
+ *   BETWEEN_30S_AND_5M: Number, MORE_THAN_5M: Number}}
  */
 const TWO_WAY_MEDIA_CONN_LENGTH = {
-  SHORTER_THAN_10S: "SHORTER_THAN_10S",
-  BETWEEN_10S_AND_30S: "BETWEEN_10S_AND_30S",
-  BETWEEN_30S_AND_5M: "BETWEEN_30S_AND_5M",
-  MORE_THAN_5M: "MORE_THAN_5M",
+  SHORTER_THAN_10S: 0,
+  BETWEEN_10S_AND_30S: 1,
+  BETWEEN_30S_AND_5M: 2,
+  MORE_THAN_5M: 3,
 };
 
 /**
- * Buckets that we segment sharing state change telemetry probes into.
+ * Values that we segment sharing state change telemetry probes into.
  *
- * @type {{WINDOW_ENABLED: String, WINDOW_DISABLED: String,
- *   BROWSER_ENABLED: String, BROWSER_DISABLED: String}}
+ * @type {{WINDOW_ENABLED: Number, WINDOW_DISABLED: Number,
+ *   BROWSER_ENABLED: Number, BROWSER_DISABLED: Number}}
  */
 const SHARING_STATE_CHANGE = {
-  WINDOW_ENABLED: "WINDOW_ENABLED",
-  WINDOW_DISABLED: "WINDOW_DISABLED",
-  BROWSER_ENABLED: "BROWSER_ENABLED",
-  BROWSER_DISABLED: "BROWSER_DISABLED"
+  WINDOW_ENABLED: 0,
+  WINDOW_DISABLED: 1,
+  BROWSER_ENABLED: 2,
+  BROWSER_DISABLED: 3
 };
 
 // See LOG_LEVELS in Console.jsm. Common examples: "All", "Info", "Warn", & "Error".
 const PREF_LOG_LEVEL = "loop.debug.loglevel";
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
--- a/browser/components/loop/content/shared/js/otSdkDriver.js
+++ b/browser/components/loop/content/shared/js/otSdkDriver.js
@@ -664,18 +664,17 @@ loop.OTSdkDriver = (function() {
       if (callLengthSeconds >= 10 && callLengthSeconds <= 30) {
         bucket = buckets.BETWEEN_10S_AND_30S;
       } else if (callLengthSeconds > 30 && callLengthSeconds <= 300) {
         bucket = buckets.BETWEEN_30S_AND_5M;
       } else if (callLengthSeconds > 300) {
         bucket = buckets.MORE_THAN_5M;
       }
 
-      this.mozLoop.telemetryAddKeyedValue("LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
-        bucket);
+      this.mozLoop.telemetryAddValue("LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1", bucket);
       this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_ALREADY_NOTED);
 
       this._connectionLengthNotedCalls++;
       if (this._debugTwoWayMediaTelemetry) {
         console.log('Loop Telemetry: noted two-way media connection ' +
           'in bucket: ', bucket);
       }
     },
@@ -733,15 +732,15 @@ loop.OTSdkDriver = (function() {
 
       var bucket = this.mozLoop.SHARING_STATE_CHANGE[type.toUpperCase() + "_" +
         (enabled ? "ENABLED" : "DISABLED")];
       if (!bucket) {
         console.error("No sharing state bucket found for '" + type + "'");
         return;
       }
 
-      this.mozLoop.telemetryAddKeyedValue("LOOP_SHARING_STATE_CHANGE", bucket);
+      this.mozLoop.telemetryAddValue("LOOP_SHARING_STATE_CHANGE_1", bucket);
     }
   };
 
   return OTSdkDriver;
 
 })();
--- a/browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js
+++ b/browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js
@@ -18,58 +18,58 @@ add_task(function* test_initialize() {
     Services.telemetry.canRecordExtended = oldCanRecord;
   });
 });
 
 /**
  * Tests that enumerated bucket histograms exist and can be updated.
  */
 add_task(function* test_mozLoop_telemetryAdd_buckets() {
-  let histogramId = "LOOP_TWO_WAY_MEDIA_CONN_LENGTH";
-  let histogram = Services.telemetry.getKeyedHistogramById(histogramId);
+  let histogramId = "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1";
+  let histogram = Services.telemetry.getHistogramById(histogramId);
   let CONN_LENGTH = gMozLoopAPI.TWO_WAY_MEDIA_CONN_LENGTH;
 
   histogram.clear();
   for (let value of [CONN_LENGTH.SHORTER_THAN_10S,
                      CONN_LENGTH.BETWEEN_10S_AND_30S,
                      CONN_LENGTH.BETWEEN_10S_AND_30S,
                      CONN_LENGTH.BETWEEN_30S_AND_5M,
                      CONN_LENGTH.BETWEEN_30S_AND_5M,
                      CONN_LENGTH.BETWEEN_30S_AND_5M,
                      CONN_LENGTH.MORE_THAN_5M,
                      CONN_LENGTH.MORE_THAN_5M,
                      CONN_LENGTH.MORE_THAN_5M,
                      CONN_LENGTH.MORE_THAN_5M]) {
-    gMozLoopAPI.telemetryAddKeyedValue(histogramId, value);
+    gMozLoopAPI.telemetryAddValue(histogramId, value);
   }
 
   let snapshot = histogram.snapshot();
-  is(snapshot["SHORTER_THAN_10S"].sum, 1, "TWO_WAY_MEDIA_CONN_LENGTH.SHORTER_THAN_10S");
-  is(snapshot["BETWEEN_10S_AND_30S"].sum, 2, "TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_10S_AND_30S");
-  is(snapshot["BETWEEN_30S_AND_5M"].sum, 3, "TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_30S_AND_5M");
-  is(snapshot["MORE_THAN_5M"].sum, 4, "TWO_WAY_MEDIA_CONN_LENGTH.MORE_THAN_5M");
+  is(snapshot.counts[CONN_LENGTH.SHORTER_THAN_10S], 1, "TWO_WAY_MEDIA_CONN_LENGTH.SHORTER_THAN_10S");
+  is(snapshot.counts[CONN_LENGTH.BETWEEN_10S_AND_30S], 2, "TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_10S_AND_30S");
+  is(snapshot.counts[CONN_LENGTH.BETWEEN_30S_AND_5M], 3, "TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_30S_AND_5M");
+  is(snapshot.counts[CONN_LENGTH.MORE_THAN_5M], 4, "TWO_WAY_MEDIA_CONN_LENGTH.MORE_THAN_5M");
 });
 
 add_task(function* test_mozLoop_telemetryAdd_sharing_buckets() {
-  let histogramId = "LOOP_SHARING_STATE_CHANGE";
-  let histogram = Services.telemetry.getKeyedHistogramById(histogramId);
+  let histogramId = "LOOP_SHARING_STATE_CHANGE_1";
+  let histogram = Services.telemetry.getHistogramById(histogramId);
   const SHARING_STATES = gMozLoopAPI.SHARING_STATE_CHANGE;
 
   histogram.clear();
   for (let value of [SHARING_STATES.WINDOW_ENABLED,
                      SHARING_STATES.WINDOW_DISABLED,
                      SHARING_STATES.WINDOW_DISABLED,
                      SHARING_STATES.BROWSER_ENABLED,
                      SHARING_STATES.BROWSER_ENABLED,
                      SHARING_STATES.BROWSER_ENABLED,
                      SHARING_STATES.BROWSER_DISABLED,
                      SHARING_STATES.BROWSER_DISABLED,
                      SHARING_STATES.BROWSER_DISABLED,
                      SHARING_STATES.BROWSER_DISABLED]) {
-    gMozLoopAPI.telemetryAddKeyedValue(histogramId, value);
+    gMozLoopAPI.telemetryAddValue(histogramId, value);
   }
 
   let snapshot = histogram.snapshot();
-  Assert.strictEqual(snapshot["WINDOW_ENABLED"].sum, 1, "SHARING_STATE_CHANGE.WINDOW_ENABLED");
-  Assert.strictEqual(snapshot["WINDOW_DISABLED"].sum, 2, "SHARING_STATE_CHANGE.WINDOW_DISABLED");
-  Assert.strictEqual(snapshot["BROWSER_ENABLED"].sum, 3, "SHARING_STATE_CHANGE.BROWSER_ENABLED");
-  Assert.strictEqual(snapshot["BROWSER_DISABLED"].sum, 4, "SHARING_STATE_CHANGE.BROWSER_DISABLED");
+  Assert.strictEqual(snapshot.counts[SHARING_STATES.WINDOW_ENABLED], 1, "SHARING_STATE_CHANGE.WINDOW_ENABLED");
+  Assert.strictEqual(snapshot.counts[SHARING_STATES.WINDOW_DISABLED], 2, "SHARING_STATE_CHANGE.WINDOW_DISABLED");
+  Assert.strictEqual(snapshot.counts[SHARING_STATES.BROWSER_ENABLED], 3, "SHARING_STATE_CHANGE.BROWSER_ENABLED");
+  Assert.strictEqual(snapshot.counts[SHARING_STATES.BROWSER_DISABLED], 4, "SHARING_STATE_CHANGE.BROWSER_DISABLED");
 });
--- a/browser/components/loop/test/shared/otSdkDriver_test.js
+++ b/browser/components/loop/test/shared/otSdkDriver_test.js
@@ -60,17 +60,17 @@ describe("loop.OTSdkDriver", function ()
 
     window.OT = {
       ExceptionCodes: {
         UNABLE_TO_PUBLISH: 1500
       }
     };
 
     mozLoop = {
-      telemetryAddKeyedValue: sinon.stub(),
+      telemetryAddValue: sinon.stub(),
       TWO_WAY_MEDIA_CONN_LENGTH: {
         SHORTER_THAN_10S: "SHORTER_THAN_10S",
         BETWEEN_10S_AND_30S: "BETWEEN_10S_AND_30S",
         BETWEEN_30S_AND_5M: "BETWEEN_30S_AND_5M",
         MORE_THAN_5M: "MORE_THAN_5M"
       },
       SHARING_STATE_CHANGE: {
         WINDOW_ENABLED: "WINDOW_ENABLED",
@@ -441,103 +441,103 @@ describe("loop.OTSdkDriver", function ()
         eql(driver.CONNECTION_START_TIME_ALREADY_NOTED);
     });
 
     it("should call mozLoop.noteConnectionLength with SHORTER_THAN_10S for calls less than 10s", function() {
       var endTimeMS = 9000;
 
       driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWith(mozLoop.telemetryAddKeyedValue,
-        "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWith(mozLoop.telemetryAddValue,
+        "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1",
         mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.SHORTER_THAN_10S);
     });
 
     it("should call mozLoop.noteConnectionLength with BETWEEN_10S_AND_30S for 15s calls",
       function() {
         var endTimeMS = 15000;
 
         driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-        sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-        sinon.assert.calledWith(mozLoop.telemetryAddKeyedValue,
-          "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
+        sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+        sinon.assert.calledWith(mozLoop.telemetryAddValue,
+          "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1",
           mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_10S_AND_30S);
       });
 
     it("should call mozLoop.noteConnectionLength with BETWEEN_30S_AND_5M for 60s calls",
       function() {
         var endTimeMS = 60 * 1000;
 
         driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-        sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-        sinon.assert.calledWith(mozLoop.telemetryAddKeyedValue,
-          "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
+        sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+        sinon.assert.calledWith(mozLoop.telemetryAddValue,
+          "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1",
           mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.BETWEEN_30S_AND_5M);
       });
 
     it("should call mozLoop.noteConnectionLength with MORE_THAN_5M for 10m calls", function() {
       var endTimeMS = 10 * 60 * 1000;
 
       driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWith(mozLoop.telemetryAddKeyedValue,
-        "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWith(mozLoop.telemetryAddValue,
+        "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1",
         mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.MORE_THAN_5M);
     });
 
     it("should not call mozLoop.noteConnectionLength if" +
        " driver._sendTwoWayMediaTelemetry is false",
       function() {
         var endTimeMS = 10 * 60 * 1000;
         driver._sendTwoWayMediaTelemetry = false;
 
         driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
-        sinon.assert.notCalled(mozLoop.telemetryAddKeyedValue);
+        sinon.assert.notCalled(mozLoop.telemetryAddValue);
       });
   });
 
   describe("#_noteSharingState", function() {
     it("should record enabled sharing states for window", function() {
       driver._noteSharingState("window", true);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWithExactly(mozLoop.telemetryAddKeyedValue,
-        "LOOP_SHARING_STATE_CHANGE",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWithExactly(mozLoop.telemetryAddValue,
+        "LOOP_SHARING_STATE_CHANGE_1",
         mozLoop.SHARING_STATE_CHANGE.WINDOW_ENABLED);
     });
 
     it("should record enabled sharing states for browser", function() {
       driver._noteSharingState("browser", true);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWithExactly(mozLoop.telemetryAddKeyedValue,
-        "LOOP_SHARING_STATE_CHANGE",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWithExactly(mozLoop.telemetryAddValue,
+        "LOOP_SHARING_STATE_CHANGE_1",
         mozLoop.SHARING_STATE_CHANGE.BROWSER_ENABLED);
     });
 
     it("should record disabled sharing states for window", function() {
       driver._noteSharingState("window", false);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWithExactly(mozLoop.telemetryAddKeyedValue,
-        "LOOP_SHARING_STATE_CHANGE",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWithExactly(mozLoop.telemetryAddValue,
+        "LOOP_SHARING_STATE_CHANGE_1",
         mozLoop.SHARING_STATE_CHANGE.WINDOW_DISABLED);
     });
 
     it("should record disabled sharing states for browser", function() {
       driver._noteSharingState("browser", false);
 
-      sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
-      sinon.assert.calledWithExactly(mozLoop.telemetryAddKeyedValue,
-        "LOOP_SHARING_STATE_CHANGE",
+      sinon.assert.calledOnce(mozLoop.telemetryAddValue);
+      sinon.assert.calledWithExactly(mozLoop.telemetryAddValue,
+        "LOOP_SHARING_STATE_CHANGE_1",
         mozLoop.SHARING_STATE_CHANGE.BROWSER_DISABLED);
     });
   });
 
   describe("#forceDisconnectAll", function() {
     it("should not disconnect anything when not connected", function() {
       driver.session = session;
       driver.forceDisconnectAll(function() {});
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7512,30 +7512,31 @@
     "kind": "boolean",
     "description": "Stores 1 if generating a call URL succeeded, and 0 if it failed."
   },
   "LOOP_CLIENT_CALL_URL_SHARED": {
     "expires_in_version": "never",
     "kind": "boolean",
     "description": "Stores 1 every time the URL is copied or shared."
   },
-  "LOOP_TWO_WAY_MEDIA_CONN_LENGTH": {
+  "LOOP_TWO_WAY_MEDIA_CONN_LENGTH_1": {
+    "alert_emails": ["firefox-dev@mozilla.org", "dmose@mozilla.com"],
     "expires_in_version": "43",
-    "kind": "count",
-    "keyed": true,
+    "kind": "enumerated",
+    "n_values": 8,
     "releaseChannelCollection": "opt-out",
-    "description": "Connection length for bi-directionally connected media"
-  },
-  "LOOP_SHARING_STATE_CHANGE": {
+    "description": "Connection length for bi-directionally connected media (0=SHORTER_THAN_10S, 1=BETWEEN_10S_AND_30S, 2=BETWEEN_30S_AND_5M, 3=MORE_THAN_5M)"
+  },
+  "LOOP_SHARING_STATE_CHANGE_1": {
     "alert_emails": ["firefox-dev@mozilla.org", "mdeboer@mozilla.com"],
     "expires_in_version": "43",
-    "kind": "count",
-    "keyed": true,
-    "releaseChannelCollection": "opt-in",
-    "description": "Number of times the sharing feature has been enabled and disabled"
+    "kind": "enumerated",
+    "n_values": 8,
+    "releaseChannelCollection": "opt-out",
+    "description": "Number of times the sharing feature has been enabled and disabled (0=WINDOW_ENABLED, 1=WINDOW_DISABLED, 2=BROWSER_ENABLED, 3=BROWSER_DISABLED)"
   },
   "E10S_AUTOSTART": {
     "expires_in_version": "never",
     "kind": "boolean",
     "description": "Whether a session is set to autostart e10s windows"
   },
   "E10S_WINDOW": {
     "expires_in_version": "never",