Bug 1228147: part 1: Add telemetry RecordingEnabled support. r=gfritzsche f=froydnj

ipc/chromium/src/base/histogram.cc

@@ -436,7 +436,8 @@ Histogram::Histogram(const std::string& name, Sample minimum,
     bucket_count_(bucket_count),
     flags_(kNoFlags),
     ranges_(bucket_count + 1, 0),
-    range_checksum_(0) {
+    range_checksum_(0),
+    recording_enabled_(true) {
   Initialize();
 }
 
@@ -449,7 +450,8 @@ Histogram::Histogram(const std::string& name, TimeDelta minimum,
     bucket_count_(bucket_count),
     flags_(kNoFlags),
     ranges_(bucket_count + 1, 0),
-    range_checksum_(0) {
+    range_checksum_(0),
+    recording_enabled_(true) {
   Initialize();
 }
 

ipc/chromium/src/base/histogram.h

@@ -41,6 +41,7 @@
 #define BASE_METRICS_HISTOGRAM_H_
 #pragma once
 
+#include "mozilla/Atomics.h"
 #include "mozilla/MemoryReporting.h"
 
 #include <map>
@@ -405,6 +406,12 @@ class Histogram {
   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);
 
@@ -589,6 +596,9 @@ class Histogram {
   // 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);
 };
 

toolkit/components/telemetry/Histograms.json

@@ -5058,6 +5058,17 @@
     "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",

toolkit/components/telemetry/Telemetry.cpp

@@ -11,6 +11,7 @@
 #include <prio.h>
 
 #include "mozilla/dom/ToJSValue.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
@@ -840,6 +841,9 @@ public:
   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);
 
@@ -862,6 +866,7 @@ private:
   const uint32_t mMax;
   const uint32_t mBucketCount;
   const uint32_t mDataset;
+  mozilla::Atomic<bool, mozilla::Relaxed> mRecordingEnabled;
 };
 
 // Hardcoded probes
@@ -895,6 +900,30 @@ 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);
@@ -1142,7 +1171,7 @@ 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;
   }
 
@@ -1925,6 +1954,11 @@ mFailedLockCount(0)
     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() {
@@ -2266,6 +2300,25 @@ TelemetryImpl::UnregisterAddonHistograms(const nsACString &id)
   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,
@@ -3306,6 +3359,7 @@ TelemetryImpl::CreateTelemetryInstance()
   nsCOMPtr<nsITelemetry> ret = sTelemetry;
 
   sTelemetry->InitMemoryReporter();
+  InitHistogramRecordingEnabled(); // requires sTelemetry to exist
 
   return ret.forget();
 }
@@ -3796,6 +3850,34 @@ RecordShutdownEndTimeStamp() {
 
 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)
 {
@@ -4282,6 +4364,7 @@ KeyedHistogram::KeyedHistogram(const nsACString &name, const nsACString &expirat
   , mMax(max)
   , mBucketCount(bucketCount)
   , mDataset(dataset)
+  , mRecordingEnabled(true)
 {
 }
 
@@ -4369,6 +4452,10 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample)
   }
 #endif
 
+  if (!IsRecordingEnabled()) {
+    return NS_OK;
+  }
+
   histogram->Add(sample);
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
   subsession->Add(sample);

toolkit/components/telemetry/Telemetry.h

@@ -83,6 +83,16 @@ void Accumulate(const char *name, const nsCString& key, uint32_t sample = 1);
  */
 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().
  */

toolkit/components/telemetry/nsITelemetry.idl

@@ -12,7 +12,7 @@ interface nsIFetchTelemetryDataCallback : nsISupports
   void complete();
 };
 
-[scriptable, uuid(fabde631-c128-41c3-b7cb-9eb96f1276ff)]
+[scriptable, uuid(273d2dd0-6c63-475a-b864-cb65160a1909)]
 interface nsITelemetry : nsISupports
 {
   /**
@@ -327,6 +327,16 @@ interface nsITelemetry : nsISupports
    */
   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.

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

@@ -638,6 +638,95 @@ function test_keyed_histogram_recording() {
   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.
 
@@ -914,4 +1003,6 @@ function run_test()
   test_datasets();
   test_subsession();
   test_keyed_subsession();
+  test_histogram_recording_enabled();
+  test_keyed_histogram_recording_enabled();
 }