Bug 1500078 - Filter test probes by default, disable filtering in test mode r=chutten

Differential Revision: https://phabricator.services.mozilla.com/D9245

--HG--
extra : moz-landing-system : lando

toolkit/components/telemetry/app/TelemetryUtils.jsm

@@ -245,94 +245,6 @@ var TelemetryUtils = {
       Services.prefs.getBoolPref(this.Preferences.OverridePreRelease, false);
   },
 
-  /**
-   * Converts histograms from the raw to the packed format.
-   * This additionally filters TELEMETRY_TEST_ histograms.
-   *
-   * @param {Object} snapshot - The histogram snapshot.
-   * @param {Boolean} [testingMode=false] - Whether or not testing histograms
-   *        should be filtered.
-   * @returns {Object}
-   *
-   * {
-   *  "<process>": {
-   *    "<histogram>": {
-   *      range: [min, max],
-   *      bucket_count: <number of buckets>,
-   *      histogram_type: <histogram_type>,
-   *      sum: <sum>,
-   *      values: { bucket1: count1, bucket2: count2, ... }
-   *    },
-   *   ..
-   *   },
-   *  ..
-   * }
-   */
-  packHistograms(snapshot, testingMode = false) {
-    let ret = {};
-
-    for (let [process, histograms] of Object.entries(snapshot)) {
-      ret[process] = {};
-      for (let [name, value] of Object.entries(histograms)) {
-        if (testingMode || !name.startsWith("TELEMETRY_TEST_")) {
-          ret[process][name] = value;
-        }
-      }
-    }
-
-    return ret;
-  },
-
-  /**
-   * Converts keyed histograms from the raw to the packed format.
-   * This additionally filters TELEMETRY_TEST_ histograms and skips
-   * empty keyed histograms.
-   *
-   * @param {Object} snapshot - The keyed histogram snapshot.
-   * @param {Boolean} [testingMode=false] - Whether or not testing histograms should
-   *        be filtered.
-   * @returns {Object}
-   *
-   * {
-   *  "<process>": {
-   *    "<histogram>": {
-   *      "<key>": {
-   *        range: [min, max],
-   *        bucket_count: <number of buckets>,
-   *        histogram_type: <histogram_type>,
-   *        sum: <sum>,
-   *        values: { bucket1: count1, bucket2: count2, ... }
-   *      },
-   *      ..
-   *    },
-   *   ..
-   *   },
-   *  ..
-   * }
-   */
-  packKeyedHistograms(snapshot, testingMode = false) {
-    let ret = {};
-
-    for (let [process, histograms] of Object.entries(snapshot)) {
-      ret[process] = {};
-      for (let [name, value] of Object.entries(histograms)) {
-        if (testingMode || !name.startsWith("TELEMETRY_TEST_")) {
-          let keys = Object.keys(value);
-          if (keys.length == 0) {
-            // Skip empty keyed histogram
-            continue;
-          }
-          ret[process][name] = {};
-          for (let [key, hgram] of Object.entries(value)) {
-            ret[process][name][key] = hgram;
-          }
-        }
-      }
-    }
-
-    return ret;
-  },
-
   /**
    * @returns {string} The name of the update channel to report
    * in telemetry.

toolkit/components/telemetry/core/Telemetry.cpp

@@ -622,46 +622,54 @@ TelemetryImpl::SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled)
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForHistograms(const nsACString& aStoreName,
-                                        bool aClearStore, JSContext* aCx,
+                                        bool aClearStore,
+                                        bool aFilterTest,
+                                        JSContext* aCx,
                                         JS::MutableHandleValue aResult)
 {
   unsigned int dataset = mCanRecordExtended ?
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN :
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
-  return TelemetryHistogram::CreateHistogramSnapshots(aCx, aResult, dataset, aClearStore);
+  return TelemetryHistogram::CreateHistogramSnapshots(aCx, aResult, dataset, aClearStore, aFilterTest);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForKeyedHistograms(const nsACString& aStoreName,
-                                             bool aClearStore, JSContext* aCx,
+                                             bool aClearStore,
+                                             bool aFilterTest,
+                                             JSContext* aCx,
                                              JS::MutableHandleValue aResult)
 {
   unsigned int dataset = mCanRecordExtended ?
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN :
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
-  return TelemetryHistogram::GetKeyedHistogramSnapshots(aCx, aResult, dataset, aClearStore);
+  return TelemetryHistogram::GetKeyedHistogramSnapshots(aCx, aResult, dataset, aClearStore, aFilterTest);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForScalars(const nsACString& aStoreName,
-                                     bool aClearStore, JSContext* aCx,
+                                     bool aClearStore,
+                                     bool aFilterTest,
+                                     JSContext* aCx,
                                      JS::MutableHandleValue aResult)
 {
   unsigned int dataset = mCanRecordExtended ?
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN :
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
-  return TelemetryScalar::CreateSnapshots(dataset, aClearStore, aCx, 1, aResult);
+  return TelemetryScalar::CreateSnapshots(dataset, aClearStore, aCx, 1, aResult, aFilterTest);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForKeyedScalars(const nsACString& aStoreName,
-                                          bool aClearStore, JSContext* aCx,
+                                          bool aClearStore,
+                                          bool aFilterTest,
+                                          JSContext* aCx,
                                           JS::MutableHandleValue aResult)
 {
   unsigned int dataset = mCanRecordExtended ?
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN :
     nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
-  return TelemetryScalar::CreateKeyedSnapshots(dataset, aClearStore, aCx, 1, aResult);
+  return TelemetryScalar::CreateKeyedSnapshots(dataset, aClearStore, aCx, 1, aResult, aFilterTest);
 }
 
 NS_IMETHODIMP

toolkit/components/telemetry/core/TelemetryHistogram.cpp

@@ -271,6 +271,8 @@ const HistogramID kRecordingInitiallyDisabledIDs[] = {
   mozilla::Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD
 };
 
+const char* TEST_HISTOGRAM_PREFIX = "TELEMETRY_TEST_";
+
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
@@ -872,6 +874,7 @@ internal_GetHistogramsSnapshot(const StaticMutexAutoLock& aLock,
                                unsigned int aDataset,
                                bool aClearSubsession,
                                bool aIncludeGPU,
+                               bool aFilterTest,
                                HistogramProcessSnapshotsArray& aOutSnapshot)
 {
   if (!aOutSnapshot.resize(static_cast<uint32_t>(ProcessID::Count))) {
@@ -906,6 +909,15 @@ internal_GetHistogramsSnapshot(const StaticMutexAutoLock& aLock,
         continue;
       }
 
+      const char* name = info.name();
+      if (aFilterTest && strncmp(TEST_HISTOGRAM_PREFIX, name, strlen(TEST_HISTOGRAM_PREFIX)) == 0) {
+        if (aClearSubsession) {
+          h->Clear();
+        }
+        continue;
+      }
+
+
       HistogramSnapshotData snapshotData;
       if (NS_FAILED(internal_GetHistogramAndSamples(aLock, h, snapshotData))) {
         continue;
@@ -1173,6 +1185,7 @@ internal_GetKeyedHistogramsSnapshot(const StaticMutexAutoLock& aLock,
                                     unsigned int aDataset,
                                     bool aClearSubsession,
                                     bool aIncludeGPU,
+                                    bool aFilterTest,
                                     KeyedHistogramProcessSnapshotsArray& aOutSnapshot,
                                     bool aSkipEmpty = false)
 {
@@ -1206,6 +1219,14 @@ internal_GetKeyedHistogramsSnapshot(const StaticMutexAutoLock& aLock,
         continue;
       }
 
+      const char* name = info.name();
+      if (aFilterTest && strncmp(TEST_HISTOGRAM_PREFIX, name, strlen(TEST_HISTOGRAM_PREFIX)) == 0) {
+        if (aClearSubsession) {
+          keyed->Clear();
+        }
+        continue;
+      }
+
       // Take a snapshot of the keyed histogram data!
       KeyedHistogramSnapshotData snapshot;
       if (!NS_SUCCEEDED(keyed->GetSnapshot(aLock, snapshot, aClearSubsession))) {
@@ -2470,7 +2491,8 @@ nsresult
 TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx,
                                              JS::MutableHandleValue aResult,
                                              unsigned int aDataset,
-                                             bool aClearSubsession)
+                                             bool aClearSubsession,
+                                             bool aFilterTest)
 {
   // Runs without protection from |gTelemetryHistogramMutex|
   JS::Rooted<JSObject*> root_obj(aCx, JS_NewPlainObject(aCx));
@@ -2490,6 +2512,7 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext* aCx,
                                                  aDataset,
                                                  aClearSubsession,
                                                  includeGPUProcess,
+                                                 aFilterTest,
                                                  processHistArray);
     if (NS_FAILED(rv)) {
       return rv;
@@ -2536,7 +2559,8 @@ nsresult
 TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext* aCx,
                                                JS::MutableHandleValue aResult,
                                                unsigned int aDataset,
-                                               bool aClearSubsession)
+                                               bool aClearSubsession,
+                                               bool aFilterTest)
 {
   // Runs without protection from |gTelemetryHistogramMutex|
   JS::Rooted<JSObject*> obj(aCx, JS_NewPlainObject(aCx));
@@ -2557,7 +2581,9 @@ TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext* aCx,
                                                       aDataset,
                                                       aClearSubsession,
                                                       includeGPUProcess,
-                                                      processHistArray);
+                                                      aFilterTest,
+                                                      processHistArray,
+                                                      true /* skipEmpty */);
     if (NS_FAILED(rv)) {
       return rv;
     }
@@ -2823,6 +2849,7 @@ TelemetryHistogram::SerializeHistograms(mozilla::JSONWriter& aWriter)
                                                  nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN,
                                                  false /* aClearSubsession */,
                                                  includeGPUProcess,
+                                                 false /* aFilterTest */,
                                                  processHistArray))) {
       return NS_ERROR_FAILURE;
     }
@@ -2870,6 +2897,7 @@ TelemetryHistogram::SerializeKeyedHistograms(mozilla::JSONWriter& aWriter)
                                                       nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN,
                                                       false /* aClearSubsession */,
                                                       includeGPUProcess,
+                                                      false /* aFilterTest */,
                                                       processHistArray,
                                                       true /* aSkipEmpty */))) {
       return NS_ERROR_FAILURE;

toolkit/components/telemetry/core/TelemetryHistogram.h

@@ -69,11 +69,11 @@ GetHistogramName(mozilla::Telemetry::HistogramID id);
 
 nsresult
 CreateHistogramSnapshots(JSContext* aCx, JS::MutableHandleValue aResult, unsigned int aDataset,
-                         bool aClearSubsession);
+                         bool aClearSubsession, bool aFilterTest=false);
 
 nsresult
 GetKeyedHistogramSnapshots(JSContext *aCx, JS::MutableHandleValue aResult, unsigned int aDataset,
-                           bool aClearSubsession);
+                           bool aClearSubsession, bool aFilterTest=false);
 
 size_t
 GetHistogramSizesOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf);

toolkit/components/telemetry/core/TelemetryScalar.cpp

@@ -115,6 +115,8 @@ const uint32_t kScalarCount =
 // a certain high water mark of elements.
 const size_t kScalarActionsArrayHighWaterMark = 10000;
 
+const char* TEST_SCALAR_PREFIX = "telemetry.test.";
+
 enum class ScalarResult : uint8_t {
   // Nothing went wrong.
   Ok,
@@ -2978,7 +2980,7 @@ TelemetryScalar::SetMaximum(mozilla::Telemetry::ScalarID aId, const nsAString& a
  */
 nsresult
 TelemetryScalar::CreateSnapshots(unsigned int aDataset, bool aClearScalars, JSContext* aCx,
-                                 uint8_t optional_argc, JS::MutableHandle<JS::Value> aResult)
+                                 uint8_t optional_argc, JS::MutableHandle<JS::Value> aResult, bool aFilterTest)
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Snapshotting scalars should only happen in the parent processes.");
@@ -3026,6 +3028,11 @@ TelemetryScalar::CreateSnapshots(unsigned int aDataset, bool aClearScalars, JSCo
     for (ScalarTupleArray::size_type i = 0; i < processScalars.Length(); i++) {
       const ScalarDataTuple& scalar = processScalars[i];
 
+      const char* scalarName = mozilla::Get<0>(scalar);
+      if (aFilterTest && strncmp(TEST_SCALAR_PREFIX, scalarName, strlen(TEST_SCALAR_PREFIX)) == 0) {
+          continue;
+      }
+
       // Convert it to a JS Val.
       JS::Rooted<JS::Value> scalarJsValue(aCx);
       nsresult rv =
@@ -3035,7 +3042,7 @@ TelemetryScalar::CreateSnapshots(unsigned int aDataset, bool aClearScalars, JSCo
       }
 
       // Add it to the scalar object.
-      if (!JS_DefineProperty(aCx, processObj, mozilla::Get<0>(scalar), scalarJsValue, JSPROP_ENUMERATE)) {
+      if (!JS_DefineProperty(aCx, processObj, scalarName, scalarJsValue, JSPROP_ENUMERATE)) {
         return NS_ERROR_FAILURE;
       }
     }
@@ -3054,7 +3061,8 @@ TelemetryScalar::CreateSnapshots(unsigned int aDataset, bool aClearScalars, JSCo
  */
 nsresult
 TelemetryScalar::CreateKeyedSnapshots(unsigned int aDataset, bool aClearScalars, JSContext* aCx,
-                                      uint8_t optional_argc, JS::MutableHandle<JS::Value> aResult)
+                                      uint8_t optional_argc, JS::MutableHandle<JS::Value> aResult,
+                                      bool aFilterTest)
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Snapshotting scalars should only happen in the parent processes.");
@@ -3102,6 +3110,11 @@ TelemetryScalar::CreateKeyedSnapshots(unsigned int aDataset, bool aClearScalars,
     for (KeyedScalarTupleArray::size_type i = 0; i < processScalars.Length(); i++) {
       const KeyedScalarDataTuple& keyedScalarData = processScalars[i];
 
+      const char* scalarName = mozilla::Get<0>(keyedScalarData);
+      if (aFilterTest && strncmp(TEST_SCALAR_PREFIX, scalarName, strlen(TEST_SCALAR_PREFIX)) == 0) {
+          continue;
+      }
+
       // Go through each keyed scalar and create a keyed scalar object.
       // This object will hold the values for all the keyed scalar keys.
       JS::RootedObject keyedScalarObj(aCx, JS_NewPlainObject(aCx));
@@ -3128,7 +3141,7 @@ TelemetryScalar::CreateKeyedSnapshots(unsigned int aDataset, bool aClearScalars,
       }
 
       // Add the scalar to the root object.
-      if (!JS_DefineProperty(aCx, processObj, mozilla::Get<0>(keyedScalarData), keyedScalarObj, JSPROP_ENUMERATE)) {
+      if (!JS_DefineProperty(aCx, processObj, scalarName, keyedScalarObj, JSPROP_ENUMERATE)) {
         return NS_ERROR_FAILURE;
       }
     }

toolkit/components/telemetry/core/TelemetryScalar.h

@@ -41,7 +41,7 @@ nsresult Set(const nsACString& aName, JS::HandleValue aVal, JSContext* aCx);
 nsresult SetMaximum(const nsACString& aName, JS::HandleValue aVal, JSContext* aCx);
 nsresult CreateSnapshots(unsigned int aDataset, bool aClearScalars,
                          JSContext* aCx, uint8_t optional_argc,
-                         JS::MutableHandle<JS::Value> aResult);
+                         JS::MutableHandle<JS::Value> aResult, bool aFilterTest=false);
 
 // Keyed JS API Endpoints.
 nsresult Add(const nsACString& aName, const nsAString& aKey, JS::HandleValue aVal,
@@ -52,7 +52,7 @@ nsresult SetMaximum(const nsACString& aName, const nsAString& aKey, JS::HandleVa
                     JSContext* aCx);
 nsresult CreateKeyedSnapshots(unsigned int aDataset, bool aClearScalars,
                               JSContext* aCx, uint8_t optional_argc,
-                              JS::MutableHandle<JS::Value> aResult);
+                              JS::MutableHandle<JS::Value> aResult, bool aFilterTest=false);
 
 // C++ API Endpoints.
 void Add(mozilla::Telemetry::ScalarID aId, uint32_t aValue);

toolkit/components/telemetry/core/nsITelemetry.idl

@@ -66,9 +66,12 @@ interface nsITelemetry : nsISupports
    *
    * @param aStoreName The name of the store to snapshot. Ignored at the moment.
    * @param aClearStore Whether to clear out the histograms in the named store after snapshotting.
+   * @param aFilterTest If true, `TELEMETRY_TEST_` histograms will be filtered out.
+                        Filtered histograms are still cleared if `aClearStore` is true.
+   *                    Defaults to false.
    */
   [implicit_jscontext]
-  jsval getSnapshotForHistograms(in ACString aStoreName, in boolean aClearStore);
+  jsval getSnapshotForHistograms(in ACString aStoreName, in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the keyed histograms from the given store to a JSON-style object.
@@ -77,9 +80,12 @@ interface nsITelemetry : nsISupports
    *
    * @param aStoreName The name of the store to snapshot. Ignored at the moment.
    * @param aClearStore Whether to clear out the keyed histograms in the named store after snapshotting.
+   * @param aFilterTest If true, `TELEMETRY_TEST_` histograms will be filtered out.
+                        Filtered histograms are still cleared if `aClearStore` is true.
+   *                    Defaults to false.
    */
   [implicit_jscontext]
-  jsval getSnapshotForKeyedHistograms(in ACString aStoreName, in boolean aClearStore);
+  jsval getSnapshotForKeyedHistograms(in ACString aStoreName, in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the scalars from the given store to a JSON-style object.
@@ -88,9 +94,12 @@ interface nsITelemetry : nsISupports
    *
    * @param aStoreName The name of the store to snapshot. Ignored at the moment.
    * @param aClearStore Whether to clear out the scalars in the named store after snapshotting.
+   * @param aFilterTest If true, `telemetry.test` scalars will be filtered out.
+                        Filtered scalars are still cleared if `aClearStore` is true.
+   *                    Defaults to false.
    */
   [implicit_jscontext]
-  jsval getSnapshotForScalars(in ACString aStoreName, in boolean aClearStore);
+  jsval getSnapshotForScalars(in ACString aStoreName, in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the keyed scalars from the given store to a JSON-style object.
@@ -99,9 +108,12 @@ interface nsITelemetry : nsISupports
    *
    * @param aStoreName The name of the store to snapshot. Ignored at the moment.
    * @param aClearStore Whether to clear out the keyed scalars in the named store after snapshotting.
+   * @param aFilterTest If true, `telemetry.test` scalars will be filtered out.
+                        Filtered scalars are still cleared if `aClearStore` is true.
+   *                    Defaults to false.
    */
   [implicit_jscontext]
-  jsval getSnapshotForKeyedScalars(in ACString aStoreName, in boolean aClearStore);
+  jsval getSnapshotForKeyedScalars(in ACString aStoreName, in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the histograms from the given dataset to a JSON-style object.

toolkit/components/telemetry/geckoview/GeckoViewTelemetryController.jsm

@@ -88,14 +88,14 @@ const GeckoViewTelemetryController = {
   retrieveSnapshots(aClear, aCallback) {
     debug `retrieveSnapshots`;
 
-    const rawHistograms = Services.telemetry.getSnapshotForHistograms("main", /* clear */ false);
-    const rawKeyedHistograms = Services.telemetry.getSnapshotForKeyedHistograms("main", /* clear */ false);
+    const histograms = Services.telemetry.getSnapshotForHistograms("main", /* clear */ false);
+    const keyedHistograms = Services.telemetry.getSnapshotForKeyedHistograms("main", /* clear */ false);
     const scalars = Services.telemetry.getSnapshotForScalars("main", /* clear */ false);
     const keyedScalars = Services.telemetry.getSnapshotForKeyedScalars("main", /* clear */ false);
 
     const snapshots = {
-      histograms: TelemetryUtils.packHistograms(rawHistograms),
-      keyedHistograms: TelemetryUtils.packKeyedHistograms(rawKeyedHistograms),
+      histograms,
+      keyedHistograms,
       scalars,
       keyedScalars,
     };

toolkit/components/telemetry/pings/TelemetrySession.jsm

@@ -790,13 +790,11 @@ var Impl = {
   },
 
   getHistograms: function getHistograms(clearSubsession) {
-    let hls = Telemetry.getSnapshotForHistograms("main", clearSubsession);
-    return TelemetryUtils.packHistograms(hls, this._testing);
+    return Telemetry.getSnapshotForHistograms("main", clearSubsession, !this._testing);
   },
 
   getKeyedHistograms(clearSubsession) {
-    let khs = Telemetry.getSnapshotForKeyedHistograms("main", clearSubsession);
-    return TelemetryUtils.packKeyedHistograms(khs, this._testing);
+    return Telemetry.getSnapshotForKeyedHistograms("main", clearSubsession, !this._testing);
   },
 
   /**
@@ -816,25 +814,10 @@ var Impl = {
     }
 
     let scalarsSnapshot = keyed ?
-      Telemetry.getSnapshotForKeyedScalars("main", clearSubsession) :
-      Telemetry.getSnapshotForScalars("main", clearSubsession);
+      Telemetry.getSnapshotForKeyedScalars("main", clearSubsession, !this._testing) :
+      Telemetry.getSnapshotForScalars("main", clearSubsession, !this._testing);
 
-    // Don't return the test scalars.
-    let ret = {};
-    for (let processName in scalarsSnapshot) {
-      for (let name in scalarsSnapshot[processName]) {
-        if (name.startsWith("telemetry.test") && !this._testing) {
-          continue;
-        }
-        // Finally arrange the data in the returned object.
-        if (!(processName in ret)) {
-          ret[processName] = {};
-        }
-        ret[processName][name] = scalarsSnapshot[processName][name];
-      }
-    }
-
-    return ret;
+    return scalarsSnapshot;
   },
 
   /**

toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp

@@ -109,9 +109,9 @@ GetScalarsSnapshot(bool aKeyed, JSContext* aCx, JS::MutableHandle<JS::Value> aRe
   nsresult rv;
 
   if (aKeyed) {
-    rv = telemetry->GetSnapshotForKeyedScalars(NS_LITERAL_CSTRING("main"), false, aCx, &scalarsSnapshot);
+    rv = telemetry->GetSnapshotForKeyedScalars(NS_LITERAL_CSTRING("main"), false, false /* filter */, aCx, &scalarsSnapshot);
   } else {
-    rv = telemetry->GetSnapshotForScalars(NS_LITERAL_CSTRING("main"), false, aCx, &scalarsSnapshot);
+    rv = telemetry->GetSnapshotForScalars(NS_LITERAL_CSTRING("main"), false, false /* filter */, aCx, &scalarsSnapshot);
   }
 
   // Validate the snapshot.
@@ -174,8 +174,8 @@ GetSnapshots(JSContext* cx, nsCOMPtr<nsITelemetry> mTelemetry,
              const char* name, JS::MutableHandleValue valueOut, bool is_keyed)
 {
   JS::RootedValue snapshots(cx);
-  nsresult rv = is_keyed ? mTelemetry->GetSnapshotForKeyedHistograms(NS_LITERAL_CSTRING("main"), false, cx, &snapshots)
-                         : mTelemetry->GetSnapshotForHistograms(NS_LITERAL_CSTRING("main"), false, cx, &snapshots);
+  nsresult rv = is_keyed ? mTelemetry->GetSnapshotForKeyedHistograms(NS_LITERAL_CSTRING("main"), false, false /* filter */, cx, &snapshots)
+                         : mTelemetry->GetSnapshotForHistograms(NS_LITERAL_CSTRING("main"), false, false /* filter */, cx, &snapshots);
 
   JS::RootedValue snapshot(cx);
   GetProperty(cx, "parent", snapshots, &snapshot);

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

@@ -644,6 +644,44 @@ add_task(async function test_newCanRecordsMatchTheOld() {
                "Prerelease Data is the new way to say Extended Collection");
 });
 
+add_task(function test_histogram_filtering() {
+  const COUNT_ID = "TELEMETRY_TEST_COUNT";
+  const KEYED_ID = "TELEMETRY_TEST_KEYED_COUNT";
+  const count = Telemetry.getHistogramById(COUNT_ID);
+  const keyed = Telemetry.getKeyedHistogramById(KEYED_ID);
+
+  count.add(1);
+  keyed.add("a", 1);
+
+  let snapshot = Telemetry.getSnapshotForHistograms("main", false, /* filter */ false).parent;
+  let keyedSnapshot = Telemetry.getSnapshotForKeyedHistograms("main", false, /* filter */ false).parent;
+  Assert.ok(COUNT_ID in snapshot, "test histogram should be snapshotted");
+  Assert.ok(KEYED_ID in keyedSnapshot, "test keyed histogram should be snapshotted");
+
+  snapshot = Telemetry.getSnapshotForHistograms("main", false, /* filter */ true).parent;
+  keyedSnapshot = Telemetry.getSnapshotForKeyedHistograms("main", false, /* filter */ true).parent;
+  Assert.ok(!(COUNT_ID in snapshot), "test histogram should not be snapshotted");
+  Assert.ok(!(KEYED_ID in keyedSnapshot), "test keyed histogram should not be snapshotted");
+});
+
+add_task(function test_scalar_filtering() {
+  const COUNT_ID = "telemetry.test.unsigned_int_kind";
+  const KEYED_ID = "telemetry.test.keyed_unsigned_int";
+
+  Telemetry.scalarSet(COUNT_ID, 2);
+  Telemetry.keyedScalarSet(KEYED_ID, "a", 2);
+
+  let snapshot = Telemetry.getSnapshotForScalars("main", false, /* filter */ false).parent;
+  let keyedSnapshot = Telemetry.getSnapshotForKeyedScalars("main", false, /* filter */ false).parent;
+  Assert.ok(COUNT_ID in snapshot, "test scalars should be snapshotted");
+  Assert.ok(KEYED_ID in keyedSnapshot, "test keyed scalars should be snapshotted");
+
+  snapshot = Telemetry.getSnapshotForScalars("main", false, /* filter */ true).parent;
+  keyedSnapshot = Telemetry.getSnapshotForKeyedScalars("main", false, /* filter */ true).parent;
+  Assert.ok(!(COUNT_ID in snapshot), "test scalars should not be snapshotted");
+  Assert.ok(!(KEYED_ID in keyedSnapshot), "test keyed scalars should not be snapshotted");
+});
+
 add_task(async function stopServer() {
   await PingServer.stop();
 });

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

@@ -753,6 +753,18 @@ add_task(async function test_datasets() {
 
   const currentRecordExtended = Telemetry.canRecordExtended;
 
+  // Clear everything out
+  Telemetry.getSnapshotForHistograms("main", true /* clear */);
+  Telemetry.getSnapshotForKeyedHistograms("main", true /* clear */);
+
+  // Empty histograms are filtered. Let's record what we check below.
+  Telemetry.getHistogramById("TELEMETRY_TEST_RELEASE_OPTIN").add(1);
+  Telemetry.getHistogramById("TELEMETRY_TEST_RELEASE_OPTOUT").add(1);
+  // Keyed flag histograms are skipped if empty, let's add data
+  Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_FLAG").add("a", 1);
+  Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTIN").add("a", 1);
+  Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT").add("a", 1);
+
   // Check that registeredHistogram works properly
   Telemetry.canRecordExtended = true;
   let registered = Telemetry.getSnapshotForHistograms("main", false /* clear */);

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

@@ -6,164 +6,6 @@ ChromeUtils.import("resource://gre/modules/Preferences.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
 ChromeUtils.import("resource://gre/modules/UpdateUtils.jsm", this);
 
-add_task(async function testHistogramPacking() {
-  const HISTOGRAM_SNAPSHOT = {
-    sample_process: {
-      HISTOGRAM_1_DATA: {
-        range: [1, 2],
-        bucket_count: 3,
-        histogram_type: 4,
-        values: {
-          "0": 1,
-          "1": 0,
-        },
-        sum: 1,
-      },
-      TELEMETRY_TEST_HISTOGRAM_2_DATA: {
-        range: [1, 2],
-        bucket_count: 3,
-        histogram_type: 4,
-        values: {
-          "0": 1,
-          "1": 0,
-        },
-        sum: 1,
-      },
-    },
-  };
-
-  const HISTOGRAM_1_DATA = {
-    range: [1, 2],
-    bucket_count: 3,
-    histogram_type: 4,
-    values: {
-      "0": 1,
-      "1": 0,
-    },
-    sum: 1,
-  };
-
-  const packedHistograms =
-    TelemetryUtils.packHistograms(HISTOGRAM_SNAPSHOT, false /* testingMode */);
-
-  // Check that it worked and that TELEMETRY_TEST_* got filtered.
-  const EXPECTED_DATA =  {
-    sample_process: {
-      HISTOGRAM_1_DATA,
-    },
-  };
-  Assert.ok(!("TELEMETRY_TEST_HISTOGRAM_2_DATA" in packedHistograms.sample_process),
-            "Test histograms must not be reported outside of test mode.");
-  Assert.ok(ObjectUtils.deepEqual(EXPECTED_DATA, packedHistograms),
-            "Packed data must be correct.");
-
-  // Do the same packing in testing mode.
-  const packedHistogramsTest =
-    TelemetryUtils.packHistograms(HISTOGRAM_SNAPSHOT, true /* testingMode */);
-
-  // Check that TELEMETRY_TEST_* is there.
-  const EXPECTED_DATA_TEST =  {
-    sample_process: {
-      HISTOGRAM_1_DATA,
-      TELEMETRY_TEST_HISTOGRAM_2_DATA: HISTOGRAM_1_DATA,
-    },
-  };
-  Assert.ok("TELEMETRY_TEST_HISTOGRAM_2_DATA" in packedHistogramsTest.sample_process,
-            "Test histograms must be reported in test mode.");
-  Assert.ok(ObjectUtils.deepEqual(EXPECTED_DATA_TEST, packedHistogramsTest),
-            "Packed data must be correct.");
-});
-
-add_task(async function testKeyedHistogramPacking() {
-  const KEYED_HISTOGRAM_SNAPSHOT = {
-    sample_process: {
-      HISTOGRAM_1_DATA: {
-        someKey: {
-          range: [1, 2],
-          bucket_count: 3,
-          histogram_type: 4,
-          values: {
-            "0": 1,
-            "1": 0,
-          },
-          sum: 1,
-        },
-        otherKey: {
-          range: [1, 2],
-          bucket_count: 3,
-          histogram_type: 4,
-          values: {
-            "0": 1,
-            "1": 0,
-          },
-          sum: 1,
-        },
-      },
-      TELEMETRY_TEST_HISTOGRAM_2_DATA: {
-        someKey: {
-          range: [1, 2],
-          bucket_count: 3,
-          histogram_type: 4,
-          values: {
-            "0": 1,
-            "1": 0,
-          },
-          sum: 1,
-        },
-      },
-    },
-  };
-
-  const someKey = {
-    range: [1, 2],
-    bucket_count: 3,
-    histogram_type: 4,
-    values: {
-      "0": 1,
-      "1": 0,
-    },
-    sum: 1,
-  };
-
-  const packedKeyedHistograms =
-    TelemetryUtils.packKeyedHistograms(KEYED_HISTOGRAM_SNAPSHOT, false /* testingMode */);
-
-  // Check that it worked and that TELEMETRY_TEST_* got filtered.
-  const EXPECTED_DATA =  {
-    sample_process: {
-      HISTOGRAM_1_DATA: {
-        someKey,
-        otherKey: someKey,
-      },
-    },
-  };
-  Assert.ok(!("TELEMETRY_TEST_HISTOGRAM_2_DATA" in packedKeyedHistograms.sample_process),
-            "Test histograms must not be reported outside of test mode.");
-  Assert.ok(ObjectUtils.deepEqual(EXPECTED_DATA, packedKeyedHistograms),
-            "Packed data must be correct.");
-
-  // Do the same packing in testing mode.
-  const packedKeyedHistogramsTest =
-    TelemetryUtils.packKeyedHistograms(KEYED_HISTOGRAM_SNAPSHOT, true /* testingMode */);
-
-  // Check that TELEMETRY_TEST_* is there.
-  const EXPECTED_DATA_TEST =  {
-    sample_process: {
-      HISTOGRAM_1_DATA: {
-        someKey,
-        otherKey: someKey,
-      },
-      TELEMETRY_TEST_HISTOGRAM_2_DATA: {
-        someKey,
-      },
-    },
-  };
-  Assert.ok("TELEMETRY_TEST_HISTOGRAM_2_DATA" in packedKeyedHistogramsTest.sample_process,
-            "Test histograms must be reported in test mode.");
-  Assert.ok(ObjectUtils.deepEqual(EXPECTED_DATA_TEST, packedKeyedHistogramsTest),
-            "Packed data must be correct.");
-});
-
 add_task(async function testUpdateChannelOverride() {
   if (Preferences.has(TelemetryUtils.Preferences.OverrideUpdateChannel)) {
     // If the pref is already set at this point, the test is running in a build