Bug 742500 - Disable expired telemetry probes. r=vladan

toolkit/components/telemetry/Makefile.in

@@ -9,6 +9,8 @@ include $(topsrcdir)/config/makefiles/rcs.mk
 LOCAL_INCLUDES += -I$(topsrcdir)/xpcom/build
 LOCAL_INCLUDES += -I$(topsrcdir)/xpcom/threads
 
+DEFINES += -DMOZ_APP_VERSION='"$(MOZ_APP_VERSION)"'
+
 MOZ_HISTOGRAMS_VERSION ?= $(call getSourceRepo)/rev/$(firstword $(shell hg -R $(topsrcdir) parent --template='{node|short}\n' 2>/dev/null))
 ifdef MOZ_HISTOGRAMS_VERSION
 DEFINES += -DHISTOGRAMS_FILE_VERSION='$(MOZ_HISTOGRAMS_VERSION)'

toolkit/components/telemetry/Telemetry.cpp

@@ -23,6 +23,8 @@
 #include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsXPCOMPrivate.h"
+#include "nsIXULAppInfo.h"
+#include "nsVersionComparator.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/ModuleUtils.h"
 #include "nsIXPConnect.h"
@@ -57,6 +59,8 @@
 #include "shared-libraries.h"
 #endif
 
+#define EXPIRED_ID "__expired__"
+
 namespace {
 
 using namespace base;
@@ -388,9 +392,11 @@ struct TelemetryHistogram {
   uint32_t bucketCount;
   uint32_t histogramType;
   uint32_t id_offset;
+  uint32_t expiration_offset;
   bool extendedStatisticsOK;
 
   const char *id() const;
+  const char *expiration() const;
 };
 
 #include "TelemetryHistogramData.inc"
@@ -402,31 +408,27 @@ TelemetryHistogram::id() const
   return &gHistogramStringTable[this->id_offset];
 }
 
-bool
+const char *
-TelemetryHistogramType(Histogram *h, uint32_t *result)
+TelemetryHistogram::expiration() const
 {
-  switch (h->histogram_type()) {
+  return &gHistogramStringTable[this->expiration_offset];
-  case Histogram::HISTOGRAM:
+}
-    *result = nsITelemetry::HISTOGRAM_EXPONENTIAL;
+
-    break;
+bool
-  case Histogram::LINEAR_HISTOGRAM:
+IsExpired(const char *expiration){
-    *result = nsITelemetry::HISTOGRAM_LINEAR;
+  static Version current_version = Version(MOZ_APP_VERSION);
-    break;
+  MOZ_ASSERT(expiration);
-  case Histogram::BOOLEAN_HISTOGRAM:
+  return strcmp(expiration, "never") && (mozilla::Version(expiration) <= current_version);
-    *result = nsITelemetry::HISTOGRAM_BOOLEAN;
+}
-    break;
+
-  case Histogram::FLAG_HISTOGRAM:
+bool
-    *result = nsITelemetry::HISTOGRAM_FLAG;
+IsExpired(const Histogram *histogram){
-    break;
+  return histogram->histogram_name() == EXPIRED_ID;
-  default:
-    return false;
-  }
-  return true;
 }
 
 nsresult
-HistogramGet(const char *name, uint32_t min, uint32_t max, uint32_t bucketCount,
+HistogramGet(const char *name, const char *expiration, uint32_t min, uint32_t max,
-             uint32_t histogramType, Histogram **result)
+             uint32_t bucketCount, uint32_t histogramType, Histogram **result)
 {
   if (histogramType != nsITelemetry::HISTOGRAM_BOOLEAN
       && histogramType != nsITelemetry::HISTOGRAM_FLAG) {
@@ -441,6 +443,14 @@ HistogramGet(const char *name, uint32_t min, uint32_t max, uint32_t bucketCount,
       return NS_ERROR_ILLEGAL_VALUE;
   }
 
+  if (IsExpired(expiration)) {
+    name = EXPIRED_ID;
+    min = 1;
+    max = 2;
+    bucketCount = 3;
+    histogramType = nsITelemetry::HISTOGRAM_LINEAR;
+  }
+
   switch (histogramType) {
   case nsITelemetry::HISTOGRAM_EXPONENTIAL:
     *result = Histogram::FactoryGet(name, min, max, bucketCount, Histogram::kUmaTargetedHistogramFlag);
@@ -472,13 +482,14 @@ GetHistogramByEnumId(Telemetry::ID id, Histogram **ret)
   }
 
   const TelemetryHistogram &p = gHistograms[id];
-  nsresult rv = HistogramGet(p.id(), p.min, p.max, p.bucketCount, p.histogramType, &h);
+  nsresult rv = HistogramGet(p.id(), p.expiration(), p.min, p.max, p.bucketCount, p.histogramType, &h);
   if (NS_FAILED(rv))
     return rv;
 
 #ifdef DEBUG
   // Check that the C++ Histogram code computes the same ranges as the
   // Python histogram code.
+  if (!IsExpired(p.expiration())) {
     const struct bounds &b = gBucketLowerBoundIndex[id];
     if (b.length != 0) {
       MOZ_ASSERT(size_t(b.length) == h->bucket_count(),
@@ -488,6 +499,7 @@ GetHistogramByEnumId(Telemetry::ID id, Histogram **ret)
                    "C++/Python bucket mismatch");
       }
     }
+  }
 #endif
 
   if (p.extendedStatisticsOK) {
@@ -966,12 +978,13 @@ TelemetryImpl::InitMemoryReporter() {
 }
 
 NS_IMETHODIMP
-TelemetryImpl::NewHistogram(const nsACString &name, uint32_t min, uint32_t max,
+TelemetryImpl::NewHistogram(const nsACString &name, const nsACString &expiration, uint32_t min,
-                            uint32_t bucketCount, uint32_t histogramType,
+                            uint32_t max, uint32_t bucketCount, uint32_t histogramType,
                             JSContext *cx, JS::Value *ret)
 {
   Histogram *h;
-  nsresult rv = HistogramGet(PromiseFlatCString(name).get(), min, max, bucketCount, histogramType, &h);
+  nsresult rv = HistogramGet(PromiseFlatCString(name).get(), PromiseFlatCString(expiration).get(),
+                             min, max, bucketCount, histogramType, &h);
   if (NS_FAILED(rv))
     return rv;
   h->ClearFlags(Histogram::kUmaTargetedHistogramFlag);
@@ -1089,20 +1102,23 @@ NS_IMETHODIMP
 TelemetryImpl::HistogramFrom(const nsACString &name, const nsACString &existing_name,
                              JSContext *cx, JS::Value *ret)
 {
-  Histogram *existing;
+  Telemetry::ID id;
-  nsresult rv = GetHistogramByName(existing_name, &existing);
+  nsresult rv = GetHistogramEnumId(PromiseFlatCString(existing_name).get(), &id);
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return rv;
+  }
+  const TelemetryHistogram &p = gHistograms[id];
 
-  uint32_t histogramType;
+  Histogram *existing;
-  bool success = TelemetryHistogramType(existing, &histogramType);
+  rv = GetHistogramByEnumId(id, &existing);
-  if (!success)
+  if (NS_FAILED(rv)) {
-    return NS_ERROR_INVALID_ARG;
+    return rv;
+  }
 
   Histogram *clone;
-  rv = HistogramGet(PromiseFlatCString(name).get(), existing->declared_min(),
+  rv = HistogramGet(PromiseFlatCString(name).get(), p.expiration(),
-                    existing->declared_max(), existing->bucket_count(),
+                    existing->declared_min(), existing->declared_max(),
-                    histogramType, &clone);
+                    existing->bucket_count(), p.histogramType, &clone);
   if (NS_FAILED(rv))
     return rv;
 
@@ -1302,7 +1318,7 @@ TelemetryImpl::GetHistogramSnapshots(JSContext *cx, JS::Value *ret)
   JS::Rooted<JSObject*> hobj(cx);
   for (HistogramIterator it = hs.begin(); it != hs.end(); ++it) {
     Histogram *h = *it;
-    if (!ShouldReflectHistogram(h) || IsEmpty(h)) {
+    if (!ShouldReflectHistogram(h) || IsEmpty(h) || IsExpired(h)) {
       continue;
     }
 
@@ -1333,7 +1349,7 @@ TelemetryImpl::CreateHistogramForAddon(const nsACString &name,
                                        AddonHistogramInfo &info)
 {
   Histogram *h;
-  nsresult rv = HistogramGet(PromiseFlatCString(name).get(),
+  nsresult rv = HistogramGet(PromiseFlatCString(name).get(), "never",
                              info.min, info.max, info.bucketCount,
                              info.histogramType, &h);
   if (NS_FAILED(rv)) {
@@ -1950,15 +1966,21 @@ NS_IMETHODIMP
 TelemetryImpl::RegisteredHistograms(uint32_t *aCount, char*** aHistograms)
 {
   size_t count = ArrayLength(gHistograms);
+  size_t offset = 0;
   char** histograms = static_cast<char**>(nsMemory::Alloc(count * sizeof(char*)));
 
   for (size_t i = 0; i < count; ++i) {
-    const char* h = gHistograms[i].id();
+    if (IsExpired(gHistograms[i].expiration())) {
-    size_t len = strlen(h);
+      offset++;
-    histograms[i] = static_cast<char*>(nsMemory::Clone(h, len+1));
+      continue;
     }
 
-  *aCount = count;
+    const char* h = gHistograms[i].id();
+    size_t len = strlen(h);
+    histograms[i - offset] = static_cast<char*>(nsMemory::Clone(h, len+1));
+  }
+
+  *aCount = count - offset;
   *aHistograms = histograms;
   return NS_OK;
 }

toolkit/components/telemetry/gen-histogram-data.py

@@ -47,19 +47,23 @@ class StringTable:
             return ", ".join(map(toCChar, string))
         f.write("const char %s[] = {\n" % name)
         for (string, offset) in entries[:-1]:
+            e = explodeToCharArray(string)
+            if e:
                 f.write("  /* %5d */ %s, '\\0',\n"
                         % (offset, explodeToCharArray(string)))
+            else:
+                f.write("  /* %5d */ '\\0',\n" % offset)
         f.write("  /* %5d */ %s, '\\0' };\n\n"
                 % (entries[-1][1], explodeToCharArray(entries[-1][0])))
 
-def print_array_entry(histogram, name_index):
+def print_array_entry(histogram, name_index, exp_index):
     cpp_guard = histogram.cpp_guard()
     if cpp_guard:
         print "#if defined(%s)" % cpp_guard
-    print "  { %s, %s, %s, %s, %d, %s }," \
+    print "  { %s, %s, %s, %s, %d, %d, %s }," \
         % (histogram.low(), histogram.high(),
            histogram.n_buckets(), histogram.nsITelemetry_kind(),
-           name_index,
+           name_index, exp_index,
            "true" if histogram.extended_statistics_ok() else "false")
     if cpp_guard:
         print "#endif"
@@ -70,7 +74,8 @@ def write_histogram_table(histograms):
     print "const TelemetryHistogram gHistograms[] = {"
     for histogram in histograms:
         name_index = table.stringIndex(histogram.name())
-        print_array_entry(histogram, name_index)
+        exp_index = table.stringIndex(histogram.expiration())
+        print_array_entry(histogram, name_index, exp_index)
     print "};"
 
     strtab_name = "gHistogramStringTable"

toolkit/components/telemetry/histogram_tools.py

@@ -4,6 +4,7 @@
 
 import json
 import math
+import re
 
 from collections import OrderedDict
 
@@ -54,7 +55,7 @@ def exponential_buckets(dmin, dmax, n_buckets):
         ret_array[bucket_index] = current
     return ret_array
 
-always_allowed_keys = ['kind', 'description', 'cpp_guard']
+always_allowed_keys = ['kind', 'description', 'cpp_guard', 'expires_in_version']
 
 class Histogram:
     """A class for representing a histogram definition."""
@@ -75,6 +76,7 @@ symbol that should guard C/C++ definitions associated with the histogram."""
         self._kind = definition['kind']
         self._cpp_guard = definition.get('cpp_guard')
         self._extended_statistics_ok = definition.get('extended_statistics_ok', False)
+        self._expiration = definition.get('expires_in_version')
         self.compute_bucket_parameters(definition)
         table = { 'boolean': 'BOOLEAN',
                   'flag': 'FLAG',
@@ -97,6 +99,10 @@ symbol that should guard C/C++ definitions associated with the histogram."""
 Will be one of 'boolean', 'flag', 'enumerated', 'linear', or 'exponential'."""
         return self._kind
 
+    def expiration(self):
+        """Return the expiration version of the histogram."""
+        return self._expiration
+
     def nsITelemetry_kind(self):
         """Return the nsITelemetry constant corresponding to the kind of
 the histogram."""
@@ -162,6 +168,19 @@ is enabled."""
         table_dispatch(definition['kind'], table,
                        lambda allowed_keys: Histogram.check_keys(name, definition, allowed_keys))
 
+        Histogram.check_expiration(name, definition)
+
+    @staticmethod
+    def check_expiration(name, definition):
+        expiration = definition['expires_in_version']
+
+        if not expiration:
+            return
+
+        if not re.match(r'[1-9][0-9]*\..*|never', expiration):
+            raise BaseException, '%s not permitted as an expiration version for %s; the complete version name is required ' \
+                                 '(see https://developer.mozilla.org/en-US/docs/Performance/Adding_a_new_Telemetry_probe)' % (expiration, name)
+
     @staticmethod
     def check_keys(name, definition, allowed_keys):
         for key in definition.iterkeys():

toolkit/components/telemetry/nsITelemetry.idl

@@ -139,6 +139,7 @@ interface nsITelemetry : nsISupports
    * Create and return a histogram.  Parameters:
    *
    * @param name Unique histogram name
+   * @param expiration Expiration version
    * @param min - Minimal bucket size
    * @param max - Maximum bucket size
    * @param bucket_count - number of buckets in the histogram.
@@ -149,7 +150,7 @@ interface nsITelemetry : nsISupports
    *   clear() - Zeros out the histogram's buckets and sum
    */
   [implicit_jscontext]
-  jsval newHistogram(in ACString name, in uint32_t min, in uint32_t max, in uint32_t bucket_count, in unsigned long histogram_type);
+  jsval newHistogram(in ACString name, in ACString expiration, in uint32_t min, in uint32_t max, in uint32_t bucket_count, in unsigned long histogram_type);
 
   /**
    * Create a histogram using the current state of an existing histogram.  The

toolkit/components/telemetry/tests/unit/test_TelemetryPing.js

@@ -44,6 +44,16 @@ var httpserver = new HttpServer();
 var serverStarted = false;
 var gFinished = false;
 
+function test_expired_histogram() {
+  var histogram_id = "FOOBAR";
+  var dummy = Telemetry.newHistogram(histogram_id, "30", 1, 2, 3, Telemetry.HISTOGRAM_EXPONENTIAL);
+
+  dummy.add(1);
+
+  do_check_eq(TelemetryPing.getPayload()["histograms"][histogram_id], undefined);
+  do_check_eq(TelemetryPing.getPayload()["histograms"]["TELEMETRY_TEST_EXPIRED"], undefined);
+}
+
 function telemetry_ping () {
   TelemetryPing.gatherStartup();
   TelemetryPing.enableLoadSaveNotifications();
@@ -98,7 +108,7 @@ function nonexistentServerObserver(aSubject, aTopic, aData) {
 }
 
 function setupTestData() {
-  Telemetry.newHistogram(IGNORE_HISTOGRAM, 1, 2, 3, Telemetry.HISTOGRAM_BOOLEAN);
+  Telemetry.newHistogram(IGNORE_HISTOGRAM, "never", 1, 2, 3, Telemetry.HISTOGRAM_BOOLEAN);
   Telemetry.histogramFrom(IGNORE_CLONED_HISTOGRAM, IGNORE_HISTOGRAM_TO_CLONE);
   Services.startup.interrupted = true;
   Telemetry.registerAddonHistogram(ADDON_NAME, ADDON_HISTOGRAM, 1, 5, 6,
@@ -499,6 +509,7 @@ function actualTest() {
   registerFakePluginHost();
 
   runInvalidJSONTest();
+  test_expired_histogram();
 
   addWrappedObserver(nonexistentServerObserver, "telemetry-test-xhr-complete");
   telemetry_ping();

toolkit/components/telemetry/tests/unit/test_nsITelemetry.js

@@ -9,9 +9,26 @@ const INT_MAX = 0x7FFFFFFF;
 const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry);
 Cu.import("resource://gre/modules/Services.jsm");
 
-function test_histogram(histogram_type, name, min, max, bucket_count) {
+function test_expired_histogram() {
-  var h = Telemetry.newHistogram(name, min, max, bucket_count, histogram_type);
+  var histogram_id = "FOOBAR";
+  var test_expired_id = "TELEMETRY_TEST_EXPIRED";
+  var clone_id = "ExpiredClone";
+  var dummy = Telemetry.newHistogram(histogram_id, "28.0a1", 1, 2, 3, Telemetry.HISTOGRAM_EXPONENTIAL);
+  var dummy_clone = Telemetry.histogramFrom(clone_id, test_expired_id);
+  var rh = Telemetry.registeredHistograms([]);
 
+  dummy.add(1);
+  dummy_clone.add(1);
+
+  do_check_eq(Telemetry.histogramSnapshots["__expired__"], undefined);
+  do_check_eq(Telemetry.histogramSnapshots[histogram_id], undefined);
+  do_check_eq(Telemetry.histogramSnapshots[test_expired_id], undefined);
+  do_check_eq(Telemetry.histogramSnapshots[clone_id], undefined);
+  do_check_eq(rh[test_expired_id], undefined);
+}
+
+function test_histogram(histogram_type, name, min, max, bucket_count) {
+  var h = Telemetry.newHistogram(name, "never", min, max, bucket_count, histogram_type);
   var r = h.snapshot().ranges;
   var sum = 0;
   var log_sum = 0;
@@ -109,7 +126,7 @@ function expect_success(f) {
 
 function test_boolean_histogram()
 {
-  var h = Telemetry.newHistogram("test::boolean histogram", 99,1,4, Telemetry.HISTOGRAM_BOOLEAN);
+  var h = Telemetry.newHistogram("test::boolean histogram", "never", 99,1,4, Telemetry.HISTOGRAM_BOOLEAN);
   var r = h.snapshot().ranges;
   // boolean histograms ignore numeric parameters
   do_check_eq(uneval(r), uneval([0, 1, 2]))
@@ -131,7 +148,7 @@ function test_boolean_histogram()
 
 function test_flag_histogram()
 {
-  var h = Telemetry.newHistogram("test::flag histogram", 130, 4, 5, Telemetry.HISTOGRAM_FLAG);
+  var h = Telemetry.newHistogram("test::flag histogram", "never", 130, 4, 5, Telemetry.HISTOGRAM_FLAG);
   var r = h.snapshot().ranges;
   // Flag histograms ignore numeric parameters.
   do_check_eq(uneval(r), uneval([0, 1, 2]))
@@ -312,7 +329,7 @@ function test_addons() {
 
 // Check that telemetry doesn't record in private mode
 function test_privateMode() {
-  var h = Telemetry.newHistogram("test::private_mode_boolean", 1,2,3, Telemetry.HISTOGRAM_BOOLEAN);
+  var h = Telemetry.newHistogram("test::private_mode_boolean", "never", 1,2,3, Telemetry.HISTOGRAM_BOOLEAN);
   var orig = h.snapshot();
   Telemetry.canRecord = false;
   h.add(1);
@@ -351,8 +368,8 @@ function run_test()
     test_histogram(histogram_type, "test::"+histogram_type, min, max, bucket_count);
 
     const nh = Telemetry.newHistogram;
-    expect_fail(function () nh("test::min", 0, max, bucket_count, histogram_type));
+    expect_fail(function () nh("test::min", "never", 0, max, bucket_count, histogram_type));
-    expect_fail(function () nh("test::bucket_count", min, max, 1, histogram_type));
+    expect_fail(function () nh("test::bucket_count", "never", min, max, 1, histogram_type));
   }
 
   // Instantiate the storage for this histogram and make sure it doesn't
@@ -367,4 +384,5 @@ function run_test()
   test_privateMode();
   test_addons();
   test_extended_stats();
+  test_expired_histogram();
 }