Bug 1145819 - Fix loop Telemetry counting of direct calls, r=Standard8
authorDan Mosedale <dmose@meer.net>
Tue, 24 Mar 2015 14:28:45 -0700
changeset 264349 666e41236982cbb3c2914ae06074456a75a8db4f
parent 264348 cd66c01ef6415128d9db9468beb668ddb0321650
child 264350 d1f32269c3b4c16aec350b25dff444d20f2ddcda
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1145819
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1145819 - Fix loop Telemetry counting of direct calls, r=Standard8
browser/components/loop/content/shared/js/activeRoomStore.js
browser/components/loop/content/shared/js/conversationStore.js
browser/components/loop/content/shared/js/otSdkDriver.js
browser/components/loop/test/shared/activeRoomStore_test.js
browser/components/loop/test/shared/conversationStore_test.js
browser/components/loop/test/shared/otSdkDriver_test.js
--- a/browser/components/loop/content/shared/js/activeRoomStore.js
+++ b/browser/components/loop/content/shared/js/activeRoomStore.js
@@ -328,16 +328,20 @@ loop.store.ActiveRoomStore = (function()
       this.setStoreState({
         apiKey: actionData.apiKey,
         sessionToken: actionData.sessionToken,
         sessionId: actionData.sessionId,
         roomState: ROOM_STATES.JOINED
       });
 
       this._setRefreshTimeout(actionData.expires);
+
+      // Only send media telemetry on one side of the call: the desktop side.
+      actionData["sendTwoWayMediaTelemetry"] = this._isDesktop;
+
       this._sdkDriver.connectSession(actionData);
 
       this._mozLoop.addConversationContext(this._storeState.windowId,
                                            actionData.sessionId, "");
 
       // If we haven't got a room name yet, go and get one. We typically
       // need to do this in the case of the standalone window.
       // XXX When bug 1103331 lands this can be moved to earlier.
--- a/browser/components/loop/content/shared/js/conversationStore.js
+++ b/browser/components/loop/content/shared/js/conversationStore.js
@@ -265,17 +265,18 @@ loop.store = loop.store || {};
      * sdk, and setting the state appropriately.
      */
     _startCallConnection: function() {
       var state = this.getStoreState();
 
       this.sdkDriver.connectSession({
         apiKey: state.apiKey,
         sessionId: state.sessionId,
-        sessionToken: state.sessionToken
+        sessionToken: state.sessionToken,
+        sendTwoWayMediaTelemetry: state.outgoing // only one side of the call
       });
       this.mozLoop.addConversationContext(
         state.windowId,
         state.sessionId,
         state.callId);
       this.setStoreState({callState: CALL_STATES.ONGOING});
     },
 
--- a/browser/components/loop/content/shared/js/otSdkDriver.js
+++ b/browser/components/loop/content/shared/js/otSdkDriver.js
@@ -32,17 +32,16 @@ loop.OTSdkDriver = (function() {
       if (this._isDesktop) {
         if (!options.mozLoop) {
           throw new Error("Missing option mozLoop");
         }
         this.mozLoop = options.mozLoop;
       }
 
       this.connections = {};
-      this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_UNINITIALIZED);
 
       this.dispatcher.register(this, [
         "setupStreamElements",
         "setMute"
       ]);
 
       // Set loop.debug.twoWayMediaTelemetry to true in the browser
       // by changing the hidden pref loop.debug.twoWayMediaTelemetry using
@@ -205,22 +204,30 @@ loop.OTSdkDriver = (function() {
 
     /**
      * Connects a session for the SDK, listening to the required events.
      *
      * sessionData items:
      * - sessionId: The OT session ID
      * - apiKey: The OT API key
      * - sessionToken: The token for the OT session
+     * - sendTwoWayMediaTelemetry: boolean should we send telemetry on length
+     *                             of media sessions.  Callers should ensure
+     *                             that this is only set for one side of the
+     *                             session so that things don't get
+     *                             double-counted.
      *
      * @param {Object} sessionData The session data for setting up the OT session.
      */
     connectSession: function(sessionData) {
       this.session = this.sdk.initSession(sessionData.sessionId);
 
+      this._sendTwoWayMediaTelemetry = !!sessionData.sendTwoWayMediaTelemetry;
+      this._setTwoWayMediaStartTime(this.CONNECTION_START_TIME_UNINITIALIZED);
+
       this.session.on("connectionCreated", this._onConnectionCreated.bind(this));
       this.session.on("streamCreated", this._onRemoteStreamCreated.bind(this));
       this.session.on("streamDestroyed", this._onRemoteStreamDestroyed.bind(this));
       this.session.on("connectionDestroyed",
         this._onConnectionDestroyed.bind(this));
       this.session.on("sessionDisconnected",
         this._onSessionDisconnected.bind(this));
       this.session.on("streamPropertyChanged", this._onStreamPropertyChanged.bind(this));
@@ -463,27 +470,27 @@ loop.OTSdkDriver = (function() {
 
     /**
      * Set and get the start time of the two-way media connection.  These
      * are done as wrapper functions so that we can log sets to make manual
      * verification of various telemetry scenarios possible.  The get API is
      * analogous in order to follow the principle of least surprise for
      * people consuming this code.
      *
-     * If this._isDesktop is not true, returns immediately without making
-     * any changes, since this data is not used, and it makes reading
-     * the logs confusing for manual verification of both ends of the call in
-     * the same browser, which is a case we care about.
+     * If this._sendTwoWayMediaTelemetry is not true, returns immediately
+     * without making any changes, since this data is not used, and it makes
+     * reading the logs confusing for manual verification of both ends of the
+     * call in the same browser, which is a case we care about.
      *
      * @param start  start time in milliseconds, as returned by
      *               performance.now()
      * @private
      */
     _setTwoWayMediaStartTime: function(start) {
-      if (!this._isDesktop) {
+      if (!this._sendTwoWayMediaTelemetry) {
         return;
       }
 
       this.__twoWayMediaStartTime = start;
       if (this._debugTwoWayMediaTelemetry) {
         console.log("Loop Telemetry: noted two-way connection start, " +
                     "start time in ms:", start);
       }
@@ -585,17 +592,17 @@ loop.OTSdkDriver = (function() {
     _maybePublishLocalStream: function() {
       if (this._sessionConnected && this._publisherReady) {
         // We are clear to publish the stream to the session.
         this.session.publish(this.publisher);
 
         // Now record the fact, and check if we've got all media yet.
         this._publishedLocalStream = true;
         if (this._checkAllStreamsConnected()) {
-          this._setTwoWayMediaStartTime(performance.now);
+          this._setTwoWayMediaStartTime(performance.now());
           this.dispatcher.dispatch(new sharedActions.MediaConnected());
         }
       }
     },
 
     /**
      * Used to check if both local and remote streams are available
      * and send an action if they are.
@@ -671,25 +678,24 @@ loop.OTSdkDriver = (function() {
         console.log('Loop Telemetry: noted two-way media connection ' +
           'in bucket: ', bucket);
       }
     },
 
     /**
      * Note connection length if it's valid (the startTime has been initialized
      * and is not later than endTime) and not yet already noted.  If
-     * this._isDesktop is not true, we're assumed to be running in the
-     * standalone client and return immediately.
+     * this._sendTwoWayMediaTelemetry is not true, we return immediately.
      *
      * @param {number} startTime  in milliseconds
      * @param {number} endTime  in milliseconds
      * @private
      */
     _noteConnectionLengthIfNeeded: function(startTime, endTime) {
-      if (!this._isDesktop) {
+      if (!this._sendTwoWayMediaTelemetry) {
         return;
       }
 
       if (startTime == this.CONNECTION_START_TIME_ALREADY_NOTED ||
           startTime == this.CONNECTION_START_TIME_UNINITIALIZED ||
           startTime > endTime) {
         if (this._debugTwoWayMediaTelemetry) {
           console.log("_noteConnectionLengthIfNeeded called with " +
--- a/browser/components/loop/test/shared/activeRoomStore_test.js
+++ b/browser/components/loop/test/shared/activeRoomStore_test.js
@@ -536,16 +536,38 @@ describe("loop.store.ActiveRoomStore", f
 
       store.joinedRoom(actionData);
 
       sinon.assert.calledOnce(fakeSdkDriver.connectSession);
       sinon.assert.calledWithExactly(fakeSdkDriver.connectSession,
         actionData);
     });
 
+    it("should pass 'sendTwoWayMediaTelemetry' as true to connectSession if " +
+       "store._isDesktop is true", function() {
+      store._isDesktop = true;
+
+      store.joinedRoom(new sharedActions.JoinedRoom(fakeJoinedData));
+
+      sinon.assert.calledOnce(fakeSdkDriver.connectSession);
+      sinon.assert.calledWithMatch(fakeSdkDriver.connectSession,
+        sinon.match.has("sendTwoWayMediaTelemetry", true));
+    });
+
+    it("should pass 'sendTwoWayTelemetry' as false to connectionSession if " +
+       "store._isDesktop is false", function() {
+      store._isDesktop = false;
+
+      store.joinedRoom(new sharedActions.JoinedRoom(fakeJoinedData));
+
+      sinon.assert.calledOnce(fakeSdkDriver.connectSession);
+      sinon.assert.calledWithMatch(fakeSdkDriver.connectSession,
+        sinon.match.has("sendTwoWayMediaTelemetry", false));
+    });
+
     it("should call mozLoop.addConversationContext", function() {
       var actionData = new sharedActions.JoinedRoom(fakeJoinedData);
 
       store.setupWindowData(new sharedActions.SetupWindowData({
         windowId: "42",
         type: "room",
       }));
 
--- a/browser/components/loop/test/shared/conversationStore_test.js
+++ b/browser/components/loop/test/shared/conversationStore_test.js
@@ -247,17 +247,18 @@ describe("loop.store.ConversationStore",
 
         store.connectionProgress(
           new sharedActions.ConnectionProgress({wsState: WS_STATES.CONNECTING}));
 
         sinon.assert.calledOnce(sdkDriver.connectSession);
         sinon.assert.calledWithExactly(sdkDriver.connectSession, {
           apiKey: "fakeKey",
           sessionId: "321456",
-          sessionToken: "341256"
+          sessionToken: "341256",
+          sendTwoWayMediaTelemetry: true
         });
       });
 
       it("should call mozLoop.addConversationContext", function() {
         store.setStoreState(fakeSessionData);
 
         store.connectionProgress(
           new sharedActions.ConnectionProgress({wsState: WS_STATES.CONNECTING}));
@@ -575,27 +576,28 @@ describe("loop.store.ConversationStore",
 
     it("should change the state to 'ongoing'", function() {
       store.acceptCall(
         new sharedActions.AcceptCall({callType: CALL_TYPES.AUDIO_ONLY}));
 
       expect(store.getStoreState("callState")).eql(CALL_STATES.ONGOING);
     });
 
-    it("should connect the session", function() {
+    it("should connect the session with sendTwoWayMediaTelemetry set as falsy", function() {
       store.setStoreState(fakeSessionData);
 
       store.acceptCall(
         new sharedActions.AcceptCall({callType: CALL_TYPES.AUDIO_ONLY}));
 
       sinon.assert.calledOnce(sdkDriver.connectSession);
       sinon.assert.calledWithExactly(sdkDriver.connectSession, {
         apiKey: "fakeKey",
         sessionId: "321456",
-        sessionToken: "341256"
+        sessionToken: "341256",
+        sendTwoWayMediaTelemetry: undefined
       });
     });
 
     it("should call mozLoop.addConversationContext", function() {
       store.setStoreState(fakeSessionData);
 
       store.acceptCall(
         new sharedActions.AcceptCall({callType: CALL_TYPES.AUDIO_ONLY}));
--- a/browser/components/loop/test/shared/otSdkDriver_test.js
+++ b/browser/components/loop/test/shared/otSdkDriver_test.js
@@ -99,24 +99,16 @@ describe("loop.OTSdkDriver", function ()
       }).to.Throw(/dispatcher/);
     });
 
     it("should throw an error if the sdk is missing", function() {
       expect(function() {
         new loop.OTSdkDriver({dispatcher: dispatcher});
       }).to.Throw(/sdk/);
     });
-
-    it("should set the two-way media start time to 'uninitialized'", function() {
-      var driver = new loop.OTSdkDriver(
-        {sdk: sdk, dispatcher: dispatcher, mozLoop: mozLoop, isDesktop: true});
-
-      expect(driver._getTwoWayMediaStartTime()).to.
-        eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
-    });
   });
 
   describe("#setupStreamElements", function() {
     it("should call initPublisher", function() {
       dispatcher.dispatch(new sharedActions.SetupStreamElements({
         getLocalElementFunc: function() {return fakeLocalElement;},
         getRemoteElementFunc: function() {return fakeRemoteElement;},
         publisherConfig: publisherConfig
@@ -347,16 +339,25 @@ describe("loop.OTSdkDriver", function ()
 
     it("should connect the session", function () {
       driver.connectSession(sessionData);
 
       sinon.assert.calledOnce(session.connect);
       sinon.assert.calledWith(session.connect, "1234567890", "1357924680");
     });
 
+    it("should set the two-way media start time to 'uninitialized' " +
+       "when sessionData.sendTwoWayMediaTelemetry is true'", function() {
+      driver.connectSession(_.extend(sessionData,
+                                     {sendTwoWayMediaTelemetry: true}));
+
+      expect(driver._getTwoWayMediaStartTime()).to.
+        eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
+    });
+
     describe("On connection complete", function() {
       it("should publish the stream if the publisher is ready", function() {
         driver._publisherReady = true;
         session.connect.callsArg(2);
 
         driver.connectSession(sessionData);
 
         sinon.assert.calledOnce(session.publish);
@@ -393,44 +394,47 @@ describe("loop.OTSdkDriver", function ()
 
       sinon.assert.calledOnce(publisher.destroy);
     });
 
     it("should call _noteConnectionLengthIfNeeded with connection duration", function() {
       driver.session = session;
       var startTime = 1;
       var endTime = 3;
+      driver._sendTwoWayMediaTelemetry = true;
       driver._setTwoWayMediaStartTime(startTime);
       sandbox.stub(performance, "now").returns(endTime);
       sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
       driver.disconnectSession();
 
       sinon.assert.calledWith(driver._noteConnectionLengthIfNeeded, startTime,
                               endTime);
     });
 
     it("should reset the two-way media connection start time", function() {
       driver.session = session;
       var startTime = 1;
+      driver._sendTwoWayMediaTelemetry = true;
       driver._setTwoWayMediaStartTime(startTime);
       sandbox.stub(performance, "now");
       sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
       driver.disconnectSession();
 
       expect(driver._getTwoWayMediaStartTime()).to.
         eql(driver.CONNECTION_START_TIME_UNINITIALIZED);
     });
   });
 
   describe("#_noteConnectionLengthIfNeeded", function() {
     var startTimeMS;
     beforeEach(function() {
       startTimeMS = 1;
+      driver._sendTwoWayMediaTelemetry = true;
       driver._setTwoWayMediaStartTime(startTimeMS);
     });
 
     it("should set two-way media start time to CONNECTION_START_TIME_ALREADY_NOTED", function() {
       var endTimeMS = 3;
       driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
       expect(driver._getTwoWayMediaStartTime()).to.
@@ -478,21 +482,21 @@ describe("loop.OTSdkDriver", function ()
       driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
       sinon.assert.calledOnce(mozLoop.telemetryAddKeyedValue);
       sinon.assert.calledWith(mozLoop.telemetryAddKeyedValue,
         "LOOP_TWO_WAY_MEDIA_CONN_LENGTH",
         mozLoop.TWO_WAY_MEDIA_CONN_LENGTH.MORE_THAN_5M);
     });
 
-    it("should not call mozLoop.noteConnectionLength if driver._isDesktop " +
-       "is false",
+    it("should not call mozLoop.noteConnectionLength if" +
+       " driver._sendTwoWayMediaTelemetry is false",
       function() {
         var endTimeMS = 10 * 60 * 1000;
-        driver._isDesktop = false;
+        driver._sendTwoWayMediaTelemetry = false;
 
         driver._noteConnectionLengthIfNeeded(startTimeMS, endTimeMS);
 
         sinon.assert.notCalled(mozLoop.telemetryAddKeyedValue);
       });
   });
 
   describe("#_noteSharingState", function() {
@@ -611,16 +615,17 @@ describe("loop.OTSdkDriver", function ()
             sinon.match.hasOwn("peerHungup", false));
       });
 
 
       it("should call _noteConnectionLengthIfNeeded with connection duration", function() {
         driver.session = session;
         var startTime = 1;
         var endTime = 3;
+        driver._sendTwoWayMediaTelemetry = true;
         driver._setTwoWayMediaStartTime(startTime);
         sandbox.stub(performance, "now").returns(endTime);
         sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
         session.trigger("connectionDestroyed", {
           reason: "clientDisconnected"
         });
 
@@ -655,16 +660,17 @@ describe("loop.OTSdkDriver", function ()
           sinon.assert.calledWithMatch(dispatcher.dispatch,
             sinon.match.hasOwn("reason", FAILURE_DETAILS.EXPIRED_OR_INVALID));
         });
 
       it("should call _noteConnectionLengthIfNeeded with connection duration", function() {
         driver.session = session;
         var startTime = 1;
         var endTime = 3;
+        driver._sendTwoWayMediaTelemetry = true;
         driver._setTwoWayMediaStartTime(startTime);
         sandbox.stub(performance, "now").returns(endTime);
         sandbox.stub(driver, "_noteConnectionLengthIfNeeded");
 
         session.trigger("sessionDisconnected", {
           reason: "networkDisconnected"
         });
 
@@ -742,17 +748,18 @@ describe("loop.OTSdkDriver", function ()
 
         // Called twice due to the VideoDimensionsChanged above.
         sinon.assert.calledTwice(dispatcher.dispatch);
         sinon.assert.calledWithMatch(dispatcher.dispatch,
           sinon.match.hasOwn("name", "mediaConnected"));
       });
 
       it("should store the start time when both streams are up and" +
-      " driver._isDesktop is true", function() {
+      " driver._sendTwoWayMediaTelemetry is true", function() {
+        driver._sendTwoWayMediaTelemetry = true;
         driver._publishedLocalStream = true;
         var startTime = 1;
         sandbox.stub(performance, "now").returns(startTime);
 
         session.trigger("streamCreated", {stream: fakeStream});
 
         expect(driver._getTwoWayMediaStartTime()).to.eql(startTime);
       });