From a24d21503fe885a12af43bbab0e5ae0df4cc1572 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Thu, 18 Sep 2014 13:05:39 +0100 Subject: [PATCH] Bug 1059186 - Client needs to report number of generated URLs via Telemetry. r=MattN --- browser/components/loop/MozLoopAPI.jsm | 16 ++++++- browser/components/loop/content/js/client.js | 23 +++++++++- .../loop/test/desktop-local/client_test.js | 45 ++++++++++++++++++- .../loop/test/mochitest/browser.ini | 2 + .../mochitest/browser_mozLoop_telemetry.js | 36 +++++++++++++++ toolkit/components/telemetry/Histograms.json | 5 +++ 6 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js diff --git a/browser/components/loop/MozLoopAPI.jsm b/browser/components/loop/MozLoopAPI.jsm index 4121361c24b6..d5c1c76fceb7 100644 --- a/browser/components/loop/MozLoopAPI.jsm +++ b/browser/components/loop/MozLoopAPI.jsm @@ -474,7 +474,21 @@ 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 {integer} value Value to add to the histogram. + */ + telemetryAdd: { + enumerable: true, + writable: true, + value: function(histogramId, value) { + Services.telemetry.getHistogramById(histogramId).add(value); + } + }, }; function onStatusChanged(aSubject, aTopic, aData) { diff --git a/browser/components/loop/content/js/client.js b/browser/components/loop/content/js/client.js index 5016435e9cb4..e3c6ebf2358b 100644 --- a/browser/components/loop/content/js/client.js +++ b/browser/components/loop/content/js/client.js @@ -111,6 +111,7 @@ loop.Client = (function($) { this.mozLoop.hawkRequest("/call-url/", "POST", {callerId: nickname}, function (error, responseText) { if (error) { + this._telemetryAdd("LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", false); this._failureHandler(cb, error); return; } @@ -118,8 +119,14 @@ loop.Client = (function($) { try { var urlData = JSON.parse(responseText); - cb(null, this._validate(urlData, expectedCallUrlProperties)); + // This throws if the data is invalid, in which case only the failure + // telementry will be recorded. + var returnData = this._validate(urlData, expectedCallUrlProperties); + + this._telemetryAdd("LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", true); + cb(null, returnData); } catch (err) { + this._telemetryAdd("LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", false); console.log("Error requesting call info", err); cb(err); } @@ -187,6 +194,20 @@ loop.Client = (function($) { this._requestCallUrlInternal(nickname, cb); }.bind(this)); }, + + /** + * Adds a value to a telemetry histogram, ignoring errors. + * + * @param {string} histogramId Name of the telemetry histogram to update. + * @param {integer} value Value to add to the histogram. + */ + _telemetryAdd: function(histogramId, value) { + try { + this.mozLoop.telemetryAdd(histogramId, value); + } catch (err) { + console.error("Error recording telemetry", err); + } + }, }; return Client; diff --git a/browser/components/loop/test/desktop-local/client_test.js b/browser/components/loop/test/desktop-local/client_test.js index f9195e738c70..e3876b709435 100644 --- a/browser/components/loop/test/desktop-local/client_test.js +++ b/browser/components/loop/test/desktop-local/client_test.js @@ -34,7 +34,8 @@ describe("loop.Client", function() { .returns(fakeToken), ensureRegistered: sinon.stub().callsArgWith(0, null), noteCallUrlExpiry: sinon.spy(), - hawkRequest: sinon.stub() + hawkRequest: sinon.stub(), + telemetryAdd: sinon.spy(), }; // Alias for clearer tests. hawkRequestStub = mozLoop.hawkRequest; @@ -156,6 +157,30 @@ describe("loop.Client", function() { sinon.assert.notCalled(mozLoop.noteCallUrlExpiry); }); + it("should call mozLoop.telemetryAdd when the request succeeds", + function(done) { + var callUrlData = { + "callUrl": "fakeCallUrl", + "expiresAt": 60 + }; + + // Sets up the hawkRequest stub to trigger the callback with no error + // and the url. + hawkRequestStub.callsArgWith(3, null, + JSON.stringify(callUrlData)); + + client.requestCallUrl("foo", function(err) { + expect(err).to.be.null; + + sinon.assert.calledOnce(mozLoop.telemetryAdd); + sinon.assert.calledWith(mozLoop.telemetryAdd, + "LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", + true); + + done(); + }); + }); + it("should send an error when the request fails", function() { // Sets up the hawkRequest stub to trigger the callback with // an error @@ -181,6 +206,24 @@ describe("loop.Client", function() { return /Invalid data received/.test(err.message); })); }); + + it("should call mozLoop.telemetryAdd when the request fails", + function(done) { + // Sets up the hawkRequest stub to trigger the callback with + // an error + hawkRequestStub.callsArgWith(3, fakeErrorRes); + + client.requestCallUrl("foo", function(err) { + expect(err).not.to.be.null; + + sinon.assert.calledOnce(mozLoop.telemetryAdd); + sinon.assert.calledWith(mozLoop.telemetryAdd, + "LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", + false); + + done(); + }); + }); }); }); }); diff --git a/browser/components/loop/test/mochitest/browser.ini b/browser/components/loop/test/mochitest/browser.ini index 693c919b7d37..e75536da5ca1 100644 --- a/browser/components/loop/test/mochitest/browser.ini +++ b/browser/components/loop/test/mochitest/browser.ini @@ -16,3 +16,5 @@ skip-if = e10s skip-if = buildapp == 'mulet' [browser_toolbarbutton.js] [browser_mozLoop_pluralStrings.js] +[browser_mozLoop_telemetry.js] +skip-if = e10s diff --git a/browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js b/browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js new file mode 100644 index 000000000000..7e0210092de9 --- /dev/null +++ b/browser/components/loop/test/mochitest/browser_mozLoop_telemetry.js @@ -0,0 +1,36 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* + * This file contains tests for the mozLoop telemetry API. + */ + +add_task(loadLoopPanel); + +/** + * Tests that boolean histograms exist and can be updated. + */ +add_task(function* test_mozLoop_telemetryAdd_boolean() { + for (let histogramId of [ + "LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS", + ]) { + let snapshot = Services.telemetry.getHistogramById(histogramId).snapshot(); + + let initialFalseCount = snapshot.counts[0]; + let initialTrueCount = snapshot.counts[1]; + + for (let value of [false, false, true]) { + gMozLoopAPI.telemetryAdd(histogramId, value); + } + + // The telemetry service updates histograms asynchronously, so we need to + // poll for the final values and time out otherwise. + info("Waiting for update of " + histogramId); + do { + yield new Promise(resolve => setTimeout(resolve, 50)); + snapshot = Services.telemetry.getHistogramById(histogramId).snapshot(); + } while (snapshot.counts[0] == initialFalseCount + 2 && + snapshot.counts[1] == initialTrueCount + 1); + ok(true, "Correctly updated " + histogramId); + } +}); diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 5a4f90fe0e55..0131af49a39d 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -6589,6 +6589,11 @@ "n_values": 3, "description": "Doorhanger shown = 0, Disable = 1, Enable = 2" }, + "LOOP_CLIENT_CALL_URL_REQUESTS_SUCCESS": { + "expires_in_version": "never", + "kind": "boolean", + "description": "Stores 1 if generating a call URL succeeded, and 0 if it failed." + }, "E10S_AUTOSTART": { "expires_in_version": "never", "kind": "boolean",