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

remoting/client/plugin/chromoting_instance.cc

 // 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.
 
 #include "remoting/client/plugin/chromoting_instance.h"
 
 #include <string>
 #include <vector>
 
 #if defined(OS_NACL)
@@ skipping 14 matching lines @@
 #include "base/threading/thread.h"
 #include "base/values.h"
 #include "crypto/random.h"
 #include "jingle/glue/thread_wrapper.h"
 #include "media/base/yuv_convert.h"
 #include "net/socket/ssl_server_socket.h"
 #include "ppapi/cpp/completion_callback.h"
 #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"
 #include "remoting/base/constants.h"
 #include "remoting/base/util.h"
 #include "remoting/client/chromoting_client.h"
 #include "remoting/client/plugin/delegating_signal_strategy.h"
 #include "remoting/client/plugin/normalizing_input_filter_cros.h"
 #include "remoting/client/plugin/normalizing_input_filter_mac.h"
 #include "remoting/client/plugin/pepper_audio_player.h"
@@ skipping 15 matching lines @@
 #undef PostMessage
 #endif
 
 namespace remoting {
 
 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. Leaving some room for future improvements, we'll set
+// the max frame rate to 60fps.
+// Histograms expect samples to be less than the boundary value, so set to 41.

[rkaplow, 2015/07/10 17:41:40]

61

[anandc, 2015/07/10 19:37:14]

Done.

+const int kMaxFrameRate = 61;
+
+// For bandwidth, based on expected real-world numbers, we'll use a histogram
+// ranging from 0 to 10MB/s, spread across 1000 buckets.

[rkaplow, 2015/07/10 17:41:40]

I feel 1000 buckets is excessive. I suspect you should be fine with less, such
as 100. Note that since this is log scaled (as you mention) you will get more
precision for the smaller bucket sizes.

[anandc, 2015/07/10 19:37:14]

Thanks. Done.

+// 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 = 10240000;
+const int kBandwidthHistogramNumBuckets = 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
 // entropy (see http://wiki.openssl.org/index.php/Random_Numbers), but stores
 // 1039 bytes of state, so we initialize it with 1k or random data.
 const int kRandomSeedSize = 1024;
 #endif  // defined(USE_OPENSSL)
 
 std::string ConnectionStateToString(protocol::ConnectionToHost::State state) {
   // Values returned by this function must match the
@@ skipping 635 matching lines @@
           host_public_key));
   scoped_ptr<protocol::Authenticator> authenticator(
       new protocol::NegotiatingClientAuthenticator(
           client_pairing_id, client_paired_secret, authentication_tag,
           fetch_secret_callback, token_fetcher.Pass(), auth_methods));
 
   // Kick off the connection.
   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.
   plugin_task_runner_->PostDelayedTask(
       FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
                             weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+      base::TimeDelta::FromSeconds(
+        ChromotingStats::kStatsUpdateFrequencyInSeconds));
 }
 
 void ChromotingInstance::HandleDisconnect(const base::DictionaryValue& data) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
   Disconnect();
 }
 
 void ChromotingInstance::HandleOnIncomingIq(const base::DictionaryValue& data) {
   std::string iq;
   if (!data.GetString("iq", &iq)) {
@@ skipping 299 matching lines @@
 }
 
 void ChromotingInstance::SendPerfStats() {
   if (!video_renderer_.get()) {
     return;
   }
 
   plugin_task_runner_->PostDelayedTask(
       FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
                             weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+      base::TimeDelta::FromSeconds(
+        ChromotingStats::kStatsUpdateFrequencyInSeconds));
 
+  // Update performance stats and send them to the client for display to users.
+  // The rate metrics are averaged over 1s, and the latency metrics are averaged
+  // over the 10 most recent samples.
   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("captureLatency", stats->video_capture_ms()->Average());
   data->SetDouble("encodeLatency", stats->video_encode_ms()->Average());
   data->SetDouble("decodeLatency", stats->video_decode_ms()->Average());
   data->SetDouble("renderLatency", stats->video_paint_ms()->Average());
   data->SetDouble("roundtripLatency", stats->round_trip_ms()->Average());
   PostLegacyJsonMessage("onPerfStats", data.Pass());
+
+  // Upload to UMA the video frame-rate, packet-rate and bandwidth stats,
+  // averaged over 1s.
+  pp::UMAPrivate uma(this);
+  uma.HistogramEnumeration("Chromoting.Video.FrameRate",
+    stats->video_frame_rate()->Rate(), kMaxFrameRate);
+  uma.HistogramEnumeration("Chromoting.Video.PacketRate",
+    stats->video_packet_rate()->Rate(), kMaxFrameRate);

[rkaplow, 2015/07/10 17:41:40]

is the max packet rate always same as frame rate? Should you have a different
max?

[anandc, 2015/07/10 19:37:14]

Good point.

They may not always be the same: we expect some empty packets in the packet-rate
which won't be counted in the frame-rate.
Still, I think the same max should be OK.
sergeyu@: WDYT?

+  uma.HistogramCustomCounts("Chromoting.Video.Bandwidth",
+    stats->video_bandwidth()->Rate(), kBandwidthHistogramMin,
+    kBandwidthHistogramMax, kBandwidthHistogramNumBuckets);
 }
 
 // static
 void ChromotingInstance::RegisterLogMessageHandler() {
   base::AutoLock lock(g_logging_lock.Get());
 
   VLOG(1) << "Registering global log handler";
 
   // Record previous handler so we can call it in a chain.
   g_logging_old_handler = logging::GetLogMessageHandler();
@@ skipping 119 matching lines @@
 
 #if !defined(OS_NACL)
   // Log messages are forwarded to the webapp only in PNaCl version of the
   // plugin, so ProcessLogToUI() needs to be called explicitly in the non-PNaCl
   // version.
   ProcessLogToUI(message);
 #endif  // !defined(OS_NACL)
 }
 
 }  // namespace remoting