DNS histogram cleanup, part 2

This CL merges the higher-level DNS histograms to include both async and
system resolver timing in the same metrics. This will allow easier use
in Finch experiments. It also adds the new Net.DNS.TotalTimeNotCached
histogram, which is equivalent to Net.DNS.TotalTime minus cached and
other synchronous resolutions, for better alerts. The current TotalTime
histogram is hard to use for alerts because the median is 0.

Bug: 769320
Change-Id: Iea7873ddd284fffe1e0e414f1e33f2e07053c0ca
Reviewed-on: https://chromium-review.googlesource.com/673092
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Reviewed-by: Julia Tuttle <juliatuttle@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505119}

net/dns/host_resolver_impl.cc

@@ -236,42 +236,43 @@ bool ResemblesMulticastDNSName(const std::string& hostname) {
   do {                                                             \
     switch (priority) {                                            \
       case HIGHEST:                                                \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_HIGHEST", time);   \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".HIGHEST", time);   \
         break;                                                     \
       case MEDIUM:                                                 \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_MEDIUM", time);    \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".MEDIUM", time);    \
         break;                                                     \
       case LOW:                                                    \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_LOW", time);       \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".LOW", time);       \
         break;                                                     \
       case LOWEST:                                                 \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_LOWEST", time);    \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".LOWEST", time);    \
         break;                                                     \
       case IDLE:                                                   \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_IDLE", time);      \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".IDLE", time);      \
         break;                                                     \
       case THROTTLED:                                              \
-        UMA_HISTOGRAM_LONG_TIMES_100(basename "_THROTTLED", time); \
+        UMA_HISTOGRAM_LONG_TIMES_100(basename ".THROTTLED", time); \
         break;                                                     \
     }                                                              \
     UMA_HISTOGRAM_LONG_TIMES_100(basename, time);                  \
   } while (0)
 
 // Record time from Request creation until a valid DNS response.
-void RecordTotalTime(bool had_dns_config,
-                     bool speculative,
+void RecordTotalTime(bool speculative,
+                     bool from_cache,
                      base::TimeDelta duration) {
-  if (had_dns_config) {
-    if (speculative) {
-      UMA_HISTOGRAM_LONG_TIMES_100("AsyncDNS.TotalTime_speculative", duration);
-    } else {
-      UMA_HISTOGRAM_LONG_TIMES_100("AsyncDNS.TotalTime", duration);
-    }
+  if (speculative) {
+    UMA_HISTOGRAM_LONG_TIMES_100("Net.DNS.TotalTime.Speculative", duration);
   } else {
+    UMA_HISTOGRAM_LONG_TIMES_100("Net.DNS.TotalTime", duration);
+  }
+
+  if (!from_cache) {
     if (speculative) {
-      UMA_HISTOGRAM_LONG_TIMES_100("DNS.TotalTime_speculative", duration);
+      UMA_HISTOGRAM_LONG_TIMES_100("Net.DNS.TotalTimeNotCached.Speculative",
+                                   duration);
     } else {
-      UMA_HISTOGRAM_LONG_TIMES_100("DNS.TotalTime", duration);
+      UMA_HISTOGRAM_LONG_TIMES_100("Net.DNS.TotalTimeNotCached", duration);
     }
   }
 }
@@ -1536,16 +1537,9 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
     base::TimeDelta queue_time = now - creation_time_;
     base::TimeDelta queue_time_after_change = now - priority_change_time_;
 
-    if (had_dns_config_) {
-      DNS_HISTOGRAM_BY_PRIORITY("AsyncDNS.JobQueueTime", priority(),
-                                queue_time);
-      DNS_HISTOGRAM_BY_PRIORITY("AsyncDNS.JobQueueTimeAfterChange", priority(),
-                                queue_time_after_change);
-    } else {
-      DNS_HISTOGRAM_BY_PRIORITY("DNS.JobQueueTime", priority(), queue_time);
-      DNS_HISTOGRAM_BY_PRIORITY("DNS.JobQueueTimeAfterChange", priority(),
-                                queue_time_after_change);
-    }
+    DNS_HISTOGRAM_BY_PRIORITY("Net.DNS.JobQueueTime", priority(), queue_time);
+    DNS_HISTOGRAM_BY_PRIORITY("Net.DNS.JobQueueTimeAfterChange", priority(),
+                              queue_time_after_change);
 
     bool system_only =
         (key_.host_resolver_flags & HOST_RESOLVER_SYSTEM_ONLY) != 0;
@@ -1782,7 +1776,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
       LogFinishRequest(req->source_net_log(), req->info(), entry.error());
       if (did_complete) {
         // Record effective total time from creation to completion.
-        RecordTotalTime(had_dns_config_, req->info().is_speculative(),
+        RecordTotalTime(req->info().is_speculative(), false,
                         base::TimeTicks::Now() - req->request_time());
       }
       req->OnJobCompleted(this, entry.error(), entry.addresses());
@@ -1906,7 +1900,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   int rv = ResolveHelper(info, false, nullptr, source_net_log, addresses, &key);
   if (rv != ERR_DNS_CACHE_MISS) {
     LogFinishRequest(source_net_log, info, rv);
-    RecordTotalTime(HaveDnsConfig(), info.is_speculative(), base::TimeDelta());
+    RecordTotalTime(info.is_speculative(), true, base::TimeDelta());
     return rv;
   }