Bug 1827399 - Update Glean to v52.5.0 r=janerik,supply-chain-reviewers

Depends on D174979

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

build.gradle

@@ -36,7 +36,7 @@ allprojects {
         topsrcdir = gradle.mozconfig.topsrcdir
         topobjdir = gradle.mozconfig.topobjdir
 
-        gleanVersion = "52.4.2"
+        gleanVersion = "52.5.0"
         if (gleanVersion != getRustVersionFor("glean")) {
           throw new StopExecutionException("Mismatched Glean version, expected: ${gleanVersion}," +
               " found ${getRustVersionFor("glean")}")

gfx/wr/webrender/Cargo.toml

@@ -52,7 +52,7 @@ svg_fmt = "0.4"
 tracy-rs = "0.1.2"
 derive_more = { version = "0.99", default-features = false, features = ["add_assign"] }
 etagere = "0.2.6"
-glean = "52.4.2"
+glean = "52.5.0"
 firefox-on-glean = { version = "0.1.0", optional = true }
 swgl = { path = "../swgl", optional = true }
 topological-sort = "0.1"

gfx/wr/wr_glyph_rasterizer/Cargo.toml

@@ -25,7 +25,7 @@ tracy-rs = "0.1.2"
 log = "0.4"
 lazy_static = "1"
 fxhash = "0.2.1"
-glean = { version = "52.4.2", optional = true }
+glean = { version = "52.5.0", optional = true }
 firefox-on-glean = { version = "0.1.0", optional = true }
 serde = { optional = true, version = "1.0", features = ["serde_derive"] }
 

python/sites/mach.txt

@@ -137,7 +137,7 @@ pth:xpcom/geckoprocesstypes_generator
 pth:xpcom/idl-parser
 # glean-sdk may not be installable if a wheel isn't available
 # and it has to be built from source.
-pypi-optional:glean-sdk==52.4.2:telemetry will not be collected
+pypi-optional:glean-sdk==52.5.0:telemetry will not be collected
 # Mach gracefully handles the case where `psutil` is unavailable.
 # We aren't (yet) able to pin packages in automation, so we have to
 # support down to the oldest locally-installed version (5.4.2).

supply-chain/imports.lock

@@ -9,15 +9,15 @@ user-login = "martinthomson"
 user-name = "Martin Thomson"
 
 [[publisher.glean]]
-version = "52.4.2"
-when = "2023-03-15"
+version = "52.5.0"
+when = "2023-04-11"
 user-id = 48
 user-login = "badboy"
 user-name = "Jan-Erik Rediger"
 
 [[publisher.glean-core]]
-version = "52.4.2"
-when = "2023-03-15"
+version = "52.5.0"
+when = "2023-04-11"
 user-id = 48
 user-login = "badboy"
 user-name = "Jan-Erik Rediger"

third_party/rust/glean-core/Cargo.toml

@@ -13,7 +13,7 @@
 edition = "2021"
 rust-version = "1.62"
 name = "glean-core"
-version = "52.4.2"
+version = "52.5.0"
 authors = [
     "Jan-Erik Rediger <[email protected]>",
     "The Glean Team <[email protected]>",

third_party/rust/glean-core/src/core/mod.rs

@@ -12,7 +12,7 @@ use crate::event_database::EventDatabase;
 use crate::internal_metrics::{AdditionalMetrics, CoreMetrics, DatabaseMetrics};
 use crate::internal_pings::InternalPings;
 use crate::metrics::{
-    self, ExperimentMetric, Metric, MetricType, MetricsDisabledConfig, PingType, RecordedExperiment,
+    self, ExperimentMetric, Metric, MetricType, MetricsEnabledConfig, PingType, RecordedExperiment,
 };
 use crate::ping::PingMaker;
 use crate::storage::{StorageManager, INTERNAL_STORAGE};
@@ -153,7 +153,7 @@ pub struct Glean {
     pub(crate) app_build: String,
     pub(crate) schedule_metrics_pings: bool,
     pub(crate) remote_settings_epoch: AtomicU8,
-    pub(crate) remote_settings_metrics_config: Arc<Mutex<MetricsDisabledConfig>>,
+    pub(crate) remote_settings_metrics_config: Arc<Mutex<MetricsEnabledConfig>>,
 }
 
 impl Glean {
@@ -207,7 +207,7 @@ impl Glean {
             // Subprocess doesn't use "metrics" pings so has no need for a scheduler.
             schedule_metrics_pings: false,
             remote_settings_epoch: AtomicU8::new(0),
-            remote_settings_metrics_config: Arc::new(Mutex::new(MetricsDisabledConfig::new())),
+            remote_settings_metrics_config: Arc::new(Mutex::new(MetricsEnabledConfig::new())),
         };
 
         // Ensuring these pings are registered.
@@ -527,6 +527,7 @@ impl Glean {
     }
 
     /// Gets a handle to the database.
+    #[track_caller] // If this fails we're interested in the caller.
     pub fn storage(&self) -> &Database {
         self.data_store.as_ref().expect("No database found")
     }
@@ -702,14 +703,14 @@ impl Glean {
         metric.test_get_value(self)
     }
 
-    /// Set configuration for metrics' disabled property, typically from a remote_settings experiment
-    /// or rollout
+    /// Set configuration to override the default metric enabled/disabled state, typically from a
+    /// remote_settings experiment or rollout
     ///
     /// # Arguments
     ///
-    /// * `json` - The stringified JSON representation of a `MetricsDisabledConfig` object
-    pub fn set_metrics_disabled_config(&self, cfg: MetricsDisabledConfig) {
-        // Set the current MetricsDisabledConfig, keeping the lock until the epoch is
+    /// * `json` - The stringified JSON representation of a `MetricsEnabledConfig` object
+    pub fn set_metrics_enabled_config(&self, cfg: MetricsEnabledConfig) {
+        // Set the current MetricsEnabledConfig, keeping the lock until the epoch is
         // updated to prevent against reading a "new" config but an "old" epoch
         let mut lock = self.remote_settings_metrics_config.lock().unwrap();
         *lock = cfg;

third_party/rust/glean-core/src/database/mod.rs

@@ -52,15 +52,12 @@ pub fn rkv_new(path: &Path) -> std::result::Result<Rkv, rkv::StoreError> {
             // Now try again, we only handle that error once.
             Rkv::new::<rkv::backend::SafeMode>(path)
         }
-        // This code is currently disabled but intended to be turned on in the
-        // near future. Please reference this bug for more details:
-        // https://bugzilla.mozilla.org/show_bug.cgi?id=1820792#c2
-        // Err(rkv::StoreError::DatabaseCorrupted) => {
-        //     let safebin = path.join("data.safe.bin");
-        //     fs::remove_file(safebin).map_err(|_| rkv::StoreError::DatabaseCorrupted)?;
-        //     // Try again, only allowing the error once.
-        //     Rkv::new::<rkv::backend::SafeMode>(path)
-        // }
+        Err(rkv::StoreError::DatabaseCorrupted) => {
+            let safebin = path.join("data.safe.bin");
+            fs::remove_file(safebin).map_err(|_| rkv::StoreError::DatabaseCorrupted)?;
+            // Try again, only allowing the error once.
+            Rkv::new::<rkv::backend::SafeMode>(path)
+        }
         other => other,
     }
 }

third_party/rust/glean-core/src/event_database/mod.rs

@@ -258,16 +258,21 @@ impl EventDatabase {
     ///   monotonically increasing timer (this value is obtained on the
     ///   platform-specific side).
     /// * `extra` - Extra data values, mapping strings to strings.
+    ///
+    /// ## Returns
+    ///
+    /// `true` if a ping was submitted and should be uploaded.
+    /// `false` otherwise.
     pub fn record(
         &self,
         glean: &Glean,
         meta: &CommonMetricDataInternal,
         timestamp: u64,
         extra: Option<HashMap<String, String>>,
-    ) {
+    ) -> bool {
         // If upload is disabled we don't want to record.
         if !glean.is_upload_enabled() {
-            return;
+            return false;
         }
 
         let mut submit_max_capacity_event_ping = false;
@@ -303,6 +308,9 @@ impl EventDatabase {
         }
         if submit_max_capacity_event_ping {
             glean.submit_ping_by_name("events", Some("max_capacity"));
+            true
+        } else {
+            false
         }
     }
 

third_party/rust/glean-core/src/glean.udl

@@ -29,6 +29,9 @@ namespace glean {
     void glean_set_experiment_inactive(string experiment_id);
     RecordedExperiment? glean_test_get_experiment_data(string experiment_id);
 
+    // Server Knobs API
+    void glean_set_metrics_enabled_config(string json);
+
     boolean glean_set_debug_view_tag(string tag);
     boolean glean_set_source_tags(sequence<string> tags);
     void glean_set_log_pings(boolean value);

third_party/rust/glean-core/src/lib.rs

@@ -28,7 +28,7 @@ use crossbeam_channel::unbounded;
 use once_cell::sync::{Lazy, OnceCell};
 use uuid::Uuid;
 
-use metrics::MetricsDisabledConfig;
+use metrics::MetricsEnabledConfig;
 
 mod common_metric_data;
 mod core;
@@ -170,6 +170,7 @@ static STATE: OnceCell<Mutex<State>> = OnceCell::new();
 /// Get a reference to the global state object.
 ///
 /// Panics if no global state object was set.
+#[track_caller] // If this fails we're interested in the caller.
 fn global_state() -> &'static Mutex<State> {
     STATE.get().unwrap()
 }
@@ -772,13 +773,14 @@ pub fn glean_test_get_experiment_data(experiment_id: String) -> Option<RecordedE
     core::with_glean(|glean| glean.test_get_experiment_data(experiment_id.to_owned()))
 }
 
-/// Sets a remote configuration for the metrics' disabled property
+/// Sets a remote configuration to override metrics' default enabled/disabled
+/// state
 ///
-/// See [`core::Glean::set_metrics_disabled_config`].
-pub fn glean_set_metrics_disabled_config(json: String) {
-    match MetricsDisabledConfig::try_from(json) {
+/// See [`core::Glean::set_metrics_enabled_config`].
+pub fn glean_set_metrics_enabled_config(json: String) {
+    match MetricsEnabledConfig::try_from(json) {
         Ok(cfg) => launch_with_glean(|glean| {
-            glean.set_metrics_disabled_config(cfg);
+            glean.set_metrics_enabled_config(cfg);
         }),
         Err(e) => {
             log::error!("Error setting metrics feature config: {:?}", e);

third_party/rust/glean-core/src/lib_unit_tests.rs

@@ -852,15 +852,15 @@ fn test_set_metrics_disabled() {
     );
 
     // 2. Set a configuration to disable the metrics
-    let mut metrics_disabled_config = json!(
+    let mut metrics_enabled_config = json!(
         {
-            "category.string_metric": true,
-            "category.labeled_string_metric": true,
+            "category.string_metric": false,
+            "category.labeled_string_metric": false,
         }
     )
     .to_string();
-    glean.set_metrics_disabled_config(
-        MetricsDisabledConfig::try_from(metrics_disabled_config).unwrap(),
+    glean.set_metrics_enabled_config(
+        MetricsEnabledConfig::try_from(metrics_enabled_config).unwrap(),
     );
 
     // 3. Since the metrics were disabled, setting a new value will be ignored
@@ -883,9 +883,9 @@ fn test_set_metrics_disabled() {
     );
 
     // 4. Set a new configuration where the metrics are enabled
-    metrics_disabled_config = json!({}).to_string();
-    glean.set_metrics_disabled_config(
-        MetricsDisabledConfig::try_from(metrics_disabled_config).unwrap(),
+    metrics_enabled_config = json!({}).to_string();
+    glean.set_metrics_enabled_config(
+        MetricsEnabledConfig::try_from(metrics_enabled_config).unwrap(),
     );
 
     // 5. Since the metrics are now enabled, setting a new value should work
@@ -917,14 +917,14 @@ fn test_remote_settings_epoch() {
     assert_eq!(0u8, current_epoch, "Current epoch must start at 0");
 
     // 2. Set a configuration which will trigger incrementing the epoch
-    let metrics_disabled_config = json!(
+    let metrics_enabled_config = json!(
         {
-            "category.string_metric": true
+            "category.string_metric": false
         }
     )
     .to_string();
-    glean.set_metrics_disabled_config(
-        MetricsDisabledConfig::try_from(metrics_disabled_config).unwrap(),
+    glean.set_metrics_enabled_config(
+        MetricsEnabledConfig::try_from(metrics_enabled_config).unwrap(),
     );
 
     // 3. Ensure the epoch updated
@@ -951,14 +951,14 @@ fn test_remote_settings_epoch_updates_in_metric() {
     );
 
     // 2. Set a configuration to disable the `category.string_metric`
-    let metrics_disabled_config = json!(
+    let metrics_enabled_config = json!(
         {
-            "category.string_metric": true
+            "category.string_metric": false
         }
     )
     .to_string();
-    glean.set_metrics_disabled_config(
-        MetricsDisabledConfig::try_from(metrics_disabled_config).unwrap(),
+    glean.set_metrics_enabled_config(
+        MetricsEnabledConfig::try_from(metrics_enabled_config).unwrap(),
     );
 
     // 3. Ensure the epoch was updated

third_party/rust/glean-core/src/metrics/event.rs

@@ -68,7 +68,15 @@ impl EventMetric {
     ///             If any key is not allowed, an error is reported and no event is recorded.
     pub fn record_with_time(&self, timestamp: u64, extra: HashMap<String, String>) {
         let metric = self.clone();
-        crate::launch_with_glean(move |glean| metric.record_sync(glean, timestamp, extra));
+        crate::launch_with_glean(move |glean| {
+            let sent = metric.record_sync(glean, timestamp, extra);
+            if sent {
+                let state = crate::global_state().lock().unwrap();
+                if let Err(e) = state.callbacks.trigger_upload() {
+                    log::error!("Triggering upload failed. Error: {}", e);
+                }
+            }
+        });
     }
 
     /// Validate that extras are empty or all extra keys are allowed.
@@ -104,20 +112,30 @@ impl EventMetric {
     }
 
     /// Records an event.
+    ///
+    /// ## Returns
+    ///
+    /// `true` if a ping was submitted and should be uploaded.
+    /// `false` otherwise.
     #[doc(hidden)]
-    pub fn record_sync(&self, glean: &Glean, timestamp: u64, extra: HashMap<String, String>) {
+    pub fn record_sync(
+        &self,
+        glean: &Glean,
+        timestamp: u64,
+        extra: HashMap<String, String>,
+    ) -> bool {
         if !self.should_record(glean) {
-            return;
+            return false;
         }
 
         let extra_strings = match self.validate_extra(glean, extra) {
             Ok(extra) => extra,
-            Err(()) => return,
+            Err(()) => return false,
         };
 
         glean
             .event_storage()
-            .record(glean, &self.meta, timestamp, extra_strings);
+            .record(glean, &self.meta, timestamp, extra_strings)
     }
 
     /// **Test-only API (exported for FFI purposes).**