Bug 1228147: part 1: Add telemetry RecordingEnabled support. r=gfritzsche f=froydnj
authorAvi Halachmi <avihpit@yahoo.com>
Wed, 16 Dec 2015 20:06:40 +0200
changeset 276724 d32a676f1c396f37d0db6d3554dbebd8857a0d8e
parent 276723 bada0e17da331f58bf4bc1b1f177dd47e2a41181
child 276725 8f267491ef2a7f1d342a8f6c88420a08fdf44e56
push id29806
push usercbook@mozilla.com
push dateThu, 17 Dec 2015 10:59:52 +0000
treeherdermozilla-central@0711218a018d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1228147
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1228147: part 1: Add telemetry RecordingEnabled support. r=gfritzsche f=froydnj
ipc/chromium/src/base/histogram.cc
ipc/chromium/src/base/histogram.h
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/Telemetry.h
toolkit/components/telemetry/nsITelemetry.idl
toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
--- a/ipc/chromium/src/base/histogram.cc
+++ b/ipc/chromium/src/base/histogram.cc
@@ -431,30 +431,32 @@ Histogram::Histogram(const std::string& 
                      Sample maximum, size_t bucket_count)
   : sample_(),
     histogram_name_(name),
     declared_min_(minimum),
     declared_max_(maximum),
     bucket_count_(bucket_count),
     flags_(kNoFlags),
     ranges_(bucket_count + 1, 0),
-    range_checksum_(0) {
+    range_checksum_(0),
+    recording_enabled_(true) {
   Initialize();
 }
 
 Histogram::Histogram(const std::string& name, TimeDelta minimum,
                      TimeDelta maximum, size_t bucket_count)
   : sample_(),
     histogram_name_(name),
     declared_min_(static_cast<int> (minimum.InMilliseconds())),
     declared_max_(static_cast<int> (maximum.InMilliseconds())),
     bucket_count_(bucket_count),
     flags_(kNoFlags),
     ranges_(bucket_count + 1, 0),
-    range_checksum_(0) {
+    range_checksum_(0),
+    recording_enabled_(true) {
   Initialize();
 }
 
 Histogram::~Histogram() {
   if (StatisticsRecorder::dump_on_exit()) {
     std::string output;
     WriteAscii(true, "\n", &output);
     CHROMIUM_LOG(INFO) << output;
--- a/ipc/chromium/src/base/histogram.h
+++ b/ipc/chromium/src/base/histogram.h
@@ -36,16 +36,17 @@
 // is also completely thread safe, which results in a completely thread safe,
 // and relatively fast, set of counters.  To avoid races at shutdown, the static
 // pointer is NOT deleted, and we leak the histograms at process termination.
 
 #ifndef BASE_METRICS_HISTOGRAM_H_
 #define BASE_METRICS_HISTOGRAM_H_
 #pragma once
 
+#include "mozilla/Atomics.h"
 #include "mozilla/MemoryReporting.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/time.h"
 #include "base/lock.h"
@@ -400,16 +401,22 @@ class Histogram {
                                    base::TimeDelta minimum,
                                    base::TimeDelta maximum,
                                    size_t bucket_count,
                                    Flags flags);
 
   void Add(int value);
   void Subtract(int value);
 
+  // TODO: Currently recording_enabled_ is not used by any Histogram class, but
+  //       rather examined only by the telemetry code (via IsRecordingEnabled).
+  //       Move handling to Histogram's Add() etc after simplifying Histogram.
+  void SetRecordingEnabled(bool aEnabled) { recording_enabled_ = aEnabled; };
+  bool IsRecordingEnabled() const { return recording_enabled_; };
+
   // This method is an interface, used only by BooleanHistogram.
   virtual void AddBoolean(bool value);
 
   // Accept a TimeDelta to increment.
   void AddTime(TimeDelta time) {
     Add(static_cast<int>(time.InMilliseconds()));
   }
 
@@ -584,16 +591,19 @@ class Histogram {
   // The dimension of ranges_ is bucket_count + 1.
   Ranges ranges_;
 
   // For redundancy, we store a checksum of all the sample ranges when ranges
   // are generated.  If ever there is ever a difference, then the histogram must
   // have been corrupted.
   uint32_t range_checksum_;
 
+  // When false, new samples are completely ignored.
+  mozilla::Atomic<bool, mozilla::Relaxed> recording_enabled_;
+
   DISALLOW_COPY_AND_ASSIGN(Histogram);
 };
 
 //------------------------------------------------------------------------------
 
 // LinearHistogram is a more traditional histogram, with evenly spaced
 // buckets.
 class LinearHistogram : public Histogram {
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5053,16 +5053,27 @@
     "kind": "flag",
     "description": "a testing histogram; not meant to be touched"
   },
   "TELEMETRY_TEST_COUNT": {
     "expires_in_version": "never",
     "kind": "count",
     "description": "a testing histogram; not meant to be touched"
   },
+  "TELEMETRY_TEST_COUNT_INIT_NO_RECORD": {
+    "expires_in_version": "never",
+    "kind": "count",
+    "description": "a testing histogram; not meant to be touched - initially not recording"
+  },
+  "TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD": {
+    "expires_in_version": "never",
+    "kind": "count",
+    "keyed": true,
+    "description": "a testing histogram; not meant to be touched - initially not recording"
+  },
   "TELEMETRY_TEST_KEYED_FLAG": {
     "expires_in_version": "never",
     "kind": "flag",
     "keyed": true,
     "description": "a testing histogram; not meant to be touched"
   },
   "TELEMETRY_TEST_KEYED_COUNT": {
     "expires_in_version": "never",
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -6,16 +6,17 @@
 
 #include <algorithm>
 
 #include <fstream>
 
 #include <prio.h>
 
 #include "mozilla/dom/ToJSValue.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "base/histogram.h"
 #include "base/pickle.h"
 #include "nsIComponentManager.h"
@@ -835,16 +836,19 @@ public:
   nsresult GetHistogram(const nsCString& name, Histogram** histogram, bool subsession);
   Histogram* GetHistogram(const nsCString& name, bool subsession);
   uint32_t GetHistogramType() const { return mHistogramType; }
   nsresult GetDataset(uint32_t* dataset) const;
   nsresult GetJSKeys(JSContext* cx, JS::CallArgs& args);
   nsresult GetJSSnapshot(JSContext* cx, JS::Handle<JSObject*> obj,
                          bool subsession, bool clearSubsession);
 
+  void SetRecordingEnabled(bool aEnabled) { mRecordingEnabled = aEnabled; };
+  bool IsRecordingEnabled() const { return mRecordingEnabled; };
+
   nsresult Add(const nsCString& key, uint32_t aSample);
   void Clear(bool subsession);
 
 private:
   typedef nsBaseHashtableET<nsCStringHashKey, Histogram*> KeyedHistogramEntry;
   typedef AutoHashtable<KeyedHistogramEntry> KeyedHistogramMapType;
   KeyedHistogramMapType mHistogramMap;
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
@@ -857,16 +861,17 @@ private:
 
   const nsCString mName;
   const nsCString mExpiration;
   const uint32_t mHistogramType;
   const uint32_t mMin;
   const uint32_t mMax;
   const uint32_t mBucketCount;
   const uint32_t mDataset;
+  mozilla::Atomic<bool, mozilla::Relaxed> mRecordingEnabled;
 };
 
 // Hardcoded probes
 struct TelemetryHistogram {
   uint32_t min;
   uint32_t max;
   uint32_t bucketCount;
   uint32_t histogramType;
@@ -891,16 +896,40 @@ TelemetryHistogram::id() const
 
 const char *
 TelemetryHistogram::expiration() const
 {
   return &gHistogramStringTable[this->expiration_offset];
 }
 
 bool
+IsHistogramEnumId(Telemetry::ID aID)
+{
+  static_assert(((Telemetry::ID)-1 > 0), "ID should be unsigned.");
+  return aID < Telemetry::HistogramCount;
+}
+
+// List of histogram IDs which should have recording disabled initially.
+const Telemetry::ID kRecordingInitiallyDisabledIDs[] = {
+
+  // The array must not be empty. Leave these item here.
+  Telemetry::TELEMETRY_TEST_COUNT_INIT_NO_RECORD,
+  Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD
+};
+
+void
+InitHistogramRecordingEnabled()
+{
+  const size_t length = mozilla::ArrayLength(kRecordingInitiallyDisabledIDs);
+  for (size_t i = 0; i < length; i++) {
+    SetHistogramRecordingEnabled(kRecordingInitiallyDisabledIDs[i], false);
+  }
+}
+
+bool
 IsExpired(const char *expiration){
   static Version current_version = Version(MOZ_APP_VERSION);
   MOZ_ASSERT(expiration);
   return strcmp(expiration, "never") && strcmp(expiration, "default") &&
     (mozilla::Version(expiration) <= current_version);
 }
 
 bool
@@ -1137,17 +1166,17 @@ GetSubsessionHistogram(Histogram& existi
   return subsession[id];
 }
 #endif
 
 nsresult
 HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset)
 {
   // Check if we are allowed to record the data.
-  if (!CanRecordDataset(dataset)) {
+  if (!CanRecordDataset(dataset) || !histogram.IsRecordingEnabled()) {
     return NS_OK;
   }
 
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
   if (Histogram* subsession = GetSubsessionHistogram(histogram)) {
     subsession->Add(value);
   }
 #endif
@@ -1920,16 +1949,21 @@ mFailedLockCount(0)
       continue;
     }
 
     const nsDependentCString id(h.id());
     const nsDependentCString expiration(h.expiration());
     mKeyedHistograms.Put(id, new KeyedHistogram(id, expiration, h.histogramType,
                                                 h.min, h.max, h.bucketCount, h.dataset));
   }
+
+  // Setup of the initial recording-enabled state (for not-recording-by-default
+  // histograms) using InitHistogramRecordingEnabled() will happen after instantiating
+  // sTelemetry since it depends on the static GetKeyedHistogramById(...) - which
+  // uses the singleton instance at sTelemetry.
 }
 
 TelemetryImpl::~TelemetryImpl() {
   UnregisterWeakMemoryReporter(this);
 }
 
 void
 TelemetryImpl::InitMemoryReporter() {
@@ -2261,16 +2295,35 @@ TelemetryImpl::UnregisterAddonHistograms
     // the same names.
     delete addonEntry->mData;
     mAddonMap.RemoveEntry(addonEntry);
   }
 
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TelemetryImpl::SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled)
+{
+  Histogram *h;
+  nsresult rv = GetHistogramByName(id, &h);
+  if (NS_SUCCEEDED(rv)) {
+    h->SetRecordingEnabled(aEnabled);
+    return NS_OK;
+  }
+
+  KeyedHistogram* keyed = GetKeyedHistogramById(id);
+  if (keyed) {
+    keyed->SetRecordingEnabled(aEnabled);
+    return NS_OK;
+  }
+
+  return NS_ERROR_FAILURE;
+}
+
 nsresult
 TelemetryImpl::CreateHistogramSnapshots(JSContext *cx,
                                         JS::MutableHandle<JS::Value> ret,
                                         bool subsession,
                                         bool clearSubsession)
 {
   JS::Rooted<JSObject*> root_obj(cx, JS_NewPlainObject(cx));
   if (!root_obj)
@@ -3301,16 +3354,17 @@ TelemetryImpl::CreateTelemetryInstance()
   MOZ_ASSERT(sTelemetry == nullptr, "CreateTelemetryInstance may only be called once, via GetService()");
   sTelemetry = new TelemetryImpl();
   // AddRef for the local reference
   NS_ADDREF(sTelemetry);
   // AddRef for the caller
   nsCOMPtr<nsITelemetry> ret = sTelemetry;
 
   sTelemetry->InitMemoryReporter();
+  InitHistogramRecordingEnabled(); // requires sTelemetry to exist
 
   return ret.forget();
 }
 
 void
 TelemetryImpl::ShutdownTelemetry()
 {
   // No point in collecting IO beyond this point
@@ -3791,16 +3845,44 @@ RecordShutdownEndTimeStamp() {
     return;
   }
   PR_Delete(name.get());
   PR_Rename(tmpName.get(), name.get());
 }
 
 namespace Telemetry {
 
+// The external API for controlling recording state
+void
+SetHistogramRecordingEnabled(ID aID, bool aEnabled)
+{
+  if (!IsHistogramEnumId(aID)) {
+    MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id");
+    return;
+  }
+
+  if (gHistograms[aID].keyed) {
+    const nsDependentCString id(gHistograms[aID].id());
+    KeyedHistogram* keyed = TelemetryImpl::GetKeyedHistogramById(id);
+    if (keyed) {
+      keyed->SetRecordingEnabled(aEnabled);
+      return;
+    }
+  } else {
+    Histogram *h;
+    nsresult rv = GetHistogramByEnumId(aID, &h);
+    if (NS_SUCCEEDED(rv)) {
+      h->SetRecordingEnabled(aEnabled);
+      return;
+    }
+  }
+
+  MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found");
+}
+
 void
 Accumulate(ID aHistogram, uint32_t aSample)
 {
   if (!TelemetryImpl::CanRecordBase()) {
     return;
   }
   Histogram *h;
   nsresult rv = GetHistogramByEnumId(aHistogram, &h);
@@ -4277,16 +4359,17 @@ KeyedHistogram::KeyedHistogram(const nsA
 #endif
   , mName(name)
   , mExpiration(expiration)
   , mHistogramType(histogramType)
   , mMin(min)
   , mMax(max)
   , mBucketCount(bucketCount)
   , mDataset(dataset)
+  , mRecordingEnabled(true)
 {
 }
 
 nsresult
 KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram,
                              bool subsession)
 {
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
@@ -4364,16 +4447,20 @@ KeyedHistogram::Add(const nsCString& key
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
   Histogram* subsession = GetHistogram(key, true);
   MOZ_ASSERT(subsession);
   if (!subsession) {
     return NS_ERROR_FAILURE;
   }
 #endif
 
+  if (!IsRecordingEnabled()) {
+    return NS_OK;
+  }
+
   histogram->Add(sample);
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
   subsession->Add(sample);
 #endif
   return NS_OK;
 }
 
 void
--- a/toolkit/components/telemetry/Telemetry.h
+++ b/toolkit/components/telemetry/Telemetry.h
@@ -79,16 +79,26 @@ void Accumulate(const char *name, const 
  *
  * @param id - histogram id
  * @param start - start time
  * @param end - end time
  */
 void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
 
 /**
+ * Enable/disable recording for this histogram at runtime.
+ * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[].
+ * id must be a valid telemetry enum, otherwise an assertion is triggered.
+ *
+ * @param id - histogram id
+ * @param enabled - whether or not to enable recording from now on.
+ */
+void SetHistogramRecordingEnabled(ID id, bool enabled);
+
+/**
  * Return a raw Histogram for direct manipulation for users who can not use Accumulate().
  */
 base::Histogram* GetHistogramById(ID id);
 
 const char* GetHistogramName(ID id);
 
 /**
  * Return a raw histogram for keyed histograms.
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -7,17 +7,17 @@
 #include "nsIFile.idl"
 
 [scriptable,function, uuid(3d3b9075-5549-4244-9c08-b64fefa1dd60)]
 interface nsIFetchTelemetryDataCallback : nsISupports
 {
   void complete();
 };
 
-[scriptable, uuid(fabde631-c128-41c3-b7cb-9eb96f1276ff)]
+[scriptable, uuid(273d2dd0-6c63-475a-b864-cb65160a1909)]
 interface nsITelemetry : nsISupports
 {
   /**
    * Histogram types:
    * HISTOGRAM_EXPONENTIAL - buckets increase exponentially
    * HISTOGRAM_LINEAR - buckets increase linearly
    * HISTOGRAM_BOOLEAN - For storing 0/1 values
    * HISTOGRAM_FLAG - For storing a single value; its count is always == 1.
@@ -323,16 +323,26 @@ interface nsITelemetry : nsISupports
   /**
    * Delete all histograms associated with the given addon id.
    *
    * @param addon_id - Unique ID of the addon
    */
   void unregisterAddonHistograms(in ACString addon_id);
 
   /**
+   * Enable/disable recording for this histogram at runtime.
+   * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[].
+   * Name must be a valid Histogram identifier, otherwise an assertion will be triggered.
+   *
+   * @param id - unique identifier from histograms.json
+   * @param enabled - whether or not to enable recording from now on.
+   */
+  void setHistogramRecordingEnabled(in ACString id, in boolean enabled);
+
+  /**
    * An object containing a snapshot from all of the currently
    * registered addon histograms.
    * { addon-id1 : data1, ... }
    *
    * where data is an object whose properties are the names of the
    * addon's histograms and whose corresponding values are as in
    * histogramSnapshots.
    */
--- a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
+++ b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js
@@ -633,16 +633,105 @@ function test_keyed_histogram_recording(
 
   // Check that base histograms are still being recorded.
   h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT");
   h.clear();
   h.add(TEST_KEY, 1);
   Assert.equal(h.snapshot(TEST_KEY).sum, 1);
 }
 
+function test_histogram_recording_enabled() {
+  Telemetry.canRecordBase = true;
+  Telemetry.canRecordExtended = true;
+
+  // Check that a "normal" histogram respects recording-enabled on/off
+  var h = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT");
+  var orig = h.snapshot();
+
+  h.add(1);
+  Assert.equal(orig.sum + 1, h.snapshot().sum,
+              "add should record by default.");
+
+  // Check that when recording is disabled - add is ignored
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT", false);
+  h.add(1);
+  Assert.equal(orig.sum + 1, h.snapshot().sum,
+              "When recording is disabled add should not record.");
+
+  // Check that we're back to normal after recording is enabled
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT", true);
+  h.add(1);
+  Assert.equal(orig.sum + 2, h.snapshot().sum,
+               "When recording is re-enabled add should record.");
+
+  // Check that a histogram with recording disabled by default behaves correctly
+  h = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT_INIT_NO_RECORD");
+  orig = h.snapshot();
+
+  h.add(1);
+  Assert.equal(orig.sum, h.snapshot().sum,
+               "When recording is disabled by default, add should not record by default.");
+
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT_INIT_NO_RECORD", true);
+  h.add(1);
+  Assert.equal(orig.sum + 1, h.snapshot().sum,
+               "When recording is enabled add should record.");
+
+  // Restore to disabled
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_COUNT_INIT_NO_RECORD", false);
+  h.add(1);
+  Assert.equal(orig.sum + 1, h.snapshot().sum,
+               "When recording is disabled add should not record.");
+
+}
+
+function test_keyed_histogram_recording_enabled() {
+  Telemetry.canRecordBase = true;
+  Telemetry.canRecordExtended = true;
+
+  // Check RecordingEnabled for keyed histograms which are recording by default
+  const TEST_KEY = "record_foo";
+  h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT");
+
+  h.clear();
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 1,
+    "Keyed histogram add should record by default");
+
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT", false);
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 1,
+    "Keyed histogram add should not record when recording is disabled");
+
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT", true);
+  h.clear();
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 1,
+    "Keyed histogram add should record when recording is re-enabled");
+
+  // Check that a histogram with recording disabled by default behaves correctly
+  h = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD");
+  h.clear();
+
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 0,
+    "Keyed histogram add should not record by default for histograms which don't record by default");
+
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD", true);
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 1,
+    "Keyed histogram add should record when recording is enabled");
+
+  // Restore to disabled
+  Telemetry.setHistogramRecordingEnabled("TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD", false);
+  h.add(TEST_KEY, 1);
+  Assert.equal(h.snapshot(TEST_KEY).sum, 1,
+    "Keyed histogram add should not record when recording is disabled");
+}
+
 function test_keyed_histogram() {
   // Check that invalid names get rejected.
 
   let threw = false;
   try {
     Telemetry.newKeyedHistogram("test::invalid # histogram", "never", Telemetry.HISTOGRAM_BOOLEAN);
   } catch (e) {
     // This should throw as we reject names with the # separator
@@ -909,9 +998,11 @@ function run_test()
   test_histogramRecording();
   test_addons();
   test_extended_stats();
   test_expired_histogram();
   test_keyed_histogram();
   test_datasets();
   test_subsession();
   test_keyed_subsession();
+  test_histogram_recording_enabled();
+  test_keyed_histogram_recording_enabled();
 }