Add configuration to add a glean_timestamp extra to all events

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * General
   * Gracefully handle the waiting thread going away during shutdown ([#2503](https://github.com/mozilla/glean/pull/2503))
+  * Experimental: Add configuration to add a `glean_timestamp` extra to all events ([#2513](https://github.com/mozilla/glean/issues/2513))
 * Android
   * Update minimum supported Java byte code generation to 17 ([#2498](https://github.com/mozilla/glean/pull/2498/))
 

glean-core/rlb/src/configuration.rs

@@ -40,6 +40,8 @@ pub struct Configuration {
     pub log_level: Option<LevelFilter>,
     /// The rate pings may be uploaded before they are throttled.
     pub rate_limit: Option<glean_core::PingRateLimit>,
+    /// Whether to add a `glean_timestamp` extra key to all events. Default: `false`.
+    pub enable_event_timestamps: bool,
 }
 
 /// Configuration builder.
@@ -80,6 +82,8 @@ pub struct Builder {
     /// Optional: The internal ping upload rate limit.
     /// Default: `None`
     pub rate_limit: Option<glean_core::PingRateLimit>,
+    /// Whether to add a `glean_timestamp` extra key to all events. Default: `false`.
+    pub enable_event_timestamps: bool,
 }
 
 impl Builder {
@@ -101,6 +105,7 @@ impl Builder {
             trim_data_to_registered_pings: false,
             log_level: None,
             rate_limit: None,
+            enable_event_timestamps: false,
         }
     }
 
@@ -118,6 +123,7 @@ impl Builder {
             trim_data_to_registered_pings: self.trim_data_to_registered_pings,
             log_level: self.log_level,
             rate_limit: self.rate_limit,
+            enable_event_timestamps: self.enable_event_timestamps,
         }
     }
 
@@ -156,4 +162,10 @@ impl Builder {
         self.trim_data_to_registered_pings = value;
         self
     }
+
+    /// Set whether every event should get a wallclock timestamp.
+    pub fn with_event_timestamps(mut self, value: bool) -> Self {
+        self.enable_event_timestamps = value;
+        self
+    }
 }

glean-core/rlb/src/lib.rs

@@ -120,6 +120,7 @@ fn initialize_internal(cfg: Configuration, client_info: ClientInfoMetrics) -> Op
         trim_data_to_registered_pings: cfg.trim_data_to_registered_pings,
         log_level: cfg.log_level,
         rate_limit: cfg.rate_limit,
+        enable_event_timestamps: cfg.enable_event_timestamps,
     };
 
     glean_core::glean_initialize(core_cfg, client_info.into(), callbacks);

glean-core/src/core/mod.rs

@@ -114,6 +114,7 @@ where
 ///     trim_data_to_registered_pings: false,
 ///     log_level: None,
 ///     rate_limit: None,
+///     enable_event_timestamps: false,
 /// };
 /// let mut glean = Glean::new(cfg).unwrap();
 /// let ping = PingType::new("sample", true, false, vec![]);
@@ -172,7 +173,7 @@ impl Glean {
         }
 
         let data_path = Path::new(&cfg.data_path);
-        let event_data_store = EventDatabase::new(data_path)?;
+        let event_data_store = EventDatabase::new(data_path, cfg.enable_event_timestamps)?;
 
         // Create an upload manager with rate limiting of 15 pings every 60 seconds.
         let mut upload_manager = PingUploadManager::new(&cfg.data_path, &cfg.language_binding_name);
@@ -302,6 +303,7 @@ impl Glean {
             trim_data_to_registered_pings: false,
             log_level: None,
             rate_limit: None,
+            enable_event_timestamps: false,
         };
 
         let mut glean = Self::new(cfg).unwrap();

glean-core/src/event_database/mod.rs

@@ -13,7 +13,7 @@ use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::sync::RwLock;
 
-use chrono::{DateTime, FixedOffset};
+use chrono::{DateTime, FixedOffset, Utc};
 
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value as JsonValue};
@@ -98,6 +98,7 @@ pub struct EventDatabase {
     event_stores: RwLock<HashMap<String, Vec<StoredEvent>>>,
     /// A lock to be held when doing operations on the filesystem
     file_lock: RwLock<()>,
+    with_timestamps: bool,
 }
 
 impl EventDatabase {
@@ -107,14 +108,15 @@ impl EventDatabase {
     ///
     /// * `data_path` - The directory to store events in. A new directory
     /// * `events` - will be created inside of this directory.
-    pub fn new(data_path: &Path) -> Result<Self> {
+    pub fn new(data_path: &Path, with_timestamps: bool) -> Result<Self> {
         let path = data_path.join("events");
         create_dir_all(&path)?;
 
         Ok(Self {
             path,
             event_stores: RwLock::new(HashMap::new()),
             file_lock: RwLock::new(()),
+            with_timestamps,
         })
     }
 
@@ -268,13 +270,24 @@ impl EventDatabase {
         glean: &Glean,
         meta: &CommonMetricDataInternal,
         timestamp: u64,
-        extra: Option<HashMap<String, String>>,
+        mut extra: Option<HashMap<String, String>>,
     ) -> bool {
         // If upload is disabled we don't want to record.
         if !glean.is_upload_enabled() {
             return false;
         }
 
+        if self.with_timestamps {
+            let now = Utc::now();
+            // TODO: Is this the precision we want?
+            let ts = now.timestamp_millis() as u64;
+            if extra.is_none() {
+                extra.replace(Default::default());
+            }
+            let map = extra.get_or_insert(Default::default());
+            map.insert("glean_timestamp".to_string(), ts.to_string());
+        }
+
         let mut submit_max_capacity_event_ping = false;
         {
             let mut db = self.event_stores.write().unwrap(); // safe unwrap, only error case is poisoning
@@ -641,7 +654,7 @@ mod test {
         let (glean, t) = new_glean(None);
 
         {
-            let db = EventDatabase::new(t.path()).unwrap();
+            let db = EventDatabase::new(t.path(), false).unwrap();
             db.write_event_to_disk("events", "{\"timestamp\": 500");
             db.write_event_to_disk("events", "{\"timestamp\"");
             db.write_event_to_disk(
@@ -651,7 +664,7 @@ mod test {
         }
 
         {
-            let db = EventDatabase::new(t.path()).unwrap();
+            let db = EventDatabase::new(t.path(), false).unwrap();
             db.load_events_from_disk(&glean, false).unwrap();
             let events = &db.event_stores.read().unwrap()["events"];
             assert_eq!(1, events.len());
@@ -751,7 +764,7 @@ mod test {
     #[test]
     fn doesnt_record_when_upload_is_disabled() {
         let (mut glean, dir) = new_glean(None);
-        let db = EventDatabase::new(dir.path()).unwrap();
+        let db = EventDatabase::new(dir.path(), false).unwrap();
 
         let test_storage = "test-storage";
         let test_category = "category";

glean-core/src/glean.udl

@@ -74,6 +74,7 @@ dictionary InternalConfiguration {
     boolean trim_data_to_registered_pings;
     LevelFilter? log_level;
     PingRateLimit? rate_limit;
+    boolean enable_event_timestamps;
 };
 
 // How to specify the rate pings may be uploaded before they are throttled.

glean-core/src/lib.rs

@@ -129,6 +129,8 @@ pub struct InternalConfiguration {
     pub log_level: Option<LevelFilter>,
     /// The rate at which pings may be uploaded before they are throttled.
     pub rate_limit: Option<PingRateLimit>,
+    /// Whether to add a `glean_timestamp` extra key to all events. Default: `false`.
+    pub enable_event_timestamps: bool,
 }
 
 /// How to specify the rate at which pings may be uploaded before they are throttled.

glean-core/tests/common/mod.rs

@@ -61,6 +61,7 @@ pub fn new_glean(tempdir: Option<tempfile::TempDir>) -> (Glean, tempfile::TempDi
         trim_data_to_registered_pings: false,
         log_level: None,
         rate_limit: None,
+        enable_event_timestamps: false,
     };
     let glean = Glean::new(cfg).unwrap();
 

glean-core/tests/event.rs

@@ -452,3 +452,57 @@ fn event_storage_trimming() {
         assert!(event.get_value(&glean, store_name_2).is_none());
     }
 }
+
+#[test]
+fn with_event_timestamps() {
+    use glean_core::{Glean, InternalConfiguration};
+
+    let dir = tempfile::tempdir().unwrap();
+    let cfg = InternalConfiguration {
+        data_path: dir.path().display().to_string(),
+        application_id: GLOBAL_APPLICATION_ID.into(),
+        language_binding_name: "Rust".into(),
+        upload_enabled: true,
+        max_events: None,
+        delay_ping_lifetime_io: false,
+        app_build: "Unknown".into(),
+        use_core_mps: false,
+        trim_data_to_registered_pings: false,
+        log_level: None,
+        rate_limit: None,
+        enable_event_timestamps: true, // Enabling event timestamps
+    };
+    let glean = Glean::new(cfg).unwrap();
+
+    let store_name = "store-name";
+    let event = EventMetric::new(
+        CommonMetricData {
+            name: "name".into(),
+            category: "category".into(),
+            send_in_pings: vec![store_name.into()],
+            lifetime: Lifetime::Ping,
+            ..Default::default()
+        },
+        vec![],
+    );
+
+    event.record_sync(&glean, get_timestamp_ms(), HashMap::new());
+
+    let json = glean
+        .event_storage()
+        .snapshot_as_json(&glean, store_name, false)
+        .unwrap();
+    assert_eq!(json.as_array().unwrap().len(), 1);
+    assert_eq!(json[0]["category"], "category");
+    assert_eq!(json[0]["name"], "name");
+
+    // We can't properly validate it, but it's a timestamp in a string.
+    // So let's check it's a positive integer.
+    let glean_timestamp = json[0]["extra"]["glean_timestamp"]
+        .as_str()
+        .expect("glean_timestamp should exist");
+    let glean_timestamp: i64 = glean_timestamp
+        .parse()
+        .expect("glean_timestamp should be an integer");
+    assert!(glean_timestamp > 0);
+}

samples/rust/src/main.rs

@@ -39,6 +39,7 @@ fn main() {
         trim_data_to_registered_pings: false,
         log_level: None,
         rate_limit: None,
+        enable_event_timestamps: false,
     };
 
     let client_info = ClientInfoMetrics {