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..25e7683d28f90fa644d66ef35d03055444741672 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"
@@ -1064,6 +1065,27 @@ void ChromotingInstance::SendPerfStats() {
   data->SetDouble("renderLatency", stats->video_paint_ms()->Average());
   data->SetDouble("roundtripLatency", stats->round_trip_ms()->Average());
   PostLegacyJsonMessage("onPerfStats", data.Pass());
+
+  // Also update UMA histograms for video frame-rate, bandwidth and latencies.

[Wez, 2015/06/29 14:56:13]

nit: "Also" implies that there is a comment earlier on to explain what we were
previously doing.

Suggest just "Update UMA with stats averaged over the last second."

[anandc, 2015/06/30 20:17:39]

Done.

+  pp::UMAPrivate uma_interface(this);
+  // We don't expect a frame-rate greater than 40fps, so set boundary to 40.

[Wez, 2015/06/29 14:56:13]

This comment should appear next to a constant defining the value, used in the
calls below.

[anandc, 2015/06/30 20:17:39]

Thanks. Done.
Also updated to 41, based on explanation here:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/c/private/pp...

+  uma_interface.HistogramEnumeration(
+      "Chromoting.Video.FrameRate", stats->video_fps()->Rate(), 40);
+  uma_interface.HistogramEnumeration(
+      "Chromoting.Video.FrameRate",
+      stats->video_fps_with_empty_frames()->Rate(), 40);
+
+  // For bandwidth, set the max value to 1MB/s, spread across 1000 buckets.

[Wez, 2015/06/29 14:56:13]

Ditto.

What is the rationale for 1000 buckets?

[anandc, 2015/06/30 20:17:39]

Done.
If, after we go live with these settings, we find that the histograms we get are
not fine-grained enough, we can tweak them. 
If that does happen, we should rename the metrics, rather than change the
settings of existing metrics, because that messes up historical comparisons. 

So, while the current settings look OK to me, please let me know if you have
better recommendations for these values.

Once we have OK from our team for the current histogram values and metric names,
I plan on getting them reviewed by a UMA team member as well.

+  uma_interface.HistogramCustomCounts("Chromoting.Video.Bandwidth",
+    stats->video_bytes_per_s()->Rate(), 0, 1024000, 1000);
+
+  // For, the latency metrics use a max value of 1000ms.

[dahlke1, 2015/06/24 23:12:11]

The max latency for the past 7 days is 11,381ms. If we must have a max value, we
should give ourselves some buffer (20,000) and understand what the effect is if
we go over that max value.

[Wez, 2015/06/29 14:56:13]

Ditto re constants as above, and rationale for the 1000 buckets, and the choice
of max latency.

[anandc, 2015/06/30 20:17:39]

Done.

+  uma_interface.HistogramCustomTimes("Chromoting.Video.CaptureLatency",
+    stats->video_capture_ms()->Average(), 0, 1000, 1000);
+  uma_interface.HistogramCustomTimes("Chromoting.Video.EncodeLatency",
+    stats->video_encode_ms()->Average(), 0, 1000, 1000);
+  uma_interface.HistogramCustomTimes("Chromoting.Video.RoundTripLatency",
+    stats->round_trip_ms()->Average(), 0, 1000, 1000);
 }
 
 // static