Adds UMA to measure when the data reduction proxy via header is missing

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc

Index: components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
index 6a35460833ea1746fe69116e333fe2d9776d0b96..7b9b8978b2415bdfbffbb97a55d2cce3fca0ed8f 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
@@ -9,8 +9,12 @@
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
+#include "base/prefs/pref_member.h"
 #include "components/data_reduction_proxy/common/data_reduction_proxy_headers.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/http/http_util.h"
 #include "net/proxy/proxy_retry_info.h"
 #include "net/proxy/proxy_server.h"
 #include "net/proxy/proxy_service.h"
@@ -72,6 +76,29 @@ void DataReductionProxyUsageStats::RecordDataReductionProxyBypassInfo(
   }
 }
 
+// static
+void DataReductionProxyUsageStats::DetectAndRecordMissingViaHeaderResponseCode(
+      bool is_primary, const net::HttpResponseHeaders* headers) {

[Alexei Svitkine (slow), 2014/09/23 15:12:13]

Nit: 1 param per line if first one is wrapped.

[sclittle, 2014/09/23 18:13:41]

Done.

+  if (HasDataReductionProxyViaHeader(headers, NULL)) {
+    // The data reduction proxy via header is present, so don't record anything.
+    return;
+  }
+
+  // Record the response code in the same way that net::HttpResponseHeaders
+  // records the response code for the histogram Net.HttpResponseCode.
+  if (is_primary) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(

[Alexei Svitkine (slow), 2014/09/23 15:12:13]

It's better to use an UMA_HISTOGRAM_SPARSE_SLOWLY() when logging response codes
- that way the underlying histogram doesn't preallocate an array for all
possible response codes. Then, you don't need the util functions you call below.

[sclittle, 2014/09/23 18:13:41]

Done. I'll keep the call to MapStatusCodeForHistogram though, since the response
code could conceivably be any non-negative integer specified by a server.

[Alexei Svitkine (slow), 2014/09/23 18:20:06]

Sorry, I don't understand what you mean for your reason for keeping
MapStatusCodeForHistogram. The sparse macro allows logging any non-negative
values, so I don't think the extra MapStatusCodeForHistogram() call is needed,
but perhaps I'm just not understanding your motivation. Could you elaborate?

[sclittle, 2014/09/23 19:58:30]

On second thought, you're right, we don't need this call. The idea behind
keeping the call was to prevent the logging of response codes that don't mean
anything anyway, but we don't need to be filtering those out here in Chrome.
I've removed the call.

+        "DataReductionProxy.MissingViaHeaderResponseCodePrimary",

[Alexei Svitkine (slow), 2014/09/23 15:12:13]

Consider adding .'s to delimit different parts of the name, so that you get
better grouping on the dashboard.

e.g.:

DataReductionProxy.MissingViaHeader.ResponseCode.Primary
DataReductionProxy.MissingViaHeader.ResponseCode.Fallback

And similarly for the ones below.

[sclittle, 2014/09/23 18:13:41]

Done.

+        net::HttpUtil::MapStatusCodeForHistogram(headers->response_code()),
+        net::HttpUtil::GetStatusCodesForHistogram());
+  } else {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "DataReductionProxy.MissingViaHeaderResponseCodeFallback",
+        net::HttpUtil::MapStatusCodeForHistogram(headers->response_code()),
+        net::HttpUtil::GetStatusCodesForHistogram());
+  }
+}
+
 DataReductionProxyUsageStats::DataReductionProxyUsageStats(
     DataReductionProxyParams* params,
     MessageLoopProxy* ui_thread_proxy)
@@ -159,22 +186,31 @@ void DataReductionProxyUsageStats::SetBypassType(
   triggering_request_ = true;
 }
 
+void DataReductionProxyUsageStats::RecordBytesHistograms(
+    net::URLRequest* request,
+    const BooleanPrefMember& data_reduction_proxy_enabled,
+    const net::ProxyConfig& data_reduction_proxy_config) {
+  RecordBypassedBytesHistograms(request, data_reduction_proxy_enabled,
+                                data_reduction_proxy_config);
+  RecordMissingViaHeaderBytes(request);
+}
+
 void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
-    net::URLRequest& request,
+    net::URLRequest* request,
     const BooleanPrefMember& data_reduction_proxy_enabled,
     const net::ProxyConfig& data_reduction_proxy_config) {
-  int64 content_length = request.received_response_content_length();
+  int64 content_length = request->received_response_content_length();
 
   if (data_reduction_proxy_enabled.GetValue() &&
       !data_reduction_proxy_config.Equals(
-          request.context()->proxy_service()->config())) {
+          request->context()->proxy_service()->config())) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::MANAGED_PROXY_CONFIG,
                         content_length);
     return;
   }
 
-  if (data_reduction_proxy_params_->WasDataReductionProxyUsed(&request, NULL)) {
+  if (data_reduction_proxy_params_->WasDataReductionProxyUsed(request, NULL)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::NOT_BYPASSED,
                         content_length);
@@ -182,7 +218,7 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
   }
 
   if (data_reduction_proxy_enabled.GetValue() &&
-      request.url().SchemeIs(url::kHttpsScheme)) {
+      request->url().SchemeIs(url::kHttpsScheme)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::SSL,
                         content_length);
@@ -191,7 +227,7 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
 
   if (data_reduction_proxy_enabled.GetValue() &&
       data_reduction_proxy_params_->IsBypassedByDataReductionProxyLocalRules(
-          request, data_reduction_proxy_config)) {
+          *request, data_reduction_proxy_config)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::LOCAL_BYPASS_RULES,
                         content_length);
@@ -205,7 +241,7 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
       last_bypass_type_ ==  BYPASS_EVENT_TYPE_MEDIUM ||
       last_bypass_type_ ==  BYPASS_EVENT_TYPE_LONG)) {
     std::string mime_type;
-    request.GetMimeType(&mime_type);
+    request->GetMimeType(&mime_type);
     // MIME types are named by <media-type>/<subtype>. Check to see if the
     // media type is audio or video. Only record when triggered by short bypass,
     // there isn't an audio or video bucket for medium or long bypasses.
@@ -232,7 +268,7 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
     return;
   }
 
-  if (data_reduction_proxy_params_->AreDataReductionProxiesBypassed(request,
+  if (data_reduction_proxy_params_->AreDataReductionProxiesBypassed(*request,
                                                                     NULL)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::NETWORK_ERROR,
@@ -378,6 +414,31 @@ void DataReductionProxyUsageStats::RecordBypassedBytes(
   }
 }
 
+void DataReductionProxyUsageStats::RecordMissingViaHeaderBytes(
+    URLRequest* request) {
+  // Responses that were served from cache should have been filtered out
+  // already.
+  DCHECK(!request->was_cached());
+
+  if (!data_reduction_proxy_params_->WasDataReductionProxyUsed(request, NULL) ||
+      HasDataReductionProxyViaHeader(request->response_headers(), NULL)) {
+    // Only track requests that used the data reduction proxy and had responses
+    // that were missing the data reduction proxy via header.
+    return;
+  }
+
+  if (request->GetResponseCode() >= net::HTTP_BAD_REQUEST &&
+      request->GetResponseCode() < net::HTTP_INTERNAL_SERVER_ERROR) {
+    // Track 4xx responses that are missing via headers separately.
+    UMA_HISTOGRAM_COUNTS("DataReductionProxy.MissingViaHeader4xxResponseBytes",
+                         request->received_response_content_length());
+  } else {
+    UMA_HISTOGRAM_COUNTS(
+        "DataReductionProxy.MissingViaHeaderOtherResponseBytes",
+        request->received_response_content_length());
+  }
+}
+
 }  // namespace data_reduction_proxy