diff --git a/toolkit/components/telemetry/app/TelemetryUtils.jsm b/toolkit/components/telemetry/app/TelemetryUtils.jsm index 42da8c5ebac3..95272725f0b0 100644 --- a/toolkit/components/telemetry/app/TelemetryUtils.jsm +++ b/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} - * - * { - * "": { - * "": { - * range: [min, max], - * bucket_count: , - * histogram_type: , - * 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} - * - * { - * "": { - * "": { - * "": { - * range: [min, max], - * bucket_count: , - * histogram_type: , - * 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. diff --git a/toolkit/components/telemetry/core/Telemetry.cpp b/toolkit/components/telemetry/core/Telemetry.cpp index c1364d676e2a..7284c39373a9 100644 --- a/toolkit/components/telemetry/core/Telemetry.cpp +++ b/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 diff --git a/toolkit/components/telemetry/core/TelemetryHistogram.cpp b/toolkit/components/telemetry/core/TelemetryHistogram.cpp index 6953f350cc93..45a69012201a 100644 --- a/toolkit/components/telemetry/core/TelemetryHistogram.cpp +++ b/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(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 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 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; diff --git a/toolkit/components/telemetry/core/TelemetryHistogram.h b/toolkit/components/telemetry/core/TelemetryHistogram.h index 0bd92a144ae1..4ca667ed88ef 100644 --- a/toolkit/components/telemetry/core/TelemetryHistogram.h +++ b/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); diff --git a/toolkit/components/telemetry/core/TelemetryScalar.cpp b/toolkit/components/telemetry/core/TelemetryScalar.cpp index 5e93d42a0b5d..f07bdca892b6 100644 --- a/toolkit/components/telemetry/core/TelemetryScalar.cpp +++ b/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 aResult) + uint8_t optional_argc, JS::MutableHandle 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 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 aResult) + uint8_t optional_argc, JS::MutableHandle 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; } } diff --git a/toolkit/components/telemetry/core/TelemetryScalar.h b/toolkit/components/telemetry/core/TelemetryScalar.h index 283c2a2a703e..346d9470ed13 100644 --- a/toolkit/components/telemetry/core/TelemetryScalar.h +++ b/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 aResult); + JS::MutableHandle 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 aResult); + JS::MutableHandle aResult, bool aFilterTest=false); // C++ API Endpoints. void Add(mozilla::Telemetry::ScalarID aId, uint32_t aValue); diff --git a/toolkit/components/telemetry/core/nsITelemetry.idl b/toolkit/components/telemetry/core/nsITelemetry.idl index 791e584253cf..3eb305c88190 100644 --- a/toolkit/components/telemetry/core/nsITelemetry.idl +++ b/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. diff --git a/toolkit/components/telemetry/geckoview/GeckoViewTelemetryController.jsm b/toolkit/components/telemetry/geckoview/GeckoViewTelemetryController.jsm index 1d3d567f6e5a..ef394a1e15ae 100644 --- a/toolkit/components/telemetry/geckoview/GeckoViewTelemetryController.jsm +++ b/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, }; diff --git a/toolkit/components/telemetry/pings/TelemetrySession.jsm b/toolkit/components/telemetry/pings/TelemetrySession.jsm index a01de8cb56b2..4c84ce463175 100644 --- a/toolkit/components/telemetry/pings/TelemetrySession.jsm +++ b/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; }, /** diff --git a/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp b/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp index c9facb2b94ba..6d290e8362bb 100644 --- a/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp +++ b/toolkit/components/telemetry/tests/gtest/TelemetryTestHelpers.cpp @@ -109,9 +109,9 @@ GetScalarsSnapshot(bool aKeyed, JSContext* aCx, JS::MutableHandle 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 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); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js index e6f1e617b5ad..4a36ee3c491a 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js +++ b/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(); }); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js b/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js index 4d340dbd1e16..3a1bae3b13e8 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryHistograms.js +++ b/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 */); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryUtils.js b/toolkit/components/telemetry/tests/unit/test_TelemetryUtils.js index 1b80a0f84d94..29ad4c169e69 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryUtils.js +++ b/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