Bug 1798919 - Add "metrics_disabled" to the 'glean' feature in toolkit/components/nimbus/FeatureManifest.yaml, r=chutten

This includes:
- the addition of FOG wrappes around the Glean API for setting this feature configuration
- handling for fetching the initial configuration after FOG initializes in BrowserGlue.jsm
- registering a callback with Nimbus to listen for, and set, any new feature configurations
- test coverage of this functionality

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

browser/components/BrowserGlue.sys.mjs

@@ -61,6 +61,7 @@ XPCOMUtils.defineLazyModuleGetters(lazy, {
   Corroborate: "resource://gre/modules/Corroborate.jsm",
   Discovery: "resource:///modules/Discovery.jsm",
   DoHController: "resource:///modules/DoHController.jsm",
+  ExperimentAPI: "resource://nimbus/ExperimentAPI.jsm",
   ExtensionsUI: "resource:///modules/ExtensionsUI.jsm",
   FeatureGate: "resource://featuregates/FeatureGate.jsm",
   FxAccounts: "resource://gre/modules/FxAccounts.jsm",
@@ -2713,6 +2714,15 @@ BrowserGlue.prototype = {
         name: "initializeFOG",
         task: () => {
           Services.fog.initializeFOG();
+
+          // Register Glean to listen for experiment updates releated to the
+          // "glean" feature defined in the t/c/nimbus/FeatureManifest.yaml
+          lazy.ExperimentAPI.on("update", { featureId: "glean" }, () => {
+            let cfg = lazy.NimbusFeatures.serverKnobs.getVariable(
+              "metricsDisabled"
+            );
+            Services.fog.setMetricsFeatureConfig(JSON.stringify(cfg));
+          });
         },
       },
 

toolkit/components/glean/src/lib.rs

@@ -191,3 +191,14 @@ pub extern "C" fn fog_test_get_experiment_data(
         }
     }
 }
+
+/// Sets the remote feature configuration.
+///
+/// See [`glean_core::Glean::set_metrics_disabled_config`].
+#[no_mangle]
+pub extern "C" fn fog_set_metrics_feature_config(config_json: &nsACString) {
+    if config_json == "null" || config_json.is_empty() {
+        glean::glean_set_metrics_disabled_config("{}".to_owned());
+    }
+    glean::glean_set_metrics_disabled_config(config_json.to_string());
+}

toolkit/components/glean/tests/xpcshell/test_GleanServerKnobs.js

@@ -0,0 +1,174 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_setup(function test_setup() {
+  // Give FOG a temp profile to init within.
+  do_get_profile();
+
+  // We need to initialize it once, otherwise operations will be stuck in
+  // the pre-init queue.
+  Services.fog.initializeFOG();
+});
+
+add_task(function test_fog_metrics_disabled_remotely() {
+  // Set a cheesy string in the test metric. This should record because the
+  // metric has `disabled: false` by default.
+  const str1 = "a cheesy string!";
+  Glean.testOnly.cheesyString.set(str1);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Create and set a feature configuration that disables the test metric.
+  const feature_config = {
+    "test_only.cheesy_string": true,
+  };
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set another cheesy string in the test metric. This should not
+  // record because of the override to the metric's default value in the
+  // feature configuration.
+  const str2 = "another cheesy string!";
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Clear the configuration so it doesn't interfere with other tests.
+  Services.fog.setMetricsFeatureConfig("{}");
+});
+
+add_task(function test_fog_multiple_metrics_disabled_remotely() {
+  // Set some test metrics. This should record because the metrics are
+  // `disabled: false` by default.
+  const str1 = "yet another a cheesy string!";
+  Glean.testOnly.cheesyString.set(str1);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+  const qty1 = 42;
+  Glean.testOnly.meaningOfLife.set(qty1);
+  Assert.equal(qty1, Glean.testOnly.meaningOfLife.testGetValue("test-ping"));
+
+  // Create and set a feature configuration that disables multiple test
+  // metrics.
+  var feature_config = {
+    "test_only.cheesy_string": true,
+    "test_only.meaning_of_life": true,
+  };
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set the metrics again. This should not record because of the
+  // override to the metrics' default value in the feature configuration.
+  const str2 = "another cheesy string v2!";
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+  const qty2 = 52;
+  Glean.testOnly.meaningOfLife.set(qty2);
+  Assert.equal(qty1, Glean.testOnly.meaningOfLife.testGetValue("test-ping"));
+
+  // Change the feature configuration to re-enable the `cheesy_string` metric.
+  feature_config = {
+    "test_only.cheesy_string": false,
+    "test_only.meaning_of_life": true,
+  };
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set the metrics again. This should only record `cheesy_string`
+  // because of the most recent feature configuration.
+  const str3 = "another cheesy string v3!";
+  Glean.testOnly.cheesyString.set(str3);
+  Assert.equal(str3, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+  const qty3 = 62;
+  Glean.testOnly.meaningOfLife.set(qty3);
+  Assert.equal(qty1, Glean.testOnly.meaningOfLife.testGetValue("test-ping"));
+
+  // Clear the configuration so it doesn't interfere with other tests. This
+  // is done by passing in an empty dictionary.
+  Services.fog.setMetricsFeatureConfig("{}");
+
+  // Set some final metrics. This should record in both metrics because they
+  // are both `disabled: false` by default.
+  const str4 = "another a cheesy string v4";
+  Glean.testOnly.cheesyString.set(str4);
+  Assert.equal(str4, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+  const qty4 = 72;
+  Glean.testOnly.meaningOfLife.set(qty4);
+  Assert.equal(qty4, Glean.testOnly.meaningOfLife.testGetValue("test-ping"));
+});
+
+add_task(function test_fog_metrics_feature_config_api_handles_null_values() {
+  // Set a cheesy string in the test metric. This should record because the
+  // metric has `disabled: false` by default.
+  const str1 = "a cheesy string!";
+  Glean.testOnly.cheesyString.set(str1);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Create and set a feature configuration that disables the test metric.
+  const feature_config = {
+    "test_only.cheesy_string": true,
+  };
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set another cheesy string in the test metric. This should not
+  // record because of the override to the metric's default value in the
+  // feature configuration.
+  const str2 = "another cheesy string v2";
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Set the configuration to `null`.
+  Services.fog.setMetricsFeatureConfig(null);
+
+  // Attempt to set another cheesy string in the test metric. This should now
+  // record because `null` should clear the configuration.
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str2, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Reset back to the feature configuration that disables the test metric.
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set another cheesy string in the test metric. This should not
+  // record because of the override to the metric's default value in the
+  // feature configuration.
+  Glean.testOnly.cheesyString.set(str1);
+  Assert.equal(str2, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Set the configuration to `""` to replicate getting an empty string from
+  // Nimbus.
+  Services.fog.setMetricsFeatureConfig("");
+
+  // Attempt to set another cheesy string in the test metric. This should now
+  // record again because `""` should be clear the configuration.
+  const str3 = "another cheesy string v3";
+  Glean.testOnly.cheesyString.set(str3);
+  Assert.equal(str3, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+});
+
+add_task(function test_fog_metrics_disabled_reset_fog_behavior() {
+  // Set a cheesy string in the test metric. This should record because the
+  // metric has `disabled: false` by default.
+  const str1 = "a cheesy string!";
+  Glean.testOnly.cheesyString.set(str1);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Create and set a feature configuration that disables the test metric.
+  const feature_config = {
+    "test_only.cheesy_string": true,
+  };
+  Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config));
+
+  // Attempt to set another cheesy string in the test metric. This should not
+  // record because of the override to the metric's default value in the
+  // feature configuration.
+  const str2 = "another cheesy string!";
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Now reset FOG to ensure that the feature configuration is also reset.
+  Services.fog.testResetFOG();
+
+  // Attempt to set the string again in the test metric. This should now
+  // record normally because we reset FOG.
+  Glean.testOnly.cheesyString.set(str2);
+  Assert.equal(str2, Glean.testOnly.cheesyString.testGetValue("test-ping"));
+
+  // Clear the configuration so it doesn't interfere with other tests.
+  Services.fog.setMetricsFeatureConfig("{}");
+});

toolkit/components/glean/tests/xpcshell/xpcshell.ini

@@ -22,6 +22,9 @@ run-sequentially = very high failure rate in parallel
 [test_GleanExperiments.js]
 skip-if = os == "android" # FOG isn't responsible for experiment annotations on Android
 
+[test_GleanServerKnobs.js]
+skip-if = os == "android" # Server Knobs on mobile will be handled by the specific app
+
 [test_JOG.js]
 
 [test_JOGIPC.js]

toolkit/components/glean/xpcom/FOG.cpp

@@ -263,6 +263,19 @@ FOG::TestGetExperimentData(const nsACString& aExperimentId, JSContext* aCx,
 #endif
 }
 
+NS_IMETHODIMP
+FOG::SetMetricsFeatureConfig(const nsACString& aJsonConfig) {
+#ifdef MOZ_GLEAN_ANDROID
+  NS_WARNING(
+      "Don't set metric feature configs from Gecko in Android. Ignoring.");
+  return NS_OK;
+#else
+  MOZ_ASSERT(XRE_IsParentProcess());
+  glean::impl::fog_set_metrics_feature_config(&aJsonConfig);
+  return NS_OK;
+#endif
+}
+
 NS_IMETHODIMP
 FOG::TestFlushAllChildren(JSContext* aCx, mozilla::dom::Promise** aOutPromise) {
   MOZ_ASSERT(XRE_IsParentProcess());

toolkit/components/glean/xpcom/nsIFOG.idl

@@ -102,6 +102,19 @@ interface nsIFOG : nsISupports
   [implicit_jscontext]
   jsval testGetExperimentData(in ACString aExperimentId);
 
+  /**
+   * Set remote-configuration for metrics' disabled property.
+   *
+   * See [`glean_core::Glean::set_metrics_disabled_config`]
+   *
+   * Logs on error, but does not throw.
+   *
+   * @param aJsonConfig - The stringified JSON object in the form
+   *        {metric_base_identifier: boolean,}
+   *        which may contain multiple metric object entries.
+   */
+  void setMetricsFeatureConfig(in ACString aJsonConfig);
+
   /**
    * ** Test-only Method **
    *

toolkit/components/nimbus/FeatureManifest.yaml

@@ -711,6 +711,10 @@ glean:
       type: "boolean"
       fallbackPref: "browser.newtabpage.ping.enabled"
       description: "Whether to submit the 'newtab' ping"
+    metricsDisabled:
+      type: json
+      description: >-
+        "A map of metric base-identifiers to booleans representing the state of the 'disabled' flag for that metric"
 
 majorRelease2022:
   description: Major Release 2022