commit c5d381e83fa79c4bc1982048d1f8ba06afedc103
parent 85aa2b9e74fb6ac9606dd9d02bb80db352c8699e
Author: agoloman <agoloman@mozilla.com>
Date: Tue, 2 Dec 2025 00:17:11 +0200
Revert "Bug 1999541 - Implement Nimbus-controlled shutoff switch for "main" ping histograms r=TravisLong" for causing xpc failures @test_TelemetryController.js.
This reverts commit 3727483b1cee8a341badbb1f895e989132a74ca9.
Revert "Bug 1999541 - Implement Nimbus-controlled shutoff switch for "main" ping scalars r=TravisLong"
This reverts commit 20116405691fc30bd5cc8d915effd27443da719a.
Revert "Bug 1999541 - Implement a Legacy Telemetry shutoff switch for pings controlled by Nimbus r=TravisLong"
This reverts commit 83187fd2a59fcba6c8ca96fc05a1cb73a5a4ad4d.
Diffstat:
6 files changed, 2 insertions(+), 324 deletions(-)
diff --git a/toolkit/components/nimbus/FeatureManifest.yaml b/toolkit/components/nimbus/FeatureManifest.yaml
@@ -2725,38 +2725,6 @@ gleanInternalSdk:
branch: user
pref: telemetry.glean.internal.maxPingsPerMinute
-legacyTelemetry:
- description: "Controls for the Legacy Telemetry data collection system"
- owner: chutten@mozilla.com
- applications:
- - firefox-desktop
- hasExposure: false
- variables:
- disabledPings:
- type: json
- description: |
- A list of Legacy Telemetry pings to disable.
- Pings on this list will not be archived or uploaded.
- On submit their payloads will be dropped.
- Code that collects to and submits the ping will still operate as normal.
- Cannot be used to disable the "main", "first-shutdown", "new-profile",
- or "deletion-request" pings.
- disableMainPingScalars:
- type: boolean
- description: |
- If true, scalars will not be included in "main" pings.
- Exceptions are made for two important engagement measures:
- `browser.engagement.total_uri_count_normal_and_private_mode` and
- `browser.engagement.active_ticks`.
- These scalars will continue to be reported as normal, even if all
- others are disabled by this variable.
- disableMainPingHgrams:
- type: boolean
- description: |
- If true, histograms will not be included in "main" pings.
- Data will still be recorded locally,
- it will merely not be included in "main" pings when they are submitted.
-
browserLowMemoryPrefs:
description: Prefs which control the browser's behaviour under low memory.
owner: haftandilian@mozilla.com
diff --git a/toolkit/components/telemetry/app/TelemetryControllerParent.sys.mjs b/toolkit/components/telemetry/app/TelemetryControllerParent.sys.mjs
@@ -38,7 +38,6 @@ const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
ClientID: "resource://gre/modules/ClientID.sys.mjs",
CoveragePing: "resource://gre/modules/CoveragePing.sys.mjs",
- NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
TelemetryArchive: "resource://gre/modules/TelemetryArchive.sys.mjs",
TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.sys.mjs",
TelemetryEventPing: "resource://gre/modules/EventPing.sys.mjs",
@@ -529,27 +528,6 @@ var Impl = {
JSON.stringify(aOptions)
);
- const disabledPings =
- lazy.NimbusFeatures.legacyTelemetry.getVariable("disabledPings") ?? [];
- const UNCONTROLLABLE_PINGS = [
- "main",
- "first-shutdown",
- "new-profile",
- "deletion-request",
- ];
- if (disabledPings.includes(aType)) {
- if (UNCONTROLLABLE_PINGS.includes(aType)) {
- this._log.warn(
- `submitExternalPing - type: ${aType} not controllable, but is in the list of disabledPings ${JSON.stringify(disabledPings)}. Ping will submit as normal. Please remove ping type "${aType}" from the Nimbus config.`
- );
- } else {
- this._log.trace(
- `submitExternalPing - type ${aType} disabled by Nimbus.`
- );
- return Promise.reject(new Error("Ping disabled."));
- }
- }
-
// Reject pings sent after shutdown.
if (this._shutDown) {
const errorMessage =
diff --git a/toolkit/components/telemetry/pings/TelemetrySession.sys.mjs b/toolkit/components/telemetry/pings/TelemetrySession.sys.mjs
@@ -11,7 +11,6 @@ const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
AddonManagerPrivate: "resource://gre/modules/AddonManager.sys.mjs",
- NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
TelemetryController: "resource://gre/modules/TelemetryController.sys.mjs",
TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.sys.mjs",
TelemetryReportingPolicy:
@@ -464,35 +463,19 @@ var Impl = {
},
getHistograms: function getHistograms(clearSubsession) {
- const snapshot = Services.telemetry.getSnapshotForHistograms(
+ return Services.telemetry.getSnapshotForHistograms(
"main",
clearSubsession,
!this._testing
);
- if (
- lazy.NimbusFeatures.legacyTelemetry.getVariable("disableMainPingHgrams")
- ) {
- this._log.trace("getHistograms - Main ping histograms are disabled.");
- return {};
- }
- return snapshot;
},
getKeyedHistograms(clearSubsession) {
- const snapshot = Services.telemetry.getSnapshotForKeyedHistograms(
+ return Services.telemetry.getSnapshotForKeyedHistograms(
"main",
clearSubsession,
!this._testing
);
- if (
- lazy.NimbusFeatures.legacyTelemetry.getVariable("disableMainPingHgrams")
- ) {
- this._log.trace(
- "getKeyedHistograms - Main ping histograms are disabled."
- );
- return {};
- }
- return snapshot;
},
/**
@@ -524,31 +507,6 @@ var Impl = {
!this._testing
);
- if (
- lazy.NimbusFeatures.legacyTelemetry.getVariable("disableMainPingScalars")
- ) {
- this._log.trace("getScalars - Main ping scalars are disabled.");
- if (keyed) {
- // We don't need to preserve any keyed scalars.
- scalarsSnapshot = {};
- } else {
- let filteredSnapshot = {};
- const scalarsToKeep = [
- "browser.engagement.total_uri_count_normal_and_private_mode",
- "browser.engagement.active_ticks",
- ];
- for (let scalar of scalarsToKeep) {
- if ("parent" in scalarsSnapshot && scalar in scalarsSnapshot.parent) {
- if (!("parent" in filteredSnapshot)) {
- filteredSnapshot.parent = {};
- }
- filteredSnapshot.parent[scalar] = scalarsSnapshot.parent[scalar];
- }
- }
- scalarsSnapshot = filteredSnapshot;
- }
- }
-
return scalarsSnapshot;
},
diff --git a/toolkit/components/telemetry/tests/unit/test_MainPingDisablement.js b/toolkit/components/telemetry/tests/unit/test_MainPingDisablement.js
@@ -1,116 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/
-*/
-/**
- * Tests for disabling pieces of the "main" ping.
- */
-
-const { NimbusFeatures } = ChromeUtils.importESModule(
- "resource://nimbus/ExperimentAPI.sys.mjs"
-);
-const { NimbusTestUtils } = ChromeUtils.importESModule(
- "resource://testing-common/NimbusTestUtils.sys.mjs"
-);
-const { TelemetrySession } = ChromeUtils.importESModule(
- "resource://gre/modules/TelemetrySession.sys.mjs"
-);
-
-NimbusTestUtils.init(this);
-
-add_setup(async function () {
- // FOG needs a profile directory
- do_get_profile();
- Services.fog.initializeFOG();
-
- // Make sure we don't generate unexpected pings due to pref changes.
- await setEmptyPrefWatchlist();
-
- Services.prefs.setBoolPref(TelemetryUtils.Preferences.FhrUploadEnabled, true);
-
- await TelemetryController.testSetup();
-});
-
-add_task(async function test_scalarDisablement() {
- const URI_COUNT_SCALAR =
- "browser.engagement.total_uri_count_normal_and_private_mode";
- const ACTIVE_TICKS_SCALAR = "browser.engagement.active_ticks";
- const LAST_SHUTDOWN_SCALAR = "browser.timings.last_shutdown";
-
- // Ensure there's data to be snapshotted.
- // Both some that shouldn't be filtered...
- Glean.browserEngagement.uriCount.add(1);
- Glean.browserEngagement.activeTicks.add(1);
- // ...and some that should be.
- Glean.browserTimings.lastShutdown.set(42);
-
- info("1. Ensure things begin by storing and reporting as expected.");
- let payload = TelemetrySession.getPayload(
- "reason",
- /*clearSubsession*/ false
- );
-
- Assert.greater(payload.processes.parent.scalars[URI_COUNT_SCALAR], 0);
- Assert.greater(payload.processes.parent.scalars[ACTIVE_TICKS_SCALAR], 0);
- Assert.greater(payload.processes.parent.scalars[LAST_SHUTDOWN_SCALAR], 0);
-
- info("2. Ensure we can disable scalars, leaving important ones intact.");
- const { cleanup } = await NimbusTestUtils.setupTest();
-
- let nimbusCleanup = await NimbusTestUtils.enrollWithFeatureConfig({
- featureId: NimbusFeatures.legacyTelemetry.featureId,
- value: {
- disableMainPingScalars: true,
- },
- });
-
- let filtered = TelemetrySession.getPayload(
- "reason",
- /*clearSubsession*/ false
- );
-
- Assert.greater(filtered.processes.parent.scalars[URI_COUNT_SCALAR], 0);
- Assert.greater(filtered.processes.parent.scalars[ACTIVE_TICKS_SCALAR], 0);
- Assert.ok(!(LAST_SHUTDOWN_SCALAR in filtered.processes.parent.scalars));
-
- await nimbusCleanup();
- await cleanup();
-});
-
-add_task(async function test_hgramDisablement() {
- const ARCHIVE_HGRAM = "TELEMETRY_ARCHIVE_DIRECTORIES_COUNT";
- const SEND_KEYED_HGRAM = "TELEMETRY_SEND_FAILURE_TYPE_PER_PING";
- // Let's check both normal and keyed.
- Glean.telemetry.archiveDirectoriesCount.accumulateSingleSample(42);
- Glean.telemetry.sendFailureTypePerPing.get("some-ping", "eOK").add(1);
-
- info("1. Ensure histogram data is reported normally to begin with.");
-
- let payload = TelemetrySession.getPayload(
- "reason",
- /*clearSubsession*/ false
- );
-
- Assert.ok(ARCHIVE_HGRAM in payload.histograms);
- Assert.ok(SEND_KEYED_HGRAM in payload.keyedHistograms);
-
- info("2. Ensure we can disable histograms.");
- const { cleanup } = await NimbusTestUtils.setupTest();
-
- let nimbusCleanup = await NimbusTestUtils.enrollWithFeatureConfig({
- featureId: NimbusFeatures.legacyTelemetry.featureId,
- value: {
- disableMainPingHgrams: true,
- },
- });
-
- let filtered = TelemetrySession.getPayload(
- "reason",
- /*clearSubsession*/ false
- );
-
- Assert.ok(!(ARCHIVE_HGRAM in filtered.histograms));
- Assert.ok(!(SEND_KEYED_HGRAM in filtered.keyedHistograms));
-
- await nimbusCleanup();
- await cleanup();
-});
diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ -11,12 +11,6 @@
const { ClientID } = ChromeUtils.importESModule(
"resource://gre/modules/ClientID.sys.mjs"
);
-const { ExperimentAPI, NimbusFeatures } = ChromeUtils.importESModule(
- "resource://nimbus/ExperimentAPI.sys.mjs"
-);
-const { NimbusTestUtils } = ChromeUtils.importESModule(
- "resource://testing-common/NimbusTestUtils.sys.mjs"
-);
const { TelemetryController } = ChromeUtils.importESModule(
"resource://gre/modules/TelemetryController.sys.mjs"
);
@@ -39,8 +33,6 @@ const { TestUtils } = ChromeUtils.importESModule(
"resource://testing-common/TestUtils.sys.mjs"
);
-NimbusTestUtils.init(this);
-
const PING_FORMAT_VERSION = 4;
const DELETION_REQUEST_PING_TYPE = "deletion-request";
const TEST_PING_TYPE = "test-ping-type";
@@ -905,103 +897,6 @@ add_task(async function test_sendNewProfile() {
PingServer.resetPingHandler();
});
-add_task(async function test_pingDisablement() {
- await TelemetryController.testReset();
- PingServer.clearRequests();
-
- info("1. Ensure test pings can be sent.");
- let docid = await sendPing(false, false);
- let request = await PingServer.promiseNextRequest();
-
- let ping = decodeRequestPayload(request);
- Assert.equal(docid, ping.id, "Server delivered the ping we just submitted.");
- checkPingFormat(ping, TEST_PING_TYPE, false, false);
-
- info("2. Ensure we can disable a ping by name.");
- const { cleanup } = await NimbusTestUtils.setupTest();
- registerCleanupFunction(cleanup);
- await ExperimentAPI.ready();
- let nimbusCleanup = await NimbusTestUtils.enrollWithFeatureConfig({
- featureId: NimbusFeatures.legacyTelemetry.featureId,
- value: {
- disabledPings: [TEST_PING_TYPE],
- },
- });
-
- PingServer.registerPingHandler(() =>
- Assert.ok(false, "Telemetry must not send the disabled ping.")
- );
- await Assert.rejects(
- sendPing(true, true),
- /Ping disabled/,
- "Disabled ping should not send."
- );
-
- PingServer.resetPingHandler();
-
- info("3. Ensure disabling one kind of ping doesn't disable others.");
- const OTHER_PING_TYPE = TEST_PING_TYPE + "-other";
- await TelemetryController.submitExternalPing(OTHER_PING_TYPE, {}, {});
- request = await PingServer.promiseNextRequest();
-
- ping = decodeRequestPayload(request);
- checkPingFormat(ping, OTHER_PING_TYPE, false, false);
-
- await nimbusCleanup();
-});
-
-add_task(async function test_cantDisableImportantPings() {
- await TelemetryController.testReset();
- PingServer.clearRequests();
-
- const DO_NOT_DISABLE_THESE_PINGS = [
- "main",
- "first-shutdown",
- "new-profile",
- "deletion-request",
- ];
- const PINGS = [TEST_PING_TYPE, ...DO_NOT_DISABLE_THESE_PINGS];
- let nimbusCleanup = await NimbusTestUtils.enrollWithFeatureConfig({
- featureId: NimbusFeatures.legacyTelemetry.featureId,
- value: {
- disabledPings: PINGS,
- },
- });
-
- for (const pingName of PINGS) {
- info("Check " + pingName);
- if (DO_NOT_DISABLE_THESE_PINGS.includes(pingName)) {
- let docid = await TelemetryController.submitExternalPing(
- pingName,
- {},
- {}
- );
- let request = await PingServer.promiseNextRequest();
- let ping = decodeRequestPayload(request);
- Assert.equal(
- docid,
- ping.id,
- "Server delivered the ping we just submitted."
- );
- checkPingFormat(ping, pingName, false, false);
- // Ensure we don't get throttled for too many pings in a row.
- await TelemetrySend.reset();
- } else {
- PingServer.registerPingHandler(() =>
- Assert.ok(false, "Telemetry must not send the disabled ping.")
- );
- await Assert.rejects(
- TelemetryController.submitExternalPing(pingName, {}, {}),
- /Ping disabled/,
- "Disabled ping should not send."
- );
- PingServer.resetPingHandler();
- }
- }
-
- await nimbusCleanup();
-});
-
// Testing shutdown and checking that pings sent afterwards are rejected.
add_task(async function test_pingRejection() {
await TelemetryController.testReset();
diff --git a/toolkit/components/telemetry/tests/unit/xpcshell.toml b/toolkit/components/telemetry/tests/unit/xpcshell.toml
@@ -40,11 +40,6 @@ skip-if = [
]
tags = "addons"
-["test_MainPingDisablement.js"]
-run-if = [
- "os != 'android'", # Legacy telemetry is always disabled on Android
-]
-
["test_MigratePendingPings.js"]
run-if = [
"os != 'android'", # Legacy telemetry is a lways disabled on Android
