Add histogram for HTTP response codes

net/http/http_response_headers.cc

Index: net/http/http_response_headers.cc
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 85df8d5e15f3c390a663dfad6eecb6857564ee59..39954365625a9552bb6b7474532150397c6282ad 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -12,6 +12,7 @@
 #include <algorithm>
 
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
@@ -85,6 +86,31 @@ bool ShouldUpdateHeader(const std::string::const_iterator& name_begin,
   return true;
 }
 
+// Functions for histogram initialization.  The code 0 is put in the
+// response map to track response codes that are invalid.
+
+enum { kHistogramMinHttpResponseCode = 100,
+       kHistogramMaxHttpResponseCode = 599 };

[jar (doing other things), 2011/01/21 20:37:46]

This is going to make a slightly large histogram.  Most common are 50-100
buckets, but this will be about 500 buckets.  Memory cost for these things is
around 4 bytes per bucket for a counter, plus about 4 bytes per bucket for the
range.  That totals around 2K bytes.  That gets doubled because we have to
snapshot this array, for about 4K, plus of course a bit of string name overhead
etc.

Can you motivate this gathering of data a bit?  Will you need to gather this
forever?

The amount of memory is not the end of the world, but a death by a million cuts
is achieved one slice at a time.

[gavinp, 2011/01/21 21:38:19]

I appreciate your point Jim.  I do not believe we'll need to gather this
forever.  The goal is to learn which HTTP response codes are used in the wild;
for prerendering we might need to add a new HTTP response code, and I wanted to
learn which codes are used (in each category) in the wild before proposing a new
number.

I think eventually I'll be able to reduce this greatly: we can cut it back to
just the response codes that are actually used in practice, and add bins in each
category for "other", which we can then dynamically expand to find out what's
happening.  I'll add a TODO() to cut this back.

I believe the majority of bins we collect will be zero almost every time, but
since the point is to find out which ones aren't, initially (for a few weeks)
I'd like to leave it this wide.

+
+std::vector<int> GetAllHttpResponseCodes() {
+  std::vector<int> codes;

[cbentzel, 2011/01/21 20:33:25]

How expensive is this? 

It looks like this fairly large vector allocation and copy is done everytime
HttpResponseHeaders is called. Can you only generate it once? 

[jar (doing other things), 2011/01/21 20:40:36]

This is only done during the initialization of the HISTOGRAM.  The macro only
evals its arg once, creating a static ref to the instance, and will never
re-evaluate this.

On 2011/01/21 20:33:25, cbentzel wrote:

+  codes.reserve(
+      kHistogramMaxHttpResponseCode - kHistogramMinHttpResponseCode + 2);
+  codes.push_back(0);
+  for (int i = kHistogramMinHttpResponseCode;
+       i <= kHistogramMaxHttpResponseCode;++i)
+    codes.push_back(i);
+  return codes;
+}
+
+int MapHttpResponseCode(const int code) {
+  if (kHistogramMinHttpResponseCode <= code &&
+      code <= kHistogramMaxHttpResponseCode)
+    return code;

[jar (doing other things), 2011/01/21 20:37:46]

Probably want something more like 

code - kHistogramMinHttpResponseCode + 1

[jar (doing other things), 2011/01/21 20:40:36]

<doh>  My mistake. Please ignore. The relocation of the numbers are handled by
the histogram.  <sorry... not thinking>

On 2011/01/21 20:37:46, jar wrote:

+  else

[jar (doing other things), 2011/01/21 20:37:46]

style nit: avoid else clauses when you returned in the previous clause.

[gavinp, 2011/01/21 21:38:19]

Done.

+    return 0;
+}
+
 }  // namespace
 
 struct HttpResponseHeaders::ParsedHeader {
@@ -103,6 +129,15 @@ struct HttpResponseHeaders::ParsedHeader {
 HttpResponseHeaders::HttpResponseHeaders(const std::string& raw_input)
     : response_code_(-1) {
   Parse(raw_input);
+
+  // The most important thing to do with this histogram is find out the
+  // existence of unusual HTTP response codes.  As it happens right now,
+  // there aren't double-constructions of response headers using this
+  // constructor, so our counts should also be accurate, without
+  // instantiating the histogram in two places.
+  UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.HttpResponseCode",
+                                   MapHttpResponseCode(response_code_),
+                                   GetAllHttpResponseCodes());
 }
 
 HttpResponseHeaders::HttpResponseHeaders(const Pickle& pickle, void** iter)