Add option to defer ping lifetime metric persistence

glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt

@@ -140,7 +140,8 @@ open class GleanInternalAPI internal constructor () {
             dataDir = this.gleanDataDir.path,
             packageName = applicationContext.packageName,
             uploadEnabled = uploadEnabled,
-            maxEvents = this.configuration.maxEvents
+            maxEvents = this.configuration.maxEvents,
+            deferCollection = false
         )
 
         // Start the migration from glean-ac, if needed.

glean-core/android/src/main/java/mozilla/telemetry/glean/config/Configuration.kt

@@ -17,12 +17,13 @@ import mozilla.telemetry.glean.net.PingUploader
  * **CAUTION**: This must match _exactly_ the definition on the Rust side.
  *  If this side is changed, the Rust side need to be changed, too.
  */
-@Structure.FieldOrder("dataDir", "packageName", "uploadEnabled", "maxEvents")
+@Structure.FieldOrder("dataDir", "packageName", "uploadEnabled", "maxEvents", "deferCollection")
 internal class FfiConfiguration(
     dataDir: String,
     packageName: String,
     uploadEnabled: Boolean,
-    maxEvents: Int? = null
+    maxEvents: Int? = null,
+    deferCollection: Boolean
 ) : Structure() {
     /**
      * Expose all structure fields as actual fields,
@@ -37,6 +38,8 @@ internal class FfiConfiguration(
     public var uploadEnabled: Byte = uploadEnabled.toByte()
     @JvmField
     public var maxEvents: IntByReference = if (maxEvents == null) IntByReference() else IntByReference(maxEvents)
+    @JvmField
+    public var deferCollection: Byte = deferCollection.toByte()
 
     init {
         // Force UTF-8 string encoding when passing strings over the FFI
@@ -64,6 +67,7 @@ data class Configuration internal constructor(
     val channel: String? = null,
     val userAgent: String = DEFAULT_USER_AGENT,
     val maxEvents: Int? = null,
+    val deferCollection: Boolean = false,
     val logPings: Boolean = DEFAULT_LOG_PINGS,
     // NOTE: since only simple object or strings can be made `const val`s, if the
     // default values for the lines below are ever changed, they are required
@@ -96,6 +100,7 @@ data class Configuration internal constructor(
         serverEndpoint = serverEndpoint,
         userAgent = DEFAULT_USER_AGENT,
         maxEvents = maxEvents,
+        deferCollection = false,
         logPings = DEFAULT_LOG_PINGS,
         httpClient = httpClient,
         pingTag = null,

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: false,
     };
     let mut glean = Glean::new(cfg).unwrap();
     glean.register_ping_type(&PingType::new("baseline", true, false));

glean-core/ffi/src/lib.rs

@@ -101,6 +101,7 @@ pub struct FfiConfiguration<'a> {
     package_name: FfiStr<'a>,
     upload_enabled: u8,
     max_events: Option<&'a i32>,
+    defer_collection: u8,
 }
 
 /// Convert the FFI-compatible configuration object into the proper Rust configuration object.
@@ -112,12 +113,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 = cfg.defer_collection != 0;
 
         Ok(Self {
             upload_enabled,
             data_path,
             application_id,
             max_events,
+            defer_collection,
         })
     }
 }

glean-core/ios/Glean/Utils/Utils.swift

@@ -119,7 +119,8 @@ func withFfiConfiguration<R>(
         data_dir: dataDir,
         package_name: packageName,
         upload_enabled: uploadEnabled.toByte(),
-        max_events: maxEventsPtr
+        max_events: maxEventsPtr,
+        defer_collection: false.toByte()
     )
     return body(cfg)
 }

glean-core/python/glean/_ffi.py

@@ -46,7 +46,11 @@ def _load_header(path: str) -> str:
 
 
 def make_config(
-    data_dir: Path, package_name: str, upload_enabled: bool, max_events: int
+    data_dir: Path,
+    package_name: str,
+    upload_enabled: bool,
+    max_events: int,
+    defer_collection: bool = False,
 ) -> Any:
     """
     Make an `FfiConfiguration` object.
@@ -65,6 +69,7 @@ def make_config(
     cfg.package_name = package_name
     cfg.upload_enabled = upload_enabled
     cfg.max_events = max_events
+    cfg.defer_collection = defer_collection
 
     _global_weakkeydict[cfg] = (data_dir, package_name, max_events)
 

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 {
+            if let Some(ping_lifetime_data) = &self.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,17 @@ 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 {
+            if let Some(ping_lifetime_data) = &self.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 +283,18 @@ impl Database {
             return Ok(());
         }
 
+        // Lifetime::Ping data is not persisted to disk if
+        // Glean has `defer_collection` set to true
+        if lifetime == Lifetime::Ping {
+            if let Some(ping_lifetime_data) = &self.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,27 @@ impl Database {
             return Ok(());
         }
 
+        // Lifetime::Ping data is not persisted to disk if
+        // Glean has `defer_collection` set to true
+        if lifetime == Lifetime::Ping {
+            if let Some(ping_lifetime_data) = &self.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 +424,15 @@ impl Database {
     ///
     /// * This function will **not** panic on database errors.
     pub fn clear_ping_lifetime_storage(&self, storage_name: &str) -> Result<()> {
+        // Lifetime::Ping might have data saved to `ping_lifetime_data`
+        // in case `defer_collection` is set to true
+        if let Some(ping_lifetime_data) = &self.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 +495,18 @@ impl Database {
             return Ok(());
         }
 
+        // Lifetime::Ping data is not persisted to disk if
+        // Glean has `defer_collection` set to true
+        if lifetime == Lifetime::Ping {
+            if let Some(ping_lifetime_data) = &self.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 +537,13 @@ impl Database {
             .write()
             .expect("Can't access app lifetime data as writable")
             .clear();
+
+        if let Some(ping_lifetime_data) = &self.ping_lifetime_data {
+            ping_lifetime_data
+                .write()
+                .expect("Can't access ping lifetime data as writable")
+                .clear();
+        }
     }
 }
 
@@ -454,15 +555,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 +573,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 +610,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 +648,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 +683,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 +759,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 +810,52 @@ 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.");
+
+        let store: SingleStore = unwrap_or!(
+            db.rkv
+                .open_single(Lifetime::Ping.as_str(), StoreOptions::create()),
+            panic!()
+        );
+        let reader = unwrap_or!(db.rkv.read(), panic!());
+        assert!(store
+            .get(&reader, &test_metric_id)
+            .unwrap_or(None)
+            .is_none());
+    }
 }