Adding UMA for data reduction proxy page load size/savings

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

 // Copyright 2016 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 "chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h"
 
 #include <string>
 
+#include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
+#include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "chrome/browser/loader/chrome_navigation_data.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
 #include "chrome/browser/page_load_metrics/page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
 #include "chrome/common/page_load_metrics/page_load_timing.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_pingback_client.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_page_load_timing.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 namespace data_reduction_proxy {
 
 namespace {
 
+std::string GetConstHistogramWithSuffix(const char* suffix) {
+  return std::string(internal::kHistogramDataReductionProxyPrefix)
+      .append(suffix);
+}
+
 // A macro is needed because PAGE_LOAD_HISTOGRAM creates a static instance of
 // the histogram. A distinct histogram is needed for each place that calls
 // RECORD_HISTOGRAMS_FOR_SUFFIX. |event| is the timing event representing when
 // |value| became available.
-#define RECORD_HISTOGRAMS_FOR_SUFFIX(data, value, histogram_suffix)       \
-  do {                                                                    \
-    PAGE_LOAD_HISTOGRAM(                                                  \
-        std::string(internal::kHistogramDataReductionProxyPrefix)         \
-            .append(histogram_suffix),                                    \
-        value);                                                           \
-    if (data->lofi_requested()) {                                         \
-      PAGE_LOAD_HISTOGRAM(                                                \
-          std::string(internal::kHistogramDataReductionProxyLoFiOnPrefix) \
-              .append(histogram_suffix),                                  \
-          value);                                                         \
-    }                                                                     \
+#define RECORD_HISTOGRAMS_FOR_SUFFIX(data, value, histogram_suffix)            \
+  do {                                                                         \
+    PAGE_LOAD_HISTOGRAM(GetConstHistogramWithSuffix(histogram_suffix), value); \
+    if (data->lofi_requested()) {                                              \
+      PAGE_LOAD_HISTOGRAM(                                                     \
+          std::string(internal::kHistogramDataReductionProxyLoFiOnPrefix)      \
+              .append(histogram_suffix),                                       \
+          value);                                                              \
+    }                                                                          \
   } while (false)
 
 }  // namespace
 
 namespace internal {
 
 const char kHistogramDataReductionProxyPrefix[] =
     "PageLoad.Clients.DataReductionProxy.";
 const char kHistogramDataReductionProxyLoFiOnPrefix[] =
     "PageLoad.Clients.DataReductionProxy.LoFiOn.";
@@ skipping 10 matching lines @@
 const char kHistogramFirstImagePaintSuffix[] =
     "PaintTiming.NavigationToFirstImagePaint";
 const char kHistogramFirstPaintSuffix[] = "PaintTiming.NavigationToFirstPaint";
 const char kHistogramFirstTextPaintSuffix[] =
     "PaintTiming.NavigationToFirstTextPaint";
 const char kHistogramParseStartSuffix[] = "ParseTiming.NavigationToParseStart";
 const char kHistogramParseBlockedOnScriptLoadSuffix[] =
     "ParseTiming.ParseBlockedOnScriptLoad";
 const char kHistogramParseDurationSuffix[] = "ParseTiming.ParseDuration";
 
+const char kRequestsPercentProxied[] =
+    "Experimental.Requests.Network.PercentProxied";
+const char kBytesPercentProxied[] = "Experimental.Bytes.Network.PercentProxied";
+const char kBytesCompressionRatio[] =
+    "Experimental.Bytes.Network.CompressionRatio";
+const char kBytesInflationPercent[] =
+    "Experimental.Bytes.Network.InflationPercent";
+const char kNetworkRequests[] = "Experimental.Requests.Network";
+const char kRequestsProxied[] = "Experimental.Requests.Network.Proxied";
+const char kRequestsNotProxied[] = "Experimental.Requests.Network.NonProxied";
+const char kNetworkBytes[] = "Experimental.Bytes.Network";
+const char kBytesProxied[] = "Experimental.Bytes.Network.Proxied";
+const char kBytesNotProxied[] = "Experimental.Bytes.Network.NonProxied";
+const char kBytesOriginal[] = "Experimental.Bytes.Network.Original";
+const char kBytesSavings[] = "Experimental.Bytes.Network.Savings";
+const char kBytesInflation[] = "Experimental.Bytes.Network.Inflation";
+
 }  // namespace internal
 
 DataReductionProxyMetricsObserver::DataReductionProxyMetricsObserver()
-    : browser_context_(nullptr) {}
+    : browser_context_(nullptr),
+      num_data_reduction_proxy_requests_(0),
+      num_network_requests_(0),
+      original_network_bytes_(0),
+      network_bytes_proxied_(0),
+      network_bytes_(0) {}
 
 DataReductionProxyMetricsObserver::~DataReductionProxyMetricsObserver() {}
 
 // Check if the NavigationData indicates anything about the DataReductionProxy.
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 DataReductionProxyMetricsObserver::OnCommit(
     content::NavigationHandle* navigation_handle) {
   // This BrowserContext is valid for the lifetime of
   // DataReductionProxyMetricsObserver. BrowserContext is always valid and
   // non-nullptr in NavigationControllerImpl, which is a member of WebContents.
@@ skipping 29 matching lines @@
     bool started_in_foreground) {
   if (!started_in_foreground)
     return STOP_OBSERVING;
   return CONTINUE_OBSERVING;
 }
 
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 DataReductionProxyMetricsObserver::OnHidden(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  RecordPageSizeUMA();
   SendPingback(timing, info);
   return STOP_OBSERVING;
 }
 
 page_load_metrics::PageLoadMetricsObserver::ObservePolicy
 DataReductionProxyMetricsObserver::FlushMetricsOnAppEnterBackground(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   // FlushMetricsOnAppEnterBackground is invoked on Android in cases where the
   // app is about to be backgrounded, as part of the Activity.onPause()
   // flow. After this method is invoked, Chrome may be killed without further
   // notification, so we send a pingback with data collected up to this point.
+  RecordPageSizeUMA();
   SendPingback(timing, info);
   return STOP_OBSERVING;
 }
 
 void DataReductionProxyMetricsObserver::OnComplete(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
+  RecordPageSizeUMA();
   SendPingback(timing, info);
 }
 
+void DataReductionProxyMetricsObserver::RecordPageSizeUMA() const {
+  // If the first request didn't complete, don't record UMA.
+  if (num_network_requests_ == 0) {

[Alexei Svitkine (slow), 2017/01/19 20:12:46]

Nit: No {}'s

[RyanSturm, 2017/01/19 20:43:04]

Done.

+    return;
+  }
+
+  // TODO(ryansturm): Evaluate if any of the below histograms are unncessary
+  // once data is available. crbug.com/682782
+
+  // The percent of requests that went through the data reduction proxy.
+  UMA_HISTOGRAM_PERCENTAGE(
+      GetConstHistogramWithSuffix(internal::kRequestsPercentProxied),
+      (100 * num_data_reduction_proxy_requests_) / num_network_requests_);

[Alexei Svitkine (slow), 2017/01/19 20:12:46]

Nit: Add the static_cast<int>() cast here if you're keeping the casts.

[RyanSturm, 2017/01/19 20:43:04]

I only want to cast int64_t->int, and this (and other num_request vars) would be
int->int.

+
+  // The percent of bytes that went through the data reduction proxy.
+  if (network_bytes_ > 0) {
+    UMA_HISTOGRAM_PERCENTAGE(
+        GetConstHistogramWithSuffix(internal::kBytesPercentProxied),
+        static_cast<int>((100 * network_bytes_proxied_) / network_bytes_));
+  }
+
+  // If the data reduction proxy caused savings, record the compression ratio;
+  // otherwise, record the inflation ratio.
+  if (original_network_bytes_ > 0 &&
+      original_network_bytes_ >= network_bytes_) {
+    UMA_HISTOGRAM_PERCENTAGE(
+        GetConstHistogramWithSuffix(internal::kBytesCompressionRatio),
+        static_cast<int>((100 * network_bytes_) / original_network_bytes_));
+  } else if (original_network_bytes_ > 0) {
+    // Inflation should never be above one hundred percent.
+    UMA_HISTOGRAM_PERCENTAGE(
+        GetConstHistogramWithSuffix(internal::kBytesInflationPercent),
+        static_cast<int>((100 * network_bytes_) / original_network_bytes_ -
+                         100));
+  }
+
+  // Record the number of network requests seen.
+  UMA_HISTOGRAM_COUNTS_10000(
+      GetConstHistogramWithSuffix(internal::kNetworkRequests),
+      num_network_requests_);
+
+  // Record the number of requests that used data reduction proxy.
+  UMA_HISTOGRAM_COUNTS_10000(
+      GetConstHistogramWithSuffix(internal::kRequestsProxied),
+      num_data_reduction_proxy_requests_);
+
+  // Record the number of requests that did not use data reduction proxy.
+  UMA_HISTOGRAM_COUNTS_10000(
+      GetConstHistogramWithSuffix(internal::kRequestsNotProxied),
+      num_network_requests_ - num_data_reduction_proxy_requests_);
+
+  // Record the total KB of network bytes. Capped at 500 MB.
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      GetConstHistogramWithSuffix(internal::kNetworkBytes),
+      static_cast<int>(network_bytes_ / 1024), 1, 500 * 1024, 50);

[Alexei Svitkine (slow), 2017/01/19 20:12:46]

Since you're re-using these macro params for all of these, suggest defining a
custom macro for it so you only need to specify them once.

Also, you can do the division by 1024 inside it to make the callsites cleaner.

[RyanSturm, 2017/01/19 20:43:04]

Done.

+
+  // Record the total amount of bytes that went through the data reduction
+  // proxy. Capped at 500 MB.
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      GetConstHistogramWithSuffix(internal::kBytesProxied),
+      static_cast<int>(network_bytes_proxied_ / 1024), 1, 500 * 1024, 50);
+
+  // Record the total amount of bytes that did not go through the data reduction
+  // proxy. Capped at 500 MB.
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      GetConstHistogramWithSuffix(internal::kBytesNotProxied),
+      static_cast<int>((network_bytes_ - network_bytes_proxied_) / 1024), 1,
+      500 * 1024, 50);
+
+  // Record the total KB of network bytes that the user would have seen without
+  // using data reduction proxy. Capped at 500 MB.
+  UMA_HISTOGRAM_CUSTOM_COUNTS(
+      GetConstHistogramWithSuffix(internal::kBytesOriginal),
+      static_cast<int>(original_network_bytes_ / 1024), 1, 500 * 1024, 50);
+
+  // Record the savings the user saw by using data reduction proxy. If there was
+  // inflation instead, record that. Capped at 500 MB.
+  if (network_bytes_ <= original_network_bytes_) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        GetConstHistogramWithSuffix(internal::kBytesSavings),
+        static_cast<int>((original_network_bytes_ - network_bytes_) / 1024), 1,
+        500 * 1024, 50);
+  } else {
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        GetConstHistogramWithSuffix(internal::kBytesInflation),
+        static_cast<int>((network_bytes_proxied_ - original_network_bytes_) /
+                         1024),
+        1, 500 * 1024, 50);
+  }
+}
+
 void DataReductionProxyMetricsObserver::SendPingback(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   // TODO(ryansturm): Move to OnFirstBackgroundEvent to handle some fast
   // shutdown cases. crbug.com/618072
   if (!browser_context_)
     return;
   if (data_reduction_proxy::params::IsIncludedInHoldbackFieldTrial() ||
       data_reduction_proxy::params::IsIncludedInTamperDetectionExperiment()) {
     return;
@@ skipping 114 matching lines @@
     const page_load_metrics::PageLoadExtraInfo& info) {
   base::TimeDelta parse_duration =
       timing.parse_stop.value() - timing.parse_start.value();
   RECORD_HISTOGRAMS_FOR_SUFFIX(data_, parse_duration,
                                internal::kHistogramParseDurationSuffix);
   RECORD_HISTOGRAMS_FOR_SUFFIX(
       data_, timing.parse_blocked_on_script_load_duration.value(),
       internal::kHistogramParseBlockedOnScriptLoadSuffix);
 }
 
+void DataReductionProxyMetricsObserver::OnLoadedResource(
+    const page_load_metrics::ExtraRequestInfo& extra_request_info) {
+  if (extra_request_info.was_cached)
+    return;
+  original_network_bytes_ += extra_request_info.original_network_content_length;
+  network_bytes_ += extra_request_info.raw_body_bytes;
+  num_network_requests_++;
+  if (!extra_request_info.data_reduction_proxy_used)
+    return;
+  num_data_reduction_proxy_requests_++;
+  network_bytes_proxied_ += extra_request_info.raw_body_bytes;
+}
+
 DataReductionProxyPingbackClient*
 DataReductionProxyMetricsObserver::GetPingbackClient() const {
   return DataReductionProxyChromeSettingsFactory::GetForBrowserContext(
              browser_context_)
       ->data_reduction_proxy_service()
       ->pingback_client();
 }
 
 }  // namespace data_reduction_proxy