Geolocation: add UMA histograms for events and response codes.

content/browser/geolocation/network_location_request.cc

 // Copyright (c) 2012 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 "content/browser/geolocation/network_location_request.h"
 
 #include <set>
 #include <string>
 
 #include "base/json/json_reader.h"
@@ skipping 13 matching lines @@
 
 namespace content {
 namespace {
 
 const char kAccessTokenString[] = "accessToken";
 const char kLocationString[] = "location";
 const char kLatitudeString[] = "lat";
 const char kLongitudeString[] = "lng";
 const char kAccuracyString[] = "accuracy";
 
+// NOTE: Never remove or reorder any of these as that would confuse
+// interpretation of previously logged data. Make sure to update the enum list
+// in tools/metrics/histograms/histograms.xml to keep it in sync.

[timvolodine, 2013/09/30 16:38:49]

side-note: to slightly help this problem we could assign values to them like =0,
=1.. That way deleting entries would be ok. Also, it looks like a certain
numbering is given in the .xml file associated with this patch.

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Done.

+enum NetworkLocationRequestEvent {
+  NETWORK_LOCATION_REQUEST_EVENT_REQUEST_START,
+  NETWORK_LOCATION_REQUEST_EVENT_REQUEST_CANCEL,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_SUCCESS,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_NOT_OK,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_EMPTY,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_MALFORMED,
+  NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_INVALID_FIX,
+
+  // NOTE: Add entries only immediately above this line.
+  NETWORK_LOCATION_REQUEST_EVENT_COUNT
+};
+
+void RecordUmaEvent(NetworkLocationRequestEvent event) {
+  UMA_HISTOGRAM_ENUMERATION("Geolocation.NetworkLocationRequest.Event",
+                            event, NETWORK_LOCATION_REQUEST_EVENT_COUNT);

[timvolodine, 2013/09/30 16:38:49]

indent

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Done.

+}
+
+const int kCodeRangeStart = 100; // Range start is inclusive.
+const int kCodeRangeEnd = 600; // Range end is exclusive.
+const int kCodeBuckets = 502; // 500 codes, underflow, and overflow.
+void RecordUmaResponseCode(int code) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Geolocation.NetworkLocationRequest.ResponseCode",
+      code, kCodeRangeStart, kCodeRangeEnd, kCodeBuckets);
+}
+
 // Local functions
 // Creates the request url to send to the server.
 GURL FormRequestURL(const GURL& url);
 
 void FormUploadData(const WifiData& wifi_data,
                     const base::Time& timestamp,
                     const string16& access_token,
                     std::string* upload_data);
 
-// Parsers the server response.
+// Attempts to extract a position from the response. Detects and indicates
+// various failure cases.
 void GetLocationFromResponse(bool http_post_result,
                              int status_code,
                              const std::string& response_body,
                              const base::Time& timestamp,
                              const GURL& server_url,
                              Geoposition* position,
                              string16* access_token);
 
 // Parses the server response body. Returns true if parsing was successful.
 // Sets |*position| to the parsed location if a valid fix was received,
@@ skipping 17 matching lines @@
           callback_(callback),
           url_(url) {
 }
 
 NetworkLocationRequest::~NetworkLocationRequest() {
 }
 
 bool NetworkLocationRequest::MakeRequest(const string16& access_token,
                                          const WifiData& wifi_data,
                                          const base::Time& timestamp) {
+  RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_REQUEST_START);
   if (url_fetcher_ != NULL) {
     DVLOG(1) << "NetworkLocationRequest : Cancelling pending request";
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_REQUEST_CANCEL);
     url_fetcher_.reset();
   }
   wifi_data_ = wifi_data;
   timestamp_ = timestamp;
 
   GURL request_url = FormRequestURL(url_);
   url_fetcher_.reset(net::URLFetcher::Create(
       url_fetcher_id_for_tests, request_url, net::URLFetcher::POST, this));
   url_fetcher_->SetRequestContext(url_context_.get());
   std::string upload_data;
   FormUploadData(wifi_data, timestamp, access_token, &upload_data);
   url_fetcher_->SetUploadData("application/json", upload_data);
   url_fetcher_->SetLoadFlags(
       net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
       net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_COOKIES |
       net::LOAD_DO_NOT_SEND_AUTH_DATA);
 
   start_time_ = base::TimeTicks::Now();
   url_fetcher_->Start();
   return true;
 }
 
 void NetworkLocationRequest::OnURLFetchComplete(
     const net::URLFetcher* source) {
   DCHECK_EQ(url_fetcher_.get(), source);
 
   net::URLRequestStatus status = source->GetStatus();
   int response_code = source->GetResponseCode();
+  RecordUmaResponseCode(response_code);
 
   Geoposition position;
   string16 access_token;
   std::string data;
   source->GetResponseAsString(&data);
   GetLocationFromResponse(status.is_success(),
                           response_code,
                           data,
                           timestamp_,
                           source->GetURL(),
@@ skipping 131 matching lines @@
                              const GURL& server_url,
                              Geoposition* position,
                              string16* access_token) {
   DCHECK(position);
   DCHECK(access_token);
 
   // HttpPost can fail for a number of reasons. Most likely this is because
   // we're offline, or there was no response.
   if (!http_post_result) {
     FormatPositionError(server_url, "No response received", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_EMPTY);
     return;
   }
   if (status_code != 200) {  // HTTP OK.
     std::string message = "Returned error code ";
     message += base::IntToString(status_code);
     FormatPositionError(server_url, message, position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_NOT_OK);
     return;
   }
   // We use the timestamp from the wifi data that was used to generate
   // this position fix.
   if (!ParseServerResponse(response_body, timestamp, position, access_token)) {
     // We failed to parse the repsonse.
     FormatPositionError(server_url, "Response was malformed", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_MALFORMED);
     return;
   }
   // The response was successfully parsed, but it may not be a valid
   // position fix.
   if (!position->Validate()) {
     FormatPositionError(server_url,
                         "Did not provide a good position fix", position);
+    RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_INVALID_FIX);
     return;
   }
+  RecordUmaEvent(NETWORK_LOCATION_REQUEST_EVENT_RESPONSE_SUCCESS);
+  return;

[bulach, 2013/09/30 16:27:21]

nit: remove return

[Michael van Ouwerkerk, 2013/09/30 17:02:22]

Oops, experiment leftover. Done.

 }
 
 // Numeric values without a decimal point have type integer and IsDouble() will
 // return false. This is convenience function for detecting integer or floating
 // point numeric values. Note that isIntegral() includes boolean values, which
 // is not what we want.
 bool GetAsDouble(const base::DictionaryValue& object,
                  const std::string& property_name,
                  double* out) {
   DCHECK(out);
@@ skipping 83 matching lines @@
 
   // Other fields are optional.
   GetAsDouble(*response_object, kAccuracyString, &position->accuracy);
 
   return true;
 }
 
 }  // namespace
 
 }  // namespace content