Add option to defer ping lifetime metric persistence

glean-core/examples/sample.rs

@@ -23,6 +23,7 @@ fn main() {
         application_id: "org.mozilla.glean_core.example".into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
     let mut glean = Glean::new(cfg).unwrap();
     glean.register_ping_type(&PingType::new("baseline", true));

glean-core/ffi/src/lib.rs

@@ -112,12 +112,14 @@ impl TryFrom<&FfiConfiguration<'_>> for glean_core::Configuration {
         let application_id = cfg.package_name.to_string_fallible()?;
         let upload_enabled = cfg.upload_enabled != 0;
         let max_events = cfg.max_events.filter(|&&i| i >= 0).map(|m| *m as usize);
+        let defer_collection = Some(false);
 
         Ok(Self {
             upload_enabled,
             data_path,
             application_id,
             max_events,
+            defer_collection,
         })
     }
 }

glean-core/src/database/mod.rs

@@ -22,17 +22,28 @@ pub struct Database {
     // as the application lives: they don't need to be persisted
     // to disk using rkv. Store them in a map.
     app_lifetime_data: RwLock<BTreeMap<String, Metric>>,
+    // If the `defer_collection` Glean config option is `true`,
+    // we will save the 'ping' lifetime data in a map temporarily
+    // so as to persist them to disk using rkv in bulk on shutdown,
+    // or after a given interval, instead of everytime a new metric
+    // is created / updated.
+    ping_lifetime_data: Option<RwLock<BTreeMap<String, Metric>>>,
 }
 
 impl Database {
     /// Initialize the data store.
     ///
     /// This opens the underlying rkv store and creates
     /// the underlying directory structure.
-    pub fn new(data_path: &str) -> Result<Self> {
+    pub fn new(data_path: &str, defer_collection: bool) -> Result<Self> {
         Ok(Self {
             rkv: Self::open_rkv(data_path)?,
             app_lifetime_data: RwLock::new(BTreeMap::new()),
+            ping_lifetime_data: if defer_collection {
+                Some(RwLock::new(BTreeMap::new()))
+            } else {
+                None
+            },
         })
     }
 
@@ -117,6 +128,24 @@ impl Database {
             return;
         }
 
+        // Lifetime::Ping data is not persisted to disk if
+        // Glean has `defer_collection` set to true
+        if lifetime == Lifetime::Ping {
+            let ping_lifetime_data = &self.ping_lifetime_data;
+            if let Some(ping_lifetime_data) = ping_lifetime_data {
+                let data = ping_lifetime_data
+                    .read()
+                    .expect("Can't read ping lifetime data");
+                for (key, value) in data.iter() {
+                    if key.starts_with(&iter_start) {
+                        let key = &key[len..];
+                        transaction_fn(key.as_bytes(), value);
+                    }
+                }
+                return;
+            }
+        }
+
         let store: SingleStore = unwrap_or!(
             self.rkv
                 .open_single(lifetime.as_str(), StoreOptions::create()),
@@ -170,6 +199,18 @@ impl Database {
                 .unwrap_or(false);
         }
 
+        // Lifetime::Ping data is not persisted to disk if
+        // Glean has `defer_collection` set to true
+        if lifetime == Lifetime::Ping {
+            let ping_lifetime_data = &self.ping_lifetime_data;
+            if let Some(ping_lifetime_data) = ping_lifetime_data {
+                return ping_lifetime_data
+                    .read()
+                    .map(|data| data.contains_key(&key))
+                    .unwrap_or(false);
+            }
+        }
+
         let store: SingleStore = unwrap_or!(
             self.rkv
                 .open_single(lifetime.as_str(), StoreOptions::create()),
@@ -243,6 +284,17 @@ impl Database {
             return Ok(());
         }
 
+        if lifetime == Lifetime::Ping {
+            let ping_lifetime_data = &self.ping_lifetime_data;
+            if let Some(ping_lifetime_data) = ping_lifetime_data {
+                let mut data = ping_lifetime_data
+                    .write()
+                    .expect("Can't read ping lifetime data");
+                data.insert(final_key, metric.clone());
+                return Ok(());
+            }
+        }
+
         let encoded = bincode::serialize(&metric).expect("IMPOSSIBLE: Serializing metric failed");
         let value = rkv::Value::Blob(&encoded);
 
@@ -313,6 +365,26 @@ impl Database {
             return Ok(());
         }
 
+        if lifetime == Lifetime::Ping {
+            let ping_lifetime_data = &self.ping_lifetime_data;
+            if let Some(ping_lifetime_data) = ping_lifetime_data {
+                let mut data = ping_lifetime_data
+                    .write()
+                    .expect("Can't access ping lifetime data as writable");
+                let entry = data.entry(final_key);
+                match entry {
+                    Entry::Vacant(entry) => {
+                        entry.insert(transform(None));
+                    }
+                    Entry::Occupied(mut entry) => {
+                        let old_value = entry.get().clone();
+                        entry.insert(transform(Some(old_value)));
+                    }
+                }
+                return Ok(());
+            }
+        }
+
         let store_name = lifetime.as_str();
         let store = self.rkv.open_single(store_name, StoreOptions::create())?;
 
@@ -351,6 +423,14 @@ impl Database {
     ///
     /// * This function will **not** panic on database errors.
     pub fn clear_ping_lifetime_storage(&self, storage_name: &str) -> Result<()> {
+        let ping_lifetime_data = &self.ping_lifetime_data;
+        if let Some(ping_lifetime_data) = ping_lifetime_data {
+            ping_lifetime_data
+                .write()
+                .expect("Can't access ping lifetime data as writable")
+                .clear();
+        }
+
         self.write_with_store(Lifetime::Ping, |mut writer, store| {
             let mut metrics = Vec::new();
             {
@@ -413,6 +493,17 @@ impl Database {
             return Ok(());
         }
 
+        if lifetime == Lifetime::Ping {
+            let ping_lifetime_data = &self.ping_lifetime_data;
+            if let Some(ping_lifetime_data) = ping_lifetime_data {
+                let mut data = ping_lifetime_data
+                    .write()
+                    .expect("Can't access app lifetime data as writable");
+                data.remove(&final_key);
+                return Ok(());
+            }
+        }
+
         self.write_with_store(lifetime, |mut writer, store| {
             store.delete(&mut writer, final_key.clone())?;
             writer.commit()?;
@@ -443,6 +534,14 @@ impl Database {
             .write()
             .expect("Can't access app lifetime data as writable")
             .clear();
+
+        let ping_lifetime_data = &self.ping_lifetime_data;
+        if let Some(ping_lifetime_data) = ping_lifetime_data {
+            ping_lifetime_data
+                .write()
+                .expect("Can't access ping lifetime data as writable")
+                .clear();
+        }
     }
 }
 
@@ -454,15 +553,15 @@ mod test {
 
     #[test]
     fn test_panicks_if_fails_dir_creation() {
-        assert!(Database::new("/!#\"'@#°ç").is_err());
+        assert!(Database::new("/!#\"'@#°ç", false).is_err());
     }
 
     #[test]
     fn test_data_dir_rkv_inits() {
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
 
-        Database::new(&str_dir).unwrap();
+        Database::new(&str_dir, false).unwrap();
 
         assert!(dir.path().exists());
     }
@@ -472,7 +571,9 @@ mod test {
         // Init the database in a temporary directory.
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
-        let db = Database::new(&str_dir).unwrap();
+        let db = Database::new(&str_dir, false).unwrap();
+
+        assert!(db.ping_lifetime_data.is_none());
 
         // Attempt to record a known value.
         let test_value = "test-value";
@@ -507,7 +608,7 @@ mod test {
         // Init the database in a temporary directory.
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
-        let db = Database::new(&str_dir).unwrap();
+        let db = Database::new(&str_dir, false).unwrap();
 
         // Attempt to record a known value.
         let test_value = "test-value";
@@ -545,7 +646,7 @@ mod test {
         // Init the database in a temporary directory.
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
-        let db = Database::new(&str_dir).unwrap();
+        let db = Database::new(&str_dir, false).unwrap();
 
         // Attempt to record a known value.
         let test_value = "test-value";
@@ -580,7 +681,7 @@ mod test {
         // Init the database in a temporary directory.
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
-        let db = Database::new(&str_dir).unwrap();
+        let db = Database::new(&str_dir, false).unwrap();
 
         // Attempt to record a known value for every single lifetime.
         let test_storage = "test-storage";
@@ -656,7 +757,7 @@ mod test {
         // Init the database in a temporary directory.
         let dir = tempdir().unwrap();
         let str_dir = dir.path().display().to_string();
-        let db = Database::new(&str_dir).unwrap();
+        let db = Database::new(&str_dir, false).unwrap();
 
         let test_storage = "test-storage-single-lifetime";
         let metric_id_pattern = "telemetry_test.single_metric";
@@ -707,4 +808,41 @@ mod test {
             );
         }
     }
+
+    #[test]
+    fn test_deferred_ping_collection() {
+        // Init the database in a temporary directory.
+        let dir = tempdir().unwrap();
+        let str_dir = dir.path().display().to_string();
+        let db = Database::new(&str_dir, true).unwrap();
+
+        assert!(db.ping_lifetime_data.is_some());
+
+        // Attempt to record a known value.
+        let test_value = "test-value";
+        let test_storage = "test-storage1";
+        let test_metric_id = "telemetry_test.test_name";
+        db.record_per_lifetime(
+            Lifetime::Ping,
+            test_storage,
+            test_metric_id,
+            &Metric::String(test_value.to_string()),
+        )
+        .unwrap();
+
+        // Verify that the data is correctly recorded.
+        let mut found_metrics = 0;
+        let mut snapshotter = |metric_name: &[u8], metric: &Metric| {
+            found_metrics += 1;
+            let metric_id = String::from_utf8_lossy(metric_name).into_owned();
+            assert_eq!(test_metric_id, metric_id);
+            match metric {
+                Metric::String(s) => assert_eq!(test_value, s),
+                _ => panic!("Unexpected data found"),
+            }
+        };
+
+        db.iter_store_from(Lifetime::Ping, test_storage, None, &mut snapshotter);
+        assert_eq!(1, found_metrics, "We only expect 1 Lifetime.Ping metric.");
+    }
 }

glean-core/src/lib.rs

@@ -65,6 +65,8 @@ pub struct Configuration {
     pub application_id: String,
     /// The maximum number of events to store before sending a ping containing events.
     pub max_events: Option<usize>,
+    /// Whether Glean should defer ping collection.
+    pub defer_collection: Option<bool>,
 }
 
 /// The object holding meta information about a Glean instance.
@@ -81,6 +83,7 @@ pub struct Configuration {
 ///     application_id: "glean.sample.app".into(),
 ///     upload_enabled: true,
 ///     max_events: None,
+///     defer_collection: None,
 /// };
 /// let mut glean = Glean::new(cfg).unwrap();
 /// let ping = PingType::new("sample", true);
@@ -127,7 +130,7 @@ impl Glean {
 
         // Creating the data store creates the necessary path as well.
         // If that fails we bail out and don't initialize further.
-        let data_store = Database::new(&cfg.data_path)?;
+        let data_store = Database::new(&cfg.data_path, cfg.defer_collection.unwrap_or(false))?;
         let event_data_store = EventDatabase::new(&cfg.data_path)?;
 
         let mut glean = Self {
@@ -190,6 +193,7 @@ impl Glean {
             application_id: application_id.into(),
             upload_enabled,
             max_events: None,
+            defer_collection: None,
         };
 
         Self::new(cfg)

glean-core/src/lib_unit_tests.rs

@@ -344,6 +344,7 @@ fn glean_inits_with_migration_when_no_db_dir_exists() {
         application_id: GLOBAL_APPLICATION_ID.to_string(),
         upload_enabled: false,
         max_events: None,
+        defer_collection: None,
     };
 
     let mut ac_seq_numbers = HashMap::new();

glean-core/tests/boolean.rs

@@ -22,6 +22,7 @@ fn boolean_serializer_should_correctly_serialize_boolean() {
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
 
     {

glean-core/tests/common/mod.rs

@@ -54,6 +54,7 @@ pub fn new_glean() -> (Glean, tempfile::TempDir) {
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
     let glean = Glean::new(cfg).unwrap();
 

glean-core/tests/counter.rs

@@ -25,6 +25,7 @@ fn counter_serializer_should_correctly_serialize_counters() {
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
 
     {

glean-core/tests/custom_distribution.rs

@@ -26,6 +26,7 @@ mod linear {
             application_id: GLOBAL_APPLICATION_ID.into(),
             upload_enabled: true,
             max_events: None,
+            defer_collection: None,
         };
 
         {
@@ -245,6 +246,7 @@ mod exponential {
             application_id: GLOBAL_APPLICATION_ID.into(),
             upload_enabled: true,
             max_events: None,
+            defer_collection: None,
         };
 
         {

glean-core/tests/datetime.rs

@@ -24,6 +24,7 @@ fn datetime_serializer_should_correctly_serialize_datetime() {
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
 
     {

glean-core/tests/labeled.rs

@@ -333,6 +333,7 @@ fn seen_labels_get_reloaded_from_disk() {
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
+        defer_collection: None,
     };
 
     let glean = Glean::new(cfg.clone()).unwrap();