Certificate Transparency: Collect metrics on age of SCT vs STH

components/certificate_transparency/single_tree_tracker.cc

Index: components/certificate_transparency/single_tree_tracker.cc
diff --git a/components/certificate_transparency/single_tree_tracker.cc b/components/certificate_transparency/single_tree_tracker.cc
index 7946208ca753303a7589260e7a60fd34a58bb5e0..2d379fa066d849b2511de4aa5499e24d6c7a7355 100644
--- a/components/certificate_transparency/single_tree_tracker.cc
+++ b/components/certificate_transparency/single_tree_tracker.cc
@@ -6,12 +6,18 @@
 
 #include <utility>
 
+#include "base/metrics/histogram_macros.h"
 #include "net/cert/ct_log_verifier.h"
 #include "net/cert/signed_certificate_timestamp.h"
 #include "net/cert/x509_certificate.h"
 
 using net::ct::SignedTreeHead;
 
+namespace {
+const char kCanCheckForInclusionHistogramName[] =
+    "Net.CertificateTransparency.CanInclusionCheckSCT";

[Alexei Svitkine (slow), 2016/07/18 15:03:57]

If you're only going to use it from the same file once, you might as well inline
it into the macro invocation.

However, in this case it looks like you're actually interested in this name from
the test, so suggest declaring this in the header.

In can be in an "internal" namespace or a static class member.

[Eran Messeri, 2016/07/20 11:02:17]

Done.

+}
+
 namespace certificate_transparency {
 
 SingleTreeTracker::SingleTreeTracker(
@@ -34,15 +40,18 @@ void SingleTreeTracker::OnSCTVerified(
   if (verified_sth_.timestamp.is_null() ||
       (verified_sth_.timestamp <
        (sct->timestamp + base::TimeDelta::FromHours(24)))) {
-    // TODO(eranm): UMA - how often SCTs have to wait for a newer STH for
-    // inclusion check.
     entries_status_.insert(
         std::make_pair(sct->timestamp, SCT_PENDING_NEWER_STH));
+
+    // Do not log histogram if there's no STH for this log yet, as it does
+    // not provide any meaningful data on how fresh SCTs usually are.
+    if (!verified_sth_.timestamp.is_null())
+      UMA_HISTOGRAM_BOOLEAN(kCanCheckForInclusionHistogramName, false);
     return;
   }
 
+  UMA_HISTOGRAM_BOOLEAN(kCanCheckForInclusionHistogramName, true);

[Alexei Svitkine (slow), 2016/07/18 15:03:57]

Each UMA macro expansion results in a bunch of code. So please refactor to only
have this macro once for this histogram. (e.g. either make a helper function
that takes the bool param or change the structure of this function.

[Eran Messeri, 2016/07/20 11:02:17]

Done.

   // TODO(eranm): Check inclusion here.
-  // TODO(eranm): UMA - how often inclusion can be checked immediately.
   entries_status_.insert(
       std::make_pair(sct->timestamp, SCT_PENDING_INCLUSION_CHECK));
 }