Report video and network stats averaged over 1s, and create corresponding UMA metrics.

remoting/client/plugin/chromoting_instance.cc

Index: remoting/client/plugin/chromoting_instance.cc
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index c32d31ded27bcd4d333f047d9af82c1536d5442a..86d0d972966d407b9c38cd08474b1a24955f657a 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -32,6 +32,7 @@
 #include "ppapi/cpp/dev/url_util_dev.h"
 #include "ppapi/cpp/image_data.h"
 #include "ppapi/cpp/input_event.h"
+#include "ppapi/cpp/private/uma_private.h"
 #include "ppapi/cpp/rect.h"
 #include "ppapi/cpp/var_array_buffer.h"
 #include "ppapi/cpp/var_dictionary.h"
@@ -67,12 +68,29 @@ namespace {
 // Default DPI to assume for old clients that use notifyClientResolution.
 const int kDefaultDPI = 96;
 
-// Interval at which to sample performance statistics.
-const int kPerfStatsIntervalMs = 1000;
-
 // URL scheme used by Chrome apps and extensions.
 const char kChromeExtensionUrlScheme[] = "chrome-extension";
 
+// The boundary value for the FPS histogram: we don't expect video frame-rate to
+// be greater than 40fps.
+// Histograms expect samples to be less than the boundary value, so set to 41.
+const int kMaxFrameRate = 41;
+// For bandwidth, we'll use a histogram ranging from 0 to 1MB/s, spread across
+// 1000 buckets.
+// Histograms are log-scaled by default. This results in fine-grained buckets at
+// lower values and wider-ranged buckets closer to the maximum.
+// Values above the maximum defined here end up in the right-most bucket.
+// See $/src/base/metrics/histogram.h for more details.
+const int kBandwidthHistogramMin = 1;
+const int kBandwidthHistogramMax = 1024000;
+const int kBandwidthHistogramNumBuckets = 1000;
+
+// For the latency metrics, we'll set the max histogram value to 20,000ms, split
+// over 1000 buckets.
+const int kLatencyHistogramMin = 1;
+const int kLatencyHistogramMax = 20000;
+const int kLatencyHistogramNumBuckets = 1000;
+
 #if defined(USE_OPENSSL)
 // Size of the random seed blob used to initialize RNG in libjingle. Libjingle
 // uses the seed only for OpenSSL builds. OpenSSL needs at least 32 bytes of
@@ -728,11 +746,17 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
   client_->Start(signal_strategy_.get(), authenticator.Pass(),
                  transport_factory.Pass(), host_jid, capabilities);
 
-  // Start timer that periodically sends perf stats.
+  // Start timers that periodically send perf stats, for display and for UMA.
+  plugin_task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&ChromotingInstance::SendDisplayPerfStats,
+                            weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(
+        video_renderer_->GetStats()->kDisplayStatsUpdateWindowInSeconds));
   plugin_task_runner_->PostDelayedTask(
-      FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
+      FROM_HERE, base::Bind(&ChromotingInstance::SendUMAPerfStats,
                             weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+      base::TimeDelta::FromSeconds(
+        video_renderer_->GetStats()->kUMAStatsTimeWindowInSeconds));
 }
 
 void ChromotingInstance::HandleDisconnect(const base::DictionaryValue& data) {
@@ -1044,20 +1068,23 @@ void ChromotingInstance::SendOutgoingIq(const std::string& iq) {
   PostLegacyJsonMessage("sendOutgoingIq", data.Pass());
 }
 
-void ChromotingInstance::SendPerfStats() {
+void ChromotingInstance::SendDisplayPerfStats() {
   if (!video_renderer_.get()) {
     return;
   }
 
+  ChromotingStats* stats = video_renderer_->GetStats();

[Sergey Ulanov, 2015/07/08 20:06:47]

you can keep this line where it was - the consts in ChromotingStats should be
static.

[anandc, 2015/07/08 22:43:34]

Done.

   plugin_task_runner_->PostDelayedTask(
-      FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
+      FROM_HERE, base::Bind(&ChromotingInstance::SendDisplayPerfStats,
                             weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+      base::TimeDelta::FromSeconds(stats->kDisplayStatsUpdateWindowInSeconds));
 
+  // Update performance stats, averaged over the past few seconds.
+  // These are eventually upstreamed to the client and available for display to
+  // the user.
   scoped_ptr<base::DictionaryValue> data(new base::DictionaryValue());
-  ChromotingStats* stats = video_renderer_->GetStats();
-  data->SetDouble("videoBandwidth", stats->video_bandwidth()->Rate());
-  data->SetDouble("videoFrameRate", stats->video_frame_rate()->Rate());
+  data->SetDouble("videoBandwidth", stats->video_bandwidth_display()->Rate());
+  data->SetDouble("videoFrameRate", stats->video_frame_rate_display()->Rate());
   data->SetDouble("captureLatency", stats->video_capture_ms()->Average());
   data->SetDouble("encodeLatency", stats->video_encode_ms()->Average());
   data->SetDouble("decodeLatency", stats->video_decode_ms()->Average());
@@ -1066,6 +1093,38 @@ void ChromotingInstance::SendPerfStats() {
   PostLegacyJsonMessage("onPerfStats", data.Pass());
 }
 
+void ChromotingInstance::SendUMAPerfStats() {
+  if (!video_renderer_.get()) {
+    return;
+  }
+
+  ChromotingStats* stats = video_renderer_->GetStats();

[Sergey Ulanov, 2015/07/08 20:06:47]

move this below, before uma_interface calls. 

[anandc, 2015/07/08 22:43:34]

Done.

+  plugin_task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&ChromotingInstance::SendUMAPerfStats,
+                            weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(stats->kUMAStatsTimeWindowInSeconds));
+
+  // Update UMA histograms for video frame-rate, bandwidth and latencies.
+  // These metrics will be averaged over the last second.
+  pp::UMAPrivate uma_interface(this);

[Sergey Ulanov, 2015/07/08 20:06:47]

Maybe call this uma?

[anandc, 2015/07/08 22:43:34]

Done.

+  uma_interface.HistogramEnumeration("Chromoting.Video.FrameRate",
+    stats->video_frame_rate_uma()->Rate(), kMaxFrameRate);
+  uma_interface.HistogramEnumeration("Chromoting.Video.PacketRate",
+    stats->video_packet_rate_uma()->Rate(), kMaxFrameRate);
+  uma_interface.HistogramCustomCounts("Chromoting.Video.Bandwidth",
+    stats->video_bandwidth_uma()->Rate(), kBandwidthHistogramMin,
+    kBandwidthHistogramMax, kBandwidthHistogramNumBuckets);
+  uma_interface.HistogramCustomTimes("Chromoting.Video.CaptureLatency",
+    stats->video_capture_ms()->Average(), kLatencyHistogramMin,
+    kLatencyHistogramMax, kLatencyHistogramNumBuckets);
+  uma_interface.HistogramCustomTimes("Chromoting.Video.EncodeLatency",
+    stats->video_encode_ms()->Average(), kLatencyHistogramMin,
+    kLatencyHistogramMax, kLatencyHistogramNumBuckets);
+  uma_interface.HistogramCustomTimes("Chromoting.Video.RoundTripLatency",
+    stats->round_trip_ms()->Average(), kLatencyHistogramMin,
+    kLatencyHistogramMax, kLatencyHistogramNumBuckets);
+}
+
 // static
 void ChromotingInstance::RegisterLogMessageHandler() {
   base::AutoLock lock(g_logging_lock.Get());