[NTP Snippets] Add metric for fetch error cases

components/ntp_snippets/ntp_snippets_fetcher.cc

Index: components/ntp_snippets/ntp_snippets_fetcher.cc
diff --git a/components/ntp_snippets/ntp_snippets_fetcher.cc b/components/ntp_snippets/ntp_snippets_fetcher.cc
index 7fe3d94b0e54b22c86e052357ff1bf1e2a1f0f48..63826de5a72857903a2615c7749ad6d8189b00d8 100644
--- a/components/ntp_snippets/ntp_snippets_fetcher.cc
+++ b/components/ntp_snippets/ntp_snippets_fetcher.cc
@@ -6,6 +6,7 @@
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -120,12 +121,21 @@ void NTPSnippetsFetcher::OnURLFetchComplete(const URLFetcher* source) {
 
   std::string message;
   const URLRequestStatus& status = source->GetStatus();
+
   if (!status.is_success()) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.FailedRequestErrorCode",
+                                -status.error());

[Marc Treib, 2016/05/02 14:41:22]

I assume the minus is because the error codes tend to be negative - are we sure
that's always the case? URLRequestStatus doesn't seem to say anything to that
effect?

[mastiz, 2016/05/02 15:11:44]

Although it's not explicitly documented, URLRequestStatus' constructor has a
DCHECK that verifies it's zero or negative. In fact, there's various histograms
that make this assumption.

[Marc Treib, 2016/05/02 15:20:56]

Alright then. Maybe worth a comment?

+
     message = base::StringPrintf(kStatusMessageURLRequestErrorFormat,
                                  status.error());
-  } else if (source->GetResponseCode() != net::HTTP_OK) {
-    message = base::StringPrintf(kStatusMessageHTTPErrorFormat,
-                                 source->GetResponseCode());
+  } else {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.FetchResponseCode",
+                                source->GetResponseCode());

[Marc Treib, 2016/05/02 14:41:22]

So this one is recorded on success too, which probably makes sense, but doesn't
match the CL description.

[mastiz, 2016/05/02 15:11:44]

You're right, the description was outdated, thanks for pointing out.

+
+    if (source->GetResponseCode() != net::HTTP_OK) {
+      message = base::StringPrintf(kStatusMessageHTTPErrorFormat,
+                                   source->GetResponseCode());
+    }
   }
 
   std::string response;