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..877ebe4d036506bd24c2058b1d4042e2a27541a2 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,26 @@ 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 (GCMRegistrationRequestStatus
+// 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.
+  FAILED_PARSING_RESPONSE,    // Failed to parse the checkin response.
+  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
+};
+
 }  // namespace
 
 CheckinRequest::CheckinRequest(
@@ -94,6 +115,9 @@ 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.";
+    UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",

[Alexei Svitkine (slow), 2014/02/12 17:49:05]

Nit: Instead of repeating this histogram macro in many places, can you make a
helper function that just takes the CheckinRequestStatus enum value as a
parameter and logs this histogram? The macro expands to a lot of code and its
better for generated machine code size to not repeat it too much, if possible.

[juyik, 2014/02/12 18:15:41]

Done.

+                              URL_FETCHING_FAILED,
+                              STATUS_COUNT);
     RetryWithBackoff(true);
     return;
   }
@@ -106,6 +130,15 @@ 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) {
+      UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",
+                                HTTP_BAD_REQUEST,
+                                STATUS_COUNT);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",
+                                HTTP_UNAUTHORIZED,
+                                STATUS_COUNT);
+    }
     callback_.Run(0,0);
     return;
   }
@@ -115,6 +148,15 @@ 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) {
+      UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",
+                                HTTP_NOT_OK,
+                                STATUS_COUNT);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",
+                                FAILED_PARSING_RESPONSE,
+                                STATUS_COUNT);
+    }
     RetryWithBackoff(true);
     return;
   }
@@ -124,10 +166,14 @@ 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.";
+    UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus",
+                              ZERO_ID_OR_TOKEN,
+                              STATUS_COUNT);
     RetryWithBackoff(true);
     return;
   }
 
+  UMA_HISTOGRAM_ENUMERATION("GCM.CheckinRequestStatus", SUCCESS, STATUS_COUNT);
   callback_.Run(response_proto.android_id(), response_proto.security_token());
 }