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..dcd28a52335dfdb0ad4d5bdf0da687a96fd765ec 100644
--- a/components/certificate_transparency/single_tree_tracker.cc
+++ b/components/certificate_transparency/single_tree_tracker.cc
@@ -6,6 +6,7 @@
 
 #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"
@@ -14,6 +15,22 @@ using net::ct::SignedTreeHead;
 
 namespace certificate_transparency {
 
+namespace internal {
+
+const char kCanCheckForInclusionHistogramName[] =
+    "Net.CertificateTransparency.CanInclusionCheckSCT";
+
+}  // namespace internal
+
+namespace {
+
+void LogCanBeCheckedForInclusionToUMA(bool can_be_checked) {

[Eran Messeri, 2016/07/21 17:53:43]

FYI I've documented, in this function, what we're measuring and why.

+  UMA_HISTOGRAM_BOOLEAN(internal::kCanCheckForInclusionHistogramName,
+                        can_be_checked);
+}
+
+}  // namespace

[Ryan Sleevi, 2016/07/20 20:34:00]

Generally we try to have the unnamed namespace at the top. It's unclear if you
put internal first because of kCanCheckForInclusionHistogramName, but if so,
that wouldn't be necessary, because the .h declares that symbol so you'd be fine
referencing it before the definition.

[Eran Messeri, 2016/07/21 17:53:43]

Moved the unnamed namespace to the top, will keep that in mind for future
changes.

+
 SingleTreeTracker::SingleTreeTracker(
     scoped_refptr<const net::CTLogVerifier> ct_log)
     : ct_log_(std::move(ct_log)) {}
@@ -34,15 +51,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())
+      LogCanBeCheckedForInclusionToUMA(false);
     return;
   }
 
+  LogCanBeCheckedForInclusionToUMA(true);
   // 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));
 }