Add GCM check in status code metric collection. We also decided to collect registration success sig…

google_apis/gcm/engine/checkin_request.cc

Index: google_apis/gcm/engine/checkin_request.cc
diff --git a/google_apis/gcm/engine/checkin_request.cc b/google_apis/gcm/engine/checkin_request.cc
index b46415ab96ffb48e073e7e9ff420e5816f445551..aceecb5d6b557937537575cb3ec97646272bfd71 100644
--- a/google_apis/gcm/engine/checkin_request.cc
+++ b/google_apis/gcm/engine/checkin_request.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram.h"
 #include "google_apis/gcm/protocol/checkin.pb.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
@@ -18,6 +19,30 @@ namespace {
 const char kCheckinURL[] = "https://android.clients.google.com/checkin";
 const char kRequestContentType[] = "application/x-protobuf";
 const int kRequestVersionValue = 2;
+
+// This enum is also used in an UMA histogram (GCMCheckinRequestStatus
+// enum defined in tools/metrics/histograms/histogram.xml). Hence the entries
+// here shouldn't be deleted or re-ordered and new ones should be added to
+// the end.
+enum CheckinRequestStatus {
+  SUCCESS,                    // Checkin completed successfully.
+  URL_FETCHING_FAILED,        // URL fetching failed.
+  HTTP_BAD_REQUEST,           // The request was malformed.
+  HTTP_UNAUTHORIZED,          // The security token didn't match the android id.
+  HTTP_NOT_OK,                // HTTP status was not OK.
+  RESPONSE_PARSING_FAILED,    // Check in response parsing failed.
+  ZERO_ID_OR_TOKEN,           // Either returned android id or security token
+                              // was zero.
+  // NOTE: always keep this entry at the end. Add new status types only
+  // immediately above this line. Make sure to update the corresponding
+  // histogram enum accordingly.
+  STATUS_COUNT
+};
+
+void RecordCheckinStatusToUMA(CheckinRequestStatus status) {
+  UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus", status, STATUS_COUNT);
+}
+
 }  // namespace
 
 CheckinRequest::CheckinRequest(
@@ -94,6 +119,7 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
   checkin_proto::AndroidCheckinResponse response_proto;
   if (!source->GetStatus().is_success()) {
     LOG(ERROR) << "Failed to get checkin response. Fetcher failed. Retrying.";
+    RecordCheckinStatusToUMA(URL_FETCHING_FAILED);
     RetryWithBackoff(true);
     return;
   }
@@ -106,6 +132,11 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
     // UNAUTHORIZED indicates that security token didn't match the android id.
     LOG(ERROR) << "No point retrying the checkin with status: "
                << response_status << ". Checkin failed.";
+    if (response_status == net::HTTP_BAD_REQUEST) {
+      RecordCheckinStatusToUMA(HTTP_BAD_REQUEST);
+    } else {

[jianli, 2014/02/12 18:41:05]

nit: bracket is not needed for single line if/else.

Even better, you could simplify this block to:
RecordCheckinStatusToUMA(response_status == net::HTTP_BAD_REQUEST ?
HTTP_BAD_REQUEST : HTTP_UNAUTHORIZED);

[juyik, 2014/02/12 18:55:20]

Done.

+      RecordCheckinStatusToUMA(HTTP_UNAUTHORIZED);
+    }
     callback_.Run(0,0);
     return;
   }
@@ -115,6 +146,11 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
       !response_proto.ParseFromString(response_string)) {
     LOG(ERROR) << "Failed to get checkin response. HTTP Status: "
                << response_status << ". Retrying.";
+    if (response_status != net::HTTP_OK) {
+      RecordCheckinStatusToUMA(HTTP_NOT_OK);
+    } else {

[jianli, 2014/02/12 18:41:05]

ditto

[juyik, 2014/02/12 18:55:20]

Done.

+      RecordCheckinStatusToUMA(RESPONSE_PARSING_FAILED);
+    }
     RetryWithBackoff(true);
     return;
   }
@@ -124,10 +160,12 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
       response_proto.android_id() == 0 ||
       response_proto.security_token() == 0) {
     LOG(ERROR) << "Android ID or security token is 0. Retrying.";
+    RecordCheckinStatusToUMA(ZERO_ID_OR_TOKEN);
     RetryWithBackoff(true);
     return;
   }
 
+  RecordCheckinStatusToUMA(SUCCESS);
   callback_.Run(response_proto.android_id(), response_proto.security_token());
 }