Add UMA stats for Chromoting connection details.

remoting/webapp/base/js/client_session.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * Class handling creation and teardown of a remoting client session.
  *
  * The ClientSession class controls lifetime of the client plugin
  * object and provides the plugin with the functionality it needs to
@@ skipping 121 matching lines @@
  */
 remoting.ClientSession.EventHandler.prototype.onDisconnected =
     function(reason) {};
 
 // Note that the positive values in both of these enums are copied directly
 // from connection_to_host.h and must be kept in sync. Code in
 // chromoting_instance.cc converts the C++ enums into strings that must match
 // the names given here.
 // The negative values represent state transitions that occur within the
 // web-app that have no corresponding plugin state transition.
+// NOTE: The enums here correspond to the Chromoting.Connections enumerated
+// histogram defined in src/tools/metrics/histograms/histograms.xml. UMA
+// histograms don't work well with negative values, so only non-negative values
+// have been used for Chromoting.Connections.
+// The maximum values for the UMA enumerated histogram is included here for use
+// when uploading values to UMA.
+// The 2 lists should be kept in sync, and any new enums should be append-only.
 /** @enum {number} */
 remoting.ClientSession.State = {
+  MIN_STATE_ENUM: -3,
   CONNECTION_CANCELED: -3,  // Connection closed (gracefully) before connecting.
   CONNECTION_DROPPED: -2,  // Succeeded, but subsequently closed with an error.
   CREATED: -1,
   UNKNOWN: 0,
   INITIALIZING: 1,
   CONNECTING: 2,
   AUTHENTICATED: 3,
   CONNECTED: 4,
   CLOSED: 5,
-  FAILED: 6
+  FAILED: 6,
+  HISTOGRAM_MAX: 10, // According to src/base/metrics/histogram.h, for a UMA

[Ilya Sherman, 2015/08/21 21:39:48]

Optional nit: I think it might be clearer to define a constant that's equal to 6
or 7 here (i.e. is obviously related to "FAILED"), and then perform math to
compute the maximum for the histogram below.

[anandc, 2015/08/21 21:56:10]

Done.

+                     // enumerated histogram, the upper limit should be 1 above
+                     // the max-enum.
 };
 
 /**
  * @param {string} state The state name.
  * @return {remoting.ClientSession.State} The session state enum value.
  */
 remoting.ClientSession.State.fromString = function(state) {
   if (!remoting.ClientSession.State.hasOwnProperty(state)) {
     throw "Invalid ClientSession.State: " + state;
   }
@@ skipping 395 matching lines @@
     if (this.plugin_.hasCapability(
           remoting.ClientSession.Capability.TOUCH_EVENTS)) {
       this.plugin_.enableTouchEvents(true);
     }
   } else if (this.isFinished()) {
     base.dispose(this.connectedDisposables_);
     this.connectedDisposables_ = null;
   }
 
   this.notifyStateChanges_(oldState, this.state_);
+  // Record state count in an UMA enumerated histogram.
+  recordState(this.state_);
   this.logger_.logClientSessionStateChange(
       this.state_, this.error_, this.xmppErrorCache_.getFirstError());
 };
 
 /**
+ * Records a Chromoting Connection State, stored in an UMA enumerated histogram.
+ * @param {remoting.ClientSession.State} state State identifier.
+ */
+function recordState(state) {
+  var metricDescription = {
+    metricName: 'Chromoting.Connections',
+    type: 'histogram-linear',
+    min: 1, // According to histogram.h, min should be 1.
+    max: remoting.ClientSession.State.HISTOGRAM_MAX,
+    // The # of buckets should include 1 for overflow.

[Ilya Sherman, 2015/08/21 21:39:48]

This comment is misleading -- the +1 below is actually for the underflow bucket,
which is the bucket with index 0.  The overflow bucket is accounted for by
setting max to be 1 more than the maximum value you ever plan to record.  (The
reason that the underflow bucket is used freely, but the overflow bucket is not,
is that this allows the enumeration to be extended in subsequent versions of
Chrome.)

[anandc, 2015/08/21 21:56:10]

Done.

+    buckets: remoting.ClientSession.State.HISTOGRAM_MAX + 1
+  };
+
+  chrome.metricsPrivate.recordValue(metricDescription, state -

[kelvinp, 2015/08/21 21:32:09]

Consider state == MIN_STATE_ENUM, the difference would be 0, which is smaller
than the min of 1 defined above. 

[Ilya Sherman, 2015/08/21 21:39:48]

That is actually ok -- enumerated histograms typically do use the 0-indexed
bucket, which is normally the underflow bucket, because enumerated histograms
don't have any underflow.  This is a weird quirk of the histograms API, and
would be much easier to reason about if the metricsPrivate API just provided a
method for recording enumerated histograms.

[anandc, 2015/08/21 21:56:10]

Acknowledged.

[anandc, 2015/08/21 21:56:10]

Added comment explaining how this works. Tested by seeing that count for the
Canceled action (and, luckily, not Dropped, which would be harder to test) which
maps to MIN_STATE_ENUM gets updated as expected.

+      remoting.ClientSession.State.MIN_STATE_ENUM);
+}
+
+/**
  * @param {remoting.ClientSession.State} oldState The new state for the session.
  * @param {remoting.ClientSession.State} newState The new state for the session.
  * @private
  */
 remoting.ClientSession.prototype.notifyStateChanges_ =
     function(oldState, newState) {
   /** @type {remoting.Error} */
   var error;
   switch (this.state_) {
     case remoting.ClientSession.State.CONNECTED:
@@ skipping 76 matching lines @@
  * Enable or disable logging of connection errors due to a host being offline.
  * For example, if attempting a connection using a cached JID, host-offline
  * errors should not be logged because the JID will be refreshed and the
  * connection retried.
  *
  * @param {boolean} enable True to log host-offline errors; false to suppress.
  */
 remoting.ClientSession.prototype.logHostOfflineErrors = function(enable) {
   this.logHostOfflineErrors_ = enable;
 };