Bug 1510988 - Make store name optional and default to "main" r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Mon, 21 Jan 2019 15:57:06 +0000
changeset 514708 85ba084b3f903f4b479bf07f2c62f5184f61e44c
parent 514707 12d1bf5d9b3057c483c149826c0ca090fbe2b79c
child 514724 a9ff8ccd4e3ddb1c53267f98a75a64a3a74f87bb
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1510988
milestone66.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 1510988 - Make store name optional and default to "main" r=chutten Depends on D16967 Differential Revision: https://phabricator.services.mozilla.com/D16968
toolkit/components/telemetry/core/Telemetry.cpp
toolkit/components/telemetry/core/nsITelemetry.idl
toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
toolkit/components/telemetry/tests/unit/test_TelemetryScalars_multistore.js
--- a/toolkit/components/telemetry/core/Telemetry.cpp
+++ b/toolkit/components/telemetry/core/Telemetry.cpp
@@ -588,57 +588,65 @@ TelemetryImpl::SetHistogramRecordingEnab
   return TelemetryHistogram::SetHistogramRecordingEnabled(id, aEnabled);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForHistograms(const nsACString& aStoreName,
                                         bool aClearStore, bool aFilterTest,
                                         JSContext* aCx,
                                         JS::MutableHandleValue aResult) {
+  NS_NAMED_LITERAL_CSTRING(defaultStore, "main");
   unsigned int dataset = mCanRecordExtended
                              ? nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN
                              : nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
   return TelemetryHistogram::CreateHistogramSnapshots(
-      aCx, aResult, aStoreName, dataset, aClearStore, aFilterTest);
+      aCx, aResult, aStoreName.IsVoid() ? defaultStore : aStoreName, dataset,
+      aClearStore, aFilterTest);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForKeyedHistograms(const nsACString& aStoreName,
                                              bool aClearStore, bool aFilterTest,
                                              JSContext* aCx,
                                              JS::MutableHandleValue aResult) {
+  NS_NAMED_LITERAL_CSTRING(defaultStore, "main");
   unsigned int dataset = mCanRecordExtended
                              ? nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN
                              : nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
   return TelemetryHistogram::GetKeyedHistogramSnapshots(
-      aCx, aResult, aStoreName, dataset, aClearStore, aFilterTest);
+      aCx, aResult, aStoreName.IsVoid() ? defaultStore : aStoreName, dataset,
+      aClearStore, aFilterTest);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForScalars(const nsACString& aStoreName,
                                      bool aClearStore, bool aFilterTest,
                                      JSContext* aCx,
                                      JS::MutableHandleValue aResult) {
+  NS_NAMED_LITERAL_CSTRING(defaultStore, "main");
   unsigned int dataset = mCanRecordExtended
                              ? nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN
                              : nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
-  return TelemetryScalar::CreateSnapshots(dataset, aClearStore, aCx, 1, aResult,
-                                          aFilterTest, aStoreName);
+  return TelemetryScalar::CreateSnapshots(
+      dataset, aClearStore, aCx, 1, aResult, aFilterTest,
+      aStoreName.IsVoid() ? defaultStore : aStoreName);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetSnapshotForKeyedScalars(const nsACString& aStoreName,
                                           bool aClearStore, bool aFilterTest,
                                           JSContext* aCx,
                                           JS::MutableHandleValue aResult) {
+  NS_NAMED_LITERAL_CSTRING(defaultStore, "main");
   unsigned int dataset = mCanRecordExtended
                              ? nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN
                              : nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT;
   return TelemetryScalar::CreateKeyedSnapshots(
-      dataset, aClearStore, aCx, 1, aResult, aFilterTest, aStoreName);
+      dataset, aClearStore, aCx, 1, aResult, aFilterTest,
+      aStoreName.IsVoid() ? defaultStore : aStoreName);
 }
 
 bool TelemetryImpl::GetSQLStats(JSContext* cx, JS::MutableHandle<JS::Value> ret,
                                 bool includePrivateSql) {
   JS::Rooted<JSObject*> root_obj(cx, JS_NewPlainObject(cx));
   if (!root_obj) return false;
   ret.setObject(*root_obj);
 
--- a/toolkit/components/telemetry/core/nsITelemetry.idl
+++ b/toolkit/components/telemetry/core/nsITelemetry.idl
@@ -59,82 +59,86 @@ interface nsITelemetry : nsISupports
    * Each histogram is represented in a packed format and has the following properties:
    *   bucket_count - Number of buckets of this histogram
    *   histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, HISTOGRAM_BOOLEAN,
    *                    HISTOGRAM_FLAG, HISTOGRAM_COUNT, or HISTOGRAM_CATEGORICAL
    *   sum - sum of the bucket contents
    *   range - A 2-item array of minimum and maximum bucket size
    *   values - Map from bucket to the bucket's count
    *
-   * @param aStoreName The name of the store to snapshot. "main" will be generally available.
+   * @param aStoreName The name of the store to snapshot.
+   *                   Defaults to "main".
    *                   Custom stores are available when probes have them defined.
    *                   See the `record_into_store` attribute on histograms.
    *                   @see https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/histograms.html#record-into-store
    * @param aClearStore Whether to clear out the histograms in the named store after snapshotting.
    *                    Defaults to false.
    * @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, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
+  jsval getSnapshotForHistograms([optional] in ACString aStoreName, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the keyed histograms from the given store to a JSON-style object.
    * The returned structure looks like:
    *   { "process": { "name1": { "key_1": histogramData1, "key_2": histogramData2 }, ...}, ... }
    *
-   * @param aStoreName The name of the store to snapshot. "main" will be generally available.
+   * @param aStoreName The name of the store to snapshot.
+   *                   Defaults to "main".
    *                   Custom stores are available when probes have them defined.
    *                   See the `record_into_store` attribute on histograms.
    *                   @see https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/histograms.html#record-into-store
    * @param aClearStore Whether to clear out the keyed histograms in the named store after snapshotting.
    *                    Defaults to false.
    * @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, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
+  jsval getSnapshotForKeyedHistograms([optional] in ACString aStoreName, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the scalars from the given store to a JSON-style object.
    * The returned structure looks like:
    *   { "process": { "category1.probe": 1,"category1.other_probe": false, ... }, ... }.
    *
-   * @param aStoreName The name of the store to snapshot. "main" will be generally available.
+   * @param aStoreName The name of the store to snapshot.
+   *                   Defaults to "main".
    *                   Custom stores are available when probes have them defined.
    *                   See the `record_into_store` attribute on scalars.
    *                   @see https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/scalars.html#optional-fields
    * @param aClearStore Whether to clear out the scalars in the named store after snapshotting.
    *                    Defaults to false.
    * @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, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
+  jsval getSnapshotForScalars([optional] in ACString aStoreName, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * Serializes the keyed scalars from the given store to a JSON-style object.
    * The returned structure looks like:
    *   { "process": { "category1.probe": { "key_1": 2, "key_2": 1, ... }, ... }, ... }
    *
-   * @param aStoreName The name of the store to snapshot. "main" will be generally available.
+   * @param aStoreName The name of the store to snapshot.
+   *                   Defaults to "main".
    *                   Custom stores are available when probes have them defined.
    *                   See the `record_into_store` attribute on scalars.
    *                   @see https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/scalars.html#optional-fields
    * @param aClearStore Whether to clear out the keyed scalars in the named store after snapshotting.
    *                    Defaults to false.
    * @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, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
+  jsval getSnapshotForKeyedScalars([optional] in ACString aStoreName, [optional] in boolean aClearStore, [optional] in boolean aFilterTest);
 
   /**
    * The amount of time, in milliseconds, that the last session took
    * to shutdown.  Reads as 0 to indicate failure.
    */
   readonly attribute uint32_t lastShutdownDuration;
 
   /**
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js
@@ -1263,17 +1263,26 @@ add_task(async function test_multistore_
   id = "TELEMETRY_TEST_MULTIPLE_STORES";
   hist = Telemetry.getHistogramById(id);
   hist.add(1);
 
   id = "TELEMETRY_TEST_SYNC_ONLY";
   hist = Telemetry.getHistogramById(id);
   hist.add(1);
 
-  // Getting snapshot and clearing
+  // Getting snapshot and NOT clearing (using default values for optional parameters)
+  snapshot = Telemetry.getSnapshotForHistograms().parent;
+  id = "TELEMETRY_TEST_MAIN_ONLY";
+  Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
+  id = "TELEMETRY_TEST_MULTIPLE_STORES";
+  Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
+  id = "TELEMETRY_TEST_SYNC_ONLY";
+  Assert.ok(!(id in snapshot), `${id} should not be in a main store snapshot`);
+
+  // Data should still be in, getting snapshot and clearing
   snapshot = Telemetry.getSnapshotForHistograms("main", /* clear */ true).parent;
   id = "TELEMETRY_TEST_MAIN_ONLY";
   Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
   id = "TELEMETRY_TEST_MULTIPLE_STORES";
   Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
   id = "TELEMETRY_TEST_SYNC_ONLY";
   Assert.ok(!(id in snapshot), `${id} should not be in a main store snapshot`);
 
@@ -1292,17 +1301,24 @@ add_task(async function test_multistore_
   id = "TELEMETRY_TEST_KEYED_MULTIPLE_STORES";
   hist = Telemetry.getKeyedHistogramById(id);
   hist.add("key-a", 1);
 
   id = "TELEMETRY_TEST_KEYED_SYNC_ONLY";
   hist = Telemetry.getKeyedHistogramById(id);
   hist.add("key-b", 1);
 
-  // Getting snapshot and clearing
+  // Getting snapshot and NOT clearing (using default values for optional parameters)
+  snapshot = Telemetry.getSnapshotForKeyedHistograms().parent;
+  id = "TELEMETRY_TEST_KEYED_MULTIPLE_STORES";
+  Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
+  id = "TELEMETRY_TEST_KEYED_SYNC_ONLY";
+  Assert.ok(!(id in snapshot), `${id} should not be in a main store snapshot`);
+
+  // Data should still be in, getting snapshot and clearing
   snapshot = Telemetry.getSnapshotForKeyedHistograms("main", /* clear */ true).parent;
   id = "TELEMETRY_TEST_KEYED_MULTIPLE_STORES";
   Assert.ok(id in snapshot, `${id} should be in a main store snapshot`);
   id = "TELEMETRY_TEST_KEYED_SYNC_ONLY";
   Assert.ok(!(id in snapshot), `${id} should not be in a main store snapshot`);
 
   // Should be empty after clearing
   snapshot = Telemetry.getSnapshotForKeyedHistograms("main", /* clear */ false).parent;
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryScalars_multistore.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryScalars_multistore.js
@@ -216,8 +216,59 @@ add_task(async function test_empty_absen
   Assert.ok(MULTIPLE_STORES_KEYED in snapshot,
             `${MULTIPLE_STORES_KEYED} must be in the sync snapshot.`);
   Assert.ok(key in snapshot[MULTIPLE_STORES_KEYED],
             `${MULTIPLE_STORES_KEYED} must have the stored key.`);
   Assert.equal(snapshot[MULTIPLE_STORES_KEYED][key], 1,
                `${MULTIPLE_STORES_KEYED}[${key}] should have the correct value.`);
 
 });
+
+add_task(async function test_multistore_default_values() {
+  Telemetry.clearScalars();
+
+  const expectedUint = 3785;
+  const expectedKey = "some key";
+  Telemetry.scalarSet(MAIN_ONLY, expectedUint);
+  Telemetry.scalarSet(SYNC_ONLY, expectedUint);
+  Telemetry.scalarSet(MULTIPLE_STORES, expectedUint);
+  Telemetry.keyedScalarSet(MULTIPLE_STORES_KEYED, expectedKey, expectedUint);
+
+  let mainScalars;
+  let mainKeyedScalars;
+
+  // Getting snapshot and NOT clearing (using default values for optional parameters)
+  mainScalars = Telemetry.getSnapshotForScalars().parent;
+  mainKeyedScalars = Telemetry.getSnapshotForKeyedScalars().parent;
+
+  Assert.equal(mainScalars[MAIN_ONLY], expectedUint,
+               `Main-store scalar ${MAIN_ONLY} must have correct value.`);
+  Assert.ok(!(SYNC_ONLY in mainScalars),
+            `Sync-store scalar ${SYNC_ONLY} must not be in main snapshot.`);
+  Assert.equal(mainScalars[MULTIPLE_STORES], expectedUint,
+               `Multi-store scalar ${MULTIPLE_STORES} must have correct value in main store.`);
+  Assert.equal(mainKeyedScalars[MULTIPLE_STORES_KEYED][expectedKey], expectedUint,
+               `Multi-store scalar ${MULTIPLE_STORES_KEYED} must have correct value in main store.`);
+
+  // Getting snapshot and clearing
+  mainScalars = Telemetry.getSnapshotForScalars("main", true).parent;
+  mainKeyedScalars = Telemetry.getSnapshotForKeyedScalars("main", true).parent;
+
+  Assert.equal(mainScalars[MAIN_ONLY], expectedUint,
+               `Main-store scalar ${MAIN_ONLY} must have correct value.`);
+  Assert.ok(!(SYNC_ONLY in mainScalars),
+            `Sync-store scalar ${SYNC_ONLY} must not be in main snapshot.`);
+  Assert.equal(mainScalars[MULTIPLE_STORES], expectedUint,
+               `Multi-store scalar ${MULTIPLE_STORES} must have correct value in main store.`);
+  Assert.equal(mainKeyedScalars[MULTIPLE_STORES_KEYED][expectedKey], expectedUint,
+               `Multi-store scalar ${MULTIPLE_STORES_KEYED} must have correct value in main store.`);
+
+  // Getting snapshot (with default values), should be empty now
+  mainScalars = Telemetry.getSnapshotForScalars().parent || {};
+  mainKeyedScalars = Telemetry.getSnapshotForKeyedScalars().parent || {};
+
+  Assert.ok(!(MAIN_ONLY in mainScalars),
+               `Main-store scalar ${MAIN_ONLY} must not be in main snapshot.`);
+  Assert.ok(!(MULTIPLE_STORES in mainScalars),
+               `Multi-store scalar ${MULTIPLE_STORES} must not be in main snapshot.`);
+  Assert.ok(!(MULTIPLE_STORES_KEYED in mainKeyedScalars),
+               `Multi-store scalar ${MULTIPLE_STORES_KEYED} must not be in main snapshot.`);
+});