Domain Reliability: Histogram net error codes from uploads

components/domain_reliability/uploader.cc

 // Copyright 2014 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 "components/domain_reliability/uploader.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/stl_util.h"
 #include "base/supports_user_data.h"
 #include "net/base/load_flags.h"
+#include "net/base/net_errors.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace domain_reliability {
 
 namespace {
 
 const char* kJsonMimeType = "application/json; charset=utf-8";
 
@@ skipping 62 matching lines @@
     VLOG(1) << "Setting discard_uploads to " << discard_uploads;
   }
 
   // net::URLFetcherDelegate implementation:
   void OnURLFetchComplete(const net::URLFetcher* fetcher) override {
     DCHECK(fetcher);
 
     UploadCallbackMap::iterator callback_it = upload_callbacks_.find(fetcher);
     DCHECK(callback_it != upload_callbacks_.end());
 
-    VLOG(1) << "Upload finished with " << fetcher->GetResponseCode();
+    int net_error;
+    {
+      const net::URLRequestStatus& status = fetcher->GetStatus();
+      if (status.status() == net::URLRequestStatus::FAILED)
+        net_error = status.error();
+      else
+        net_error = net::OK;

[davidben, 2014/10/30 22:26:55]

+mmenke: FYI, here's another one of these. We really should go through and make
it so that you can query status.error() directly or something.

[mmenke, 2014/10/30 22:42:59]

I'd like to get rid of the double-value status entirely, and just make it a
NetError.

Sadly, to be sure this works properly, this code should probably check for:

status.status() == net::URLRequestStatus::FAILED && status.error() !=
net::ERR_ABORTED

[Deprecated (see juliatuttle), 2014/10/31 18:54:01]

Why does ERR_ABORTED not count?

[mmenke, 2014/10/31 18:55:55]

The same reason you're not counting net::URLRequestStatus:CANCELED.  IF you want
to count cancelled requests, should count those, too.  Don't ask why we have two
ways to indicate a request was cancelled.

[Deprecated (see juliatuttle), 2014/10/31 21:25:39]

Alright, I'm counting CANCELED as ERR_ABORTED.

+    }
+    int http_response_code = fetcher->GetResponseCode();
+
+    VLOG(1) << "Upload finished with net error " << net_error <<
+               " and HTTP response code " << http_response_code;
 
     UMA_HISTOGRAM_SPARSE_SLOWLY("DomainReliability.UploadResponseCode",
-                                fetcher->GetResponseCode());
+                                http_response_code);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("DomainReliability.UploadNetError",
+                                -net_error);

[davidben, 2014/10/30 22:26:55]

Should this be

UMA_HISTOGRAM_CUSTOM_ENUMERATION("DomainReliability.UploadNetError",
                                 -net_error,
                                 net::GetAllErrorCodesForUma());

We're not very consistent about this. Perhaps ask the histograms.xml reviewer
which is preferred?

[Deprecated (see juliatuttle), 2014/10/31 18:54:01]

This is rare enough that I think a sparse one will save a lot of RAM.

 
-    bool success = fetcher->GetResponseCode() == 200;
-    callback_it->second.Run(success);
+    callback_it->second.Run(http_response_code == 200);
 
     delete callback_it->first;
     upload_callbacks_.erase(callback_it);
   }
 
  private:
   using DomainReliabilityUploader::UploadCallback;
   typedef std::map<const net::URLFetcher*, UploadCallback> UploadCallbackMap;
 
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
@@ skipping 14 matching lines @@
       new DomainReliabilityUploaderImpl(url_request_context_getter));
 }
 
 // static
 bool DomainReliabilityUploader::URLRequestIsUpload(
     const net::URLRequest& request) {
   return request.GetUserData(UploadUserData::kUserDataKey) != NULL;
 }
 
 }  // namespace domain_reliability