Measure network error rates with and without data reduction proxy

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 5a79df93a6a956d70f40f44c0d29fd73f1467289..563a4be74e816d2d17e499120a2386a1844a114b 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -15,6 +15,7 @@
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_member.h"
@@ -40,6 +41,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_request_info.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/cookies/canonical_cookie.h"
@@ -150,6 +152,21 @@ void ReportInvalidReferrerSend(const GURL& target_url,
   NOTREACHED();
 }
 
+// Record network errors that HTTP requests complete with, including OK and
+// ABORTED.
+void RecordNetworkErrorHistograms(const net::URLRequest* request) {
+  if (request->url().SchemeIs("http")) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Net.HttpRequestCompletionErrorCodes",
+                                std::abs(request->status().error()));
+
+    if (request->load_flags() & net::LOAD_MAIN_FRAME) {
+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Net.HttpRequestCompletionErrorCodes.MainFrame",

[mmenke, 2014/12/12 21:09:30]

Not needed.  See Net.ErrorCodesForMainFrame3 and Net.ErrorCodesForSubresources2

[sclittle, 2014/12/12 22:10:20]

Please clarify, for "not needed", are you referring to both the histograms here
or just the MainFrame one?

The idea behind having these histograms is that we could compare net error rates
for HTTP requests between users who have the data reduction proxy enabled and
users who don't. By looking at these histograms together with
DRP.RequestCompletionErrorCodes, we could work out what the net error rates are
for requests that bypass the DRP, and get a better overall idea of the effects
of network flakiness on DRP usage.

The idea behind drilling down on the main frame requests is that when a client
is on a network on which the DRP isn't immediately usable (e.g. captive portal),
errors caused by this would typically show up on main frame resource loads, and
since these captive portals or other things could handle HTTP traffic
differently than other traffic (such as HTTPS), it would be nice to look at just
HTTP main frame request net error rates. Also, from looking at
Net.ErrorCodesForMainFrame3 and Net.ErrorCodesForSubresources2, the rates of
some errors between these two differ in interesting ways, so it could be useful
to narrow that down to just HTTP requests so we can better apply this data to
DRP usage.

If you believe that the existing Net.ErrorCodesForMainFrame3 and
Net.ErrorCodesForSubresources2 histograms already allow for this sort of thing,
then I'll remove these new ones.

[mmenke, 2014/12/15 16:55:43]

Ahh...you're right, they're different, I missed the http restriction.  Suppose
these also cover requests that don't come through the RDH.  This does include
incognito requests, though, which would bypass the DRP even if it were enabled,
so not completely comparable, but close.

I'm still not a huge fan of having all these near identical histograms logged
all over the place.  Histograms aren't free.  Don't have a better suggestion,
though.

Could just compare the other histograms with DRP on and off, but that doesn't
give you the fallback vs non-fallback error numbers, I suppose.

+          std::abs(request->status().error()));
+    }
+  }
+}
+
 }  // namespace
 
 ChromeNetworkDelegate::ChromeNetworkDelegate(
@@ -385,6 +402,8 @@ void ChromeNetworkDelegate::OnRawBytesRead(const net::URLRequest& request,
 
 void ChromeNetworkDelegate::OnCompleted(net::URLRequest* request,
                                         bool started) {
+  RecordNetworkErrorHistograms(request);
+
   TRACE_EVENT_ASYNC_END0("net", "URLRequest", request);
   if (request->status().status() == net::URLRequestStatus::SUCCESS) {
 #if defined(OS_ANDROID)