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 are also defined in histograms.xml for our UMA
+// Chromoting.Connections histogram. The 2 lists should be kept in sync.

[Ilya Sherman, 2015/08/21 19:48:29]

Please also document that the enum should be treated as append-only for this
reason.

[anandc, 2015/08/21 21:30:03]

Done.

 /** @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,
+  MAX_STATE_ENUM: 7 // The upper limit for a UMA enumerated histogram should be
+                    // 1 above the max-enum. See src/base/metrics/histogram.h.
 };
 
 /**
  * @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: remoting.ClientSession.State.MIN_STATE_ENUM,

[Ilya Sherman, 2015/08/21 19:48:29]

Hmm, I'm not actually sure whether negative buckets work smoothly for
histograms.  Could you please check that chrome://histograms shows what you'd
expect?  If it doesn't, an easy tweak is to simply add 3 (or, equivalently,
subtract MIN_STATE_ENUM) to the logged values.  (Though, this tweak does mean
that you'd need to freeze MIN_STATE_ENUM at -3 for all time; otherwise, the
semantics of the recorded enum values would change each time that MIN_STATE_ENUM
changes.)

[anandc, 2015/08/21 21:30:03]

Thanks. Negative values were actually not working in chrome://histograms.
Updated to use only non-negative enum values.

+    max: remoting.ClientSession.State.MAX_STATE_ENUM,
+    // The # of buckets should include 1 for overflow. Use the boundary values
+    // for the ClientSession.State enum to set the # of buckets.
+    buckets: (remoting.ClientSession.State.MAX_STATE_ENUM) -
+    (remoting.ClientSession.State.MIN_STATE_ENUM) + 2

[Ilya Sherman, 2015/08/21 19:48:29]

nit: Is the indentation correct here?

[Ilya Sherman, 2015/08/21 19:48:29]

MAX_STATE_ENUM Is already reserving an extra bucket, so I'm not sure why this is
+2 rather than +1.

[anandc, 2015/08/21 21:30:03]

Done.

[anandc, 2015/08/21 21:30:03]

Thanks for catching. Forgot to update here when changing the value in the
SessionState enum.

+  };
+
+  chrome.metricsPrivate.recordValue(metricDescription, state);
+}
+
+/**
  * @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;
 };