diff --git a/glean-core/examples/sample.rs b/glean-core/examples/sample.rs index 42827d0082..061f12b85e 100644 --- a/glean-core/examples/sample.rs +++ b/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)); diff --git a/glean-core/ffi/src/lib.rs b/glean-core/ffi/src/lib.rs index 8ad9244865..264c5f5da8 100644 --- a/glean-core/ffi/src/lib.rs +++ b/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, }) } } diff --git a/glean-core/src/database/mod.rs b/glean-core/src/database/mod.rs index 031c1e64ec..7b111be380 100644 --- a/glean-core/src/database/mod.rs +++ b/glean-core/src/database/mod.rs @@ -22,6 +22,12 @@ 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>, + // 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>>, } impl Database { @@ -29,10 +35,15 @@ impl Database { /// /// This opens the underlying rkv store and creates /// the underlying directory structure. - pub fn new(data_path: &str) -> Result { + pub fn new(data_path: &str, defer_collection: bool) -> Result { 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,7 +553,7 @@ mod test { #[test] fn test_panicks_if_fails_dir_creation() { - assert!(Database::new("/!#\"'@#°ç").is_err()); + assert!(Database::new("/!#\"'@#°ç", false).is_err()); } #[test] @@ -462,7 +561,7 @@ mod test { 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."); + } } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index 1c40f48278..83a9c37344 100755 --- a/glean-core/src/lib.rs +++ b/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, + /// Whether Glean should defer ping collection. + pub defer_collection: Option, } /// 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) diff --git a/glean-core/src/lib_unit_tests.rs b/glean-core/src/lib_unit_tests.rs index 01990820c3..0e737d56ee 100644 --- a/glean-core/src/lib_unit_tests.rs +++ b/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(); diff --git a/glean-core/tests/boolean.rs b/glean-core/tests/boolean.rs index 16b4861e2f..99a1115536 100644 --- a/glean-core/tests/boolean.rs +++ b/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, }; { diff --git a/glean-core/tests/common/mod.rs b/glean-core/tests/common/mod.rs index 64c6d4e6ac..9e09471b6b 100644 --- a/glean-core/tests/common/mod.rs +++ b/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(); diff --git a/glean-core/tests/counter.rs b/glean-core/tests/counter.rs index 83f329b4a3..8a79da982d 100644 --- a/glean-core/tests/counter.rs +++ b/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, }; { diff --git a/glean-core/tests/custom_distribution.rs b/glean-core/tests/custom_distribution.rs index 8d1a432803..42ddf37ff9 100644 --- a/glean-core/tests/custom_distribution.rs +++ b/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, }; { diff --git a/glean-core/tests/datetime.rs b/glean-core/tests/datetime.rs index 4c226e1aa1..5840a106a2 100644 --- a/glean-core/tests/datetime.rs +++ b/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, }; { diff --git a/glean-core/tests/labeled.rs b/glean-core/tests/labeled.rs index 810b8f33c6..4b87d98729 100644 --- a/glean-core/tests/labeled.rs +++ b/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(); diff --git a/glean-core/tests/memory_distribution.rs b/glean-core/tests/memory_distribution.rs index 078a1f92cf..6252abbdd7 100644 --- a/glean-core/tests/memory_distribution.rs +++ b/glean-core/tests/memory_distribution.rs @@ -26,6 +26,7 @@ fn serializer_should_correctly_serialize_memory_distribution() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; { diff --git a/glean-core/tests/ping_maker.rs b/glean-core/tests/ping_maker.rs index 6eb9258282..2f5d5c19a9 100644 --- a/glean-core/tests/ping_maker.rs +++ b/glean-core/tests/ping_maker.rs @@ -18,6 +18,7 @@ fn set_up_basic_ping() -> (Glean, PingMaker, PingType, tempfile::TempDir) { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; let mut glean = Glean::new(cfg).unwrap(); let ping_maker = PingMaker::new(); diff --git a/glean-core/tests/quantity.rs b/glean-core/tests/quantity.rs index 0691be5f8d..02c17112a9 100644 --- a/glean-core/tests/quantity.rs +++ b/glean-core/tests/quantity.rs @@ -25,6 +25,7 @@ fn quantity_serializer_should_correctly_serialize_quantities() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; { diff --git a/glean-core/tests/string.rs b/glean-core/tests/string.rs index 1dab093001..ca2a5c8012 100644 --- a/glean-core/tests/string.rs +++ b/glean-core/tests/string.rs @@ -23,6 +23,7 @@ fn string_serializer_should_correctly_serialize_strings() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; { diff --git a/glean-core/tests/string_list.rs b/glean-core/tests/string_list.rs index 5da8e0b990..4c9bd859ca 100644 --- a/glean-core/tests/string_list.rs +++ b/glean-core/tests/string_list.rs @@ -49,6 +49,7 @@ fn stringlist_serializer_should_correctly_serialize_stringlists() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; { diff --git a/glean-core/tests/timespan.rs b/glean-core/tests/timespan.rs index f338412b91..fdd046ab01 100644 --- a/glean-core/tests/timespan.rs +++ b/glean-core/tests/timespan.rs @@ -24,6 +24,7 @@ fn serializer_should_correctly_serialize_timespans() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; let duration = 60; diff --git a/glean-core/tests/timing_distribution.rs b/glean-core/tests/timing_distribution.rs index 1abb3769c2..4146908ae6 100644 --- a/glean-core/tests/timing_distribution.rs +++ b/glean-core/tests/timing_distribution.rs @@ -28,6 +28,7 @@ fn serializer_should_correctly_serialize_timing_distribution() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; { diff --git a/glean-core/tests/uuid.rs b/glean-core/tests/uuid.rs index ccfcb90bb7..2796e4e7de 100644 --- a/glean-core/tests/uuid.rs +++ b/glean-core/tests/uuid.rs @@ -47,6 +47,7 @@ fn uuid_serializer_should_correctly_serialize_uuids() { application_id: GLOBAL_APPLICATION_ID.into(), upload_enabled: true, max_events: None, + defer_collection: None, }; {