Bug 1671468 - Dispatcher and IPC support for String List metrics r=janerik

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

toolkit/components/glean/api/src/ipc.rs

@@ -24,6 +24,7 @@ pub struct IPCPayload {
     pub counters: HashMap<MetricId, i32>,
     pub events: HashMap<MetricId, Vec<(Instant, Option<HashMap<i32, String>>)>>,
     pub memory_samples: HashMap<MetricId, Vec<u64>>,
+    pub string_lists: HashMap<MetricId, Vec<String>>,
 }
 
 /// Uniquely identifies a single metric within its metric type.

toolkit/components/glean/api/src/private/string_list.rs

@@ -2,18 +2,34 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+use std::sync::Arc;
+
 use super::CommonMetricData;
 
+use crate::dispatcher;
+use crate::ipc::{need_ipc, with_ipc_payload, MetricId};
+
 /// A string list metric.
 ///
 /// This allows appending a string value with arbitrary content to a list.
+#[derive(Debug)]
+pub enum StringListMetric {
+    Parent(Arc<StringListMetricImpl>),
+    Child(StringListMetricIpc),
+}
 #[derive(Clone, Debug)]
-pub struct StringListMetric(glean_core::metrics::StringListMetric);
+pub struct StringListMetricImpl(glean_core::metrics::StringListMetric);
+#[derive(Debug)]
+pub struct StringListMetricIpc(MetricId);
 
 impl StringListMetric {
     /// Create a new string list metric.
     pub fn new(meta: CommonMetricData) -> Self {
-        Self(glean_core::metrics::StringListMetric::new(meta))
+        if need_ipc() {
+            StringListMetric::Child(StringListMetricIpc(MetricId::new(meta)))
+        } else {
+            StringListMetric::Parent(Arc::new(StringListMetricImpl::new(meta)))
+        }
     }
 
     /// Add a new string to the list.
@@ -27,7 +43,24 @@ impl StringListMetric {
     /// Truncates the value if it is longer than `MAX_STRING_LENGTH` bytes and logs an error.
     /// See [String list metric limits](https://mozilla.github.io/glean/book/user/metrics/string_list.html#limits).
     pub fn add<S: Into<String>>(&self, value: S) {
-        crate::with_glean(move |glean| self.0.add(glean, value))
+        match self {
+            StringListMetric::Parent(p) => {
+                let metric = Arc::clone(&p);
+                let value = value.into();
+                dispatcher::launch(move || metric.add(value));
+            }
+            StringListMetric::Child(c) => {
+                with_ipc_payload(move |payload| {
+                    if let Some(v) = payload.string_lists.get_mut(&c.0) {
+                        v.push(value.into());
+                    } else {
+                        let mut v = vec![];
+                        v.push(value.into());
+                        payload.string_lists.insert(c.0.clone(), v);
+                    }
+                });
+            }
+        }
     }
 
     /// Set to a specific list of strings.
@@ -42,7 +75,19 @@ impl StringListMetric {
     /// Truncates the list if it is longer than `MAX_LIST_LENGTH` and logs an error.
     /// Truncates any value in the list if it is longer than `MAX_STRING_LENGTH` and logs an error.
     pub fn set(&self, value: Vec<String>) {
-        crate::with_glean(move |glean| self.0.set(glean, value))
+        match self {
+            StringListMetric::Parent(p) => {
+                let metric = Arc::clone(&p);
+                dispatcher::launch(move || metric.set(value));
+            }
+            StringListMetric::Child(_c) => {
+                log::error!(
+                    "Unable to set string list metric {:?} in non-main process. Ignoring.",
+                    self
+                );
+                // TODO: Record an error.
+            }
+        }
     }
 
     /// **Test-only API.**
@@ -57,6 +102,33 @@ impl StringListMetric {
     /// ## Return value
     ///
     /// Returns the stored value or `None` if nothing stored.
+    pub fn test_get_value(&self, storage_name: &str) -> Option<Vec<String>> {
+        match self {
+            StringListMetric::Parent(p) => {
+                dispatcher::block_on_queue();
+                p.test_get_value(storage_name)
+            }
+            StringListMetric::Child(_c) => panic!(
+                "Cannot get test value for {:?} in non-parent process!",
+                self
+            ),
+        }
+    }
+}
+
+impl StringListMetricImpl {
+    pub fn new(meta: CommonMetricData) -> Self {
+        Self(glean_core::metrics::StringListMetric::new(meta))
+    }
+
+    pub fn add<S: Into<String>>(&self, value: S) {
+        crate::with_glean(move |glean| self.0.add(glean, value))
+    }
+
+    pub fn set(&self, value: Vec<String>) {
+        crate::with_glean(move |glean| self.0.set(glean, value))
+    }
+
     pub fn test_get_value(&self, storage_name: &str) -> Option<Vec<String>> {
         crate::with_glean(move |glean| self.0.test_get_value(glean, storage_name))
     }

toolkit/components/glean/api/tests/string_list.rs

@@ -0,0 +1,70 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+mod common;
+use common::*;
+
+use fog::ipc;
+use fog::private::{CommonMetricData, Lifetime, StringListMetric};
+
+#[test]
+fn sets_string_list_value() {
+    let _lock = lock_test();
+    let _t = setup_glean(None);
+    let store_names: Vec<String> = vec!["store1".into()];
+
+    let metric = StringListMetric::new(CommonMetricData {
+        name: "string_list_metric".into(),
+        category: "telemetry".into(),
+        send_in_pings: store_names.clone(),
+        disabled: false,
+        lifetime: Lifetime::Ping,
+        ..Default::default()
+    });
+
+    metric.set(vec!["test_string_value".to_string()]);
+    metric.add("another test value");
+
+    assert_eq!(
+        vec!["test_string_value", "another test value"],
+        metric.test_get_value("store1").unwrap()
+    );
+}
+
+#[test]
+fn string_list_ipc() {
+    // StringListMetric supports IPC only for `add`, not `set`.
+    let _lock = lock_test();
+    let _t = setup_glean(None);
+
+    let store_names: Vec<String> = vec!["store1".into()];
+    let meta = CommonMetricData {
+        name: "string metric".into(),
+        category: "ipc".into(),
+        send_in_pings: store_names.clone(),
+        disabled: false,
+        lifetime: Lifetime::Ping,
+        ..Default::default()
+    };
+
+    {
+        let _raii = ipc::test_set_need_ipc(true);
+        let child_metric = StringListMetric::new(meta.clone());
+
+        // Recording APIs do not panic, even when they don't work.
+        child_metric.set(vec!["not gonna be set".to_string()]);
+
+        child_metric.add("child_value");
+        assert!(ipc::take_buf().unwrap().len() > 0);
+
+        // Test APIs are allowed to panic, though.
+        let result = std::panic::catch_unwind(move || {
+            child_metric.test_get_value("store1");
+        });
+        assert!(result.is_err());
+    }
+
+    // TODO: implement replay. See bug 1646165.
+    // Then perform the replay and assert we have the values from both "processes".
+}