Add metrics for DHCP WPAD feature.

net/proxy/dhcp_proxy_script_fetcher_win.cc

 // Copyright (c) 2011 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 "net/proxy/dhcp_proxy_script_fetcher_win.h"
 
+#include "base/metrics/histogram.h"
+#include "base/perftimer.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/dhcp_proxy_script_adapter_fetcher_win.h"
 
 #include <winsock2.h>
 #include <iphlpapi.h>
 #pragma comment(lib, "iphlpapi.lib")
 
 namespace {
 
 // How long to wait at maximum after we get results (a PAC file or
 // knowledge that no PAC file is configured) from whichever network
 // adapter finishes first.
 const int kMaxWaitAfterFirstResultMs = 400;
 
+const unsigned long kGetAdaptersAddressesErrors[] = {
+  ERROR_ADDRESS_NOT_ASSOCIATED,
+  ERROR_BUFFER_OVERFLOW,
+  ERROR_INVALID_PARAMETER,
+  ERROR_NOT_ENOUGH_MEMORY,
+  ERROR_NO_DATA,
+
+  // We use this as a placeholder (in our result code UMA histogram)
+  // for any error we don't specifically include in this enumeration.
+  ERROR_GEN_FAILURE,
+};
+
+std::vector<int> GetAllGetAdaptersAddressesErrors() {
+  std::vector<int> errors;
+  for (size_t i = 0; i < arraysize(kGetAdaptersAddressesErrors); ++i) {
+    int error_code = kGetAdaptersAddressesErrors[i];
+    errors.push_back(error_code);
+    // Also add N+1 for each error, so the bucket that contains our expected
+    // error is of size 1. That way if we get unexpected error codes, they
+    // won't fall into the same buckets as the expected ones.
+    errors.push_back(error_code + 1);
+  }
+  return errors;
+}
+
+// Returns the passed-in error code if it is in kGetAdaptersAddressesErrors,
+// otherwise ERROR_GEN_FAILURE.
+unsigned long EnsureGetAdaptersAddressesError(unsigned long error_code) {

[eroman%chromium.org, 2011/05/18 20:33:27]

It shouldn't be necessary to map unexpected errorst o ERROR_GEN_FAILURE, since
in your enumeration list you have already made the buckets of size 1. If the
error falls outside of the expected ranges, we will see it regardless (just
won't know it's exact value.)

[Jói, 2011/05/19 14:39:49]

Ah, I misunderstood how the UMA enumerations worked. Fixed that.

+  for (size_t i = 0; i < arraysize(kGetAdaptersAddressesErrors); ++i) {
+    if (error_code == kGetAdaptersAddressesErrors[i])
+      return error_code;
+  }
+  return ERROR_GEN_FAILURE;
+}
+
 }  // namespace
 
 namespace net {
 
 DhcpProxyScriptFetcherWin::DhcpProxyScriptFetcherWin(
     URLRequestContext* url_request_context)
     : state_(STATE_START),
       ALLOW_THIS_IN_INITIALIZER_LIST(fetcher_callback_(
           this, &DhcpProxyScriptFetcherWin::OnFetcherDone)),
       num_pending_fetchers_(0),
       url_request_context_(url_request_context) {
   DCHECK(url_request_context_);
 }
 
 DhcpProxyScriptFetcherWin::~DhcpProxyScriptFetcherWin() {
-  Cancel();
+  // Count as user-initiated if we are not yet in STATE_DONE.
+  CancelImpl(true);
 }
 
 int DhcpProxyScriptFetcherWin::Fetch(string16* utf16_text,
                                      CompletionCallback* callback) {
   DCHECK(CalledOnValidThread());
   if (state_ != STATE_START && state_ != STATE_DONE) {
     NOTREACHED();
     return ERR_UNEXPECTED;
   }
 
+  fetch_start_time_ = base::TimeTicks::Now();
+
   std::set<std::string> adapter_names;
   if (!ImplGetCandidateAdapterNames(&adapter_names)) {
     return ERR_UNEXPECTED;
   }
   if (adapter_names.empty()) {
     return ERR_PAC_NOT_IN_DHCP;
   }
 
   state_ = STATE_NO_RESULTS;
 
   client_callback_ = callback;
   destination_string_ = utf16_text;
 
   for (std::set<std::string>::iterator it = adapter_names.begin();
        it != adapter_names.end();
        ++it) {
     DhcpProxyScriptAdapterFetcher* fetcher(ImplCreateAdapterFetcher());
     fetcher->Fetch(*it, &fetcher_callback_);
     fetchers_.push_back(fetcher);
   }
   num_pending_fetchers_ = fetchers_.size();
 
   return ERR_IO_PENDING;
 }
 
 void DhcpProxyScriptFetcherWin::Cancel() {
+  CancelImpl(true);
+}
+
+void DhcpProxyScriptFetcherWin::CancelImpl(bool user_initiated) {
   DCHECK(CalledOnValidThread());
 
   if (state_ != STATE_DONE) {
     wait_timer_.Stop();
     state_ = STATE_DONE;
 
     for (FetcherVector::iterator it = fetchers_.begin();
          it != fetchers_.end();
          ++it) {
       (*it)->Cancel();
     }
 
     fetchers_.reset();
+
+    if (user_initiated) {
+      // We only count this stat if the cancel was explicitly initiated by
+      // our client, and if we weren't already in STATE_DONE.
+      UMA_HISTOGRAM_TIMES("Net.DhcpWpadCancelTime",

[eroman%chromium.org, 2011/05/18 20:33:27]

I am not sure how useful the cancelation info will be, but ok.

[Jói, 2011/05/19 14:39:49]

Yeah - once I start seeing the stats I'll remove things that aren't providing
extra information, but would like to have more for now rather than less.

+                          base::TimeTicks::Now() - fetch_start_time_);
+    }
   }
 }
 
 std::string DhcpProxyScriptFetcherWin::GetFetcherName() const {
   DCHECK(CalledOnValidThread());
   return "win";
 }
 
 const GURL& DhcpProxyScriptFetcherWin::GetPacURL() const {
   DCHECK(CalledOnValidThread());
@@ skipping 31 matching lines @@
   if (state_ == STATE_NO_RESULTS) {
     state_ = STATE_SOME_RESULTS;
     wait_timer_.Start(
         base::TimeDelta::FromMilliseconds(ImplGetMaxWaitMs()),
         this, &DhcpProxyScriptFetcherWin::OnWaitTimer);
   }
 }
 
 void DhcpProxyScriptFetcherWin::OnWaitTimer() {
   DCHECK_EQ(state_, STATE_SOME_RESULTS);
+
+  // These are intended to help us understand whether our timeout may
+  // be too aggressive or not aggressive enough.
+  UMA_HISTOGRAM_COUNTS_100("Net.DhcpWpadNumAdaptersAtWaitTimer",
+                           fetchers_.size());
+  UMA_HISTOGRAM_COUNTS_100("Net.DhcpWpadNumPendingAdaptersAtWaitTimer",
+                           num_pending_fetchers_);
+
   TransitionToDone();
 }
 
 void DhcpProxyScriptFetcherWin::TransitionToDone() {
   DCHECK(state_ == STATE_NO_RESULTS || state_ == STATE_SOME_RESULTS);
 
   // Should have returned immediately at Fetch() if no adapters to check.
   DCHECK(!fetchers_.empty());
 
   // Scan twice for the result; once through the whole list for success,
@@ skipping 18 matching lines @@
          ++it) {
       if ((*it)->DidFinish()) {
         result = (*it)->GetResult();
         if (result != ERR_PAC_NOT_IN_DHCP) {
           break;
         }
       }
     }
   }
 
-  Cancel();
+  CancelImpl(false);
   DCHECK_EQ(state_, STATE_DONE);
   DCHECK(fetchers_.empty());
 
+  UMA_HISTOGRAM_TIMES("Net.DhcpWpadCompletionTime",
+                      base::TimeTicks::Now() - fetch_start_time_);
+
+  if (result != OK) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "Net.DhcpWpadFetchError", std::abs(result), GetAllErrorCodesForUma());
+  }
+
   client_callback_->Run(result);
 }
 
 DhcpProxyScriptAdapterFetcher*
     DhcpProxyScriptFetcherWin::ImplCreateAdapterFetcher() {
   return new DhcpProxyScriptAdapterFetcher(url_request_context_);
 }
 
 bool DhcpProxyScriptFetcherWin::ImplGetCandidateAdapterNames(
     std::set<std::string>* adapter_names) {
   return GetCandidateAdapterNames(adapter_names);
 }
 
 int DhcpProxyScriptFetcherWin::ImplGetMaxWaitMs() {
   return kMaxWaitAfterFirstResultMs;
 }
 
 bool DhcpProxyScriptFetcherWin::GetCandidateAdapterNames(
     std::set<std::string>* adapter_names) {
   DCHECK(adapter_names);
   adapter_names->clear();
 
   // The GetAdaptersAddresses MSDN page recommends using a size of 15000 to
   // avoid reallocation.
   ULONG adapters_size = 15000;
   scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> adapters;
   ULONG error = ERROR_SUCCESS;
   int num_tries = 0;
+
+  PerfTimer time_api_access;
   do {
     adapters.reset(
         reinterpret_cast<IP_ADAPTER_ADDRESSES*>(malloc(adapters_size)));
     // Return only unicast addresses, and skip information we do not need.
     error = GetAdaptersAddresses(AF_UNSPEC,
                                  GAA_FLAG_SKIP_ANYCAST |
                                  GAA_FLAG_SKIP_MULTICAST |
                                  GAA_FLAG_SKIP_DNS_SERVER |
                                  GAA_FLAG_SKIP_FRIENDLY_NAME,
                                  NULL,
                                  adapters.get(),
                                  &adapters_size);
     ++num_tries;
   } while (error == ERROR_BUFFER_OVERFLOW && num_tries <= 3);
 
+  // This is primarily to validate our belief that the GetAdaptersAddresses API
+  // function is fast enough to call synchronously from the network thread.
+  UMA_HISTOGRAM_TIMES("Net.DhcpWpadGetAdaptersAddressesTime",
+                      time_api_access.Elapsed());
+
+  if (error != ERROR_SUCCESS) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.DhcpWpadGetAdaptersAddressesError",
+                                     EnsureGetAdaptersAddressesError(error),
+                                     GetAllGetAdaptersAddressesErrors());
+  }
+
   if (error == ERROR_NO_DATA) {
     // There are no adapters that we care about.
     return true;
   }
 
   if (error != ERROR_SUCCESS) {
     LOG(WARNING) << "Unexpected error retrieving WPAD configuration from DHCP.";
     return false;
   }
 
   IP_ADAPTER_ADDRESSES* adapter = NULL;
   for (adapter = adapters.get(); adapter; adapter = adapter->Next) {
     if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK)
       continue;
     if ((adapter->Flags & IP_ADAPTER_DHCP_ENABLED) == 0)
       continue;
 
     DCHECK(adapter->AdapterName);
     adapter_names->insert(adapter->AdapterName);
   }
 
   return true;
 }
 
 }  // namespace net