Bug 1635255 - Test child-process counter metrics r=janerik

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

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

@@ -8,6 +8,8 @@ use crate::metrics::CommonMetricData;
 use once_cell::sync::Lazy;
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
+#[cfg(not(feature = "with_gecko"))]
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Mutex;
 #[cfg(feature = "with_gecko")]
 use {
@@ -73,9 +75,46 @@ pub fn need_ipc() -> bool {
     PROCESS_TYPE.load(Ordering::Relaxed) != nsIXULRuntime::PROCESS_TYPE_DEFAULT as u32
 }
 
+/// An RAII that, on drop, restores the value used to determine whether FOG
+/// needs IPC. Used in tests.
+/// ```rust,ignore
+/// #[test]
+/// fn test_need_ipc_raii() {
+///     assert!(false == ipc::need_ipc());
+///     {
+///         let _raii = ipc::test_set_need_ipc(true);
+///         assert!(ipc::need_ipc());
+///     }
+///     assert!(false == ipc::need_ipc());
+/// }
+/// ```
+#[cfg(not(feature = "with_gecko"))]
+pub struct TestNeedIpcRAII {
+    prev_value: bool,
+}
+
+#[cfg(not(feature = "with_gecko"))]
+impl Drop for TestNeedIpcRAII {
+    fn drop(&mut self) {
+        TEST_NEED_IPC.store(self.prev_value, Ordering::Relaxed);
+    }
+}
+
+#[cfg(not(feature = "with_gecko"))]
+static TEST_NEED_IPC: AtomicBool = AtomicBool::new(false);
+
+/// Test-only API for telling FOG to use IPC mechanisms even if the test has
+/// only the one process. See TestNeedIpcRAII for an example.
+#[cfg(not(feature = "with_gecko"))]
+pub fn test_set_need_ipc(need_ipc: bool) -> TestNeedIpcRAII {
+    TestNeedIpcRAII {
+        prev_value: TEST_NEED_IPC.swap(need_ipc, Ordering::Relaxed),
+    }
+}
+
 #[cfg(not(feature = "with_gecko"))]
 pub fn need_ipc() -> bool {
-    false
+    TEST_NEED_IPC.load(Ordering::Relaxed)
 }
 
 pub fn take_buf() -> Option<Vec<u8>> {

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

@@ -0,0 +1,82 @@
+// 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 glean::ipc;
+use glean::metrics::{CommonMetricData, CounterMetric, Lifetime};
+
+#[test]
+fn sets_counter_value_parent() {
+    let _lock = lock_test();
+    let _t = setup_glean(None);
+    let store_names: Vec<String> = vec!["store1".into()];
+
+    let metric = CounterMetric::new(CommonMetricData {
+        name: "counter_metric".into(),
+        category: "telemetry".into(),
+        send_in_pings: store_names.clone(),
+        disabled: false,
+        lifetime: Lifetime::Ping,
+        ..Default::default()
+    });
+
+    metric.add(1);
+
+    assert_eq!(1, metric.test_get_value("store1").unwrap());
+}
+
+#[test]
+fn sets_counter_value_child() {
+    let _lock = lock_test();
+    let _t = setup_glean(None);
+
+    let store_names: Vec<String> = vec!["store1".into()];
+    let meta = CommonMetricData {
+        name: "counter metric".into(),
+        category: "ipc".into(),
+        send_in_pings: store_names.clone(),
+        disabled: false,
+        lifetime: Lifetime::Ping,
+        ..Default::default()
+    };
+    assert!(
+        false == ipc::need_ipc(),
+        "Should start the test not needing ipc"
+    );
+    let parent_metric = CounterMetric::new(meta.clone());
+    parent_metric.add(3);
+
+    {
+        // scope for need_ipc RAII
+        let _raii = ipc::test_set_need_ipc(true);
+        let child_metric = CounterMetric::new(meta.clone());
+
+        child_metric.add(42);
+
+        // Need to catch the panic so that our RAIIs drop nicely.
+        let result = std::panic::catch_unwind(move || {
+            child_metric.test_get_value("store1");
+        });
+        assert!(result.is_err());
+
+        ipc::with_ipc_payload(move |payload| {
+            let metric_id = ipc::MetricId::new(meta);
+            assert!(
+                42 == *payload.counters.get(&metric_id).unwrap(),
+                "Stored the correct value in the ipc payload"
+            );
+        });
+    }
+
+    assert!(
+        false == ipc::need_ipc(),
+        "RAII dropped, should not need ipc any more"
+    );
+    assert!(ipc::replay_from_buf(&ipc::take_buf().unwrap()).is_ok());
+
+    // TODO: implement replay. See bug 1646165.
+    // assert!(45 == parent_metric.test_get_value("store1").unwrap(), "Values from the 'processes' should be summed");
+}

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

@@ -5,6 +5,7 @@
 mod common;
 use common::*;
 
+use glean::ipc;
 use glean::metrics::{CommonMetricData, Lifetime, StringMetric};
 
 #[test]
@@ -29,3 +30,31 @@ fn sets_string_value() {
         metric.test_get_value("store1").unwrap()
     );
 }
+
+#[test]
+fn string_ipc() {
+    // StringMetric doesn't support IPC.
+    let _lock = lock_test();
+    let store_names: Vec<String> = vec!["store1".into()];
+    let _raii = ipc::test_set_need_ipc(true);
+    let child_metric = StringMetric::new(CommonMetricData {
+        name: "string metric".into(),
+        category: "ipc".into(),
+        send_in_pings: store_names.clone(),
+        disabled: false,
+        lifetime: Lifetime::Ping,
+        ..Default::default()
+    });
+
+    // Instrumentation calls do not panic.
+    child_metric.set("test_string_value");
+
+    // (They also shouldn't do anything,
+    // but that's not something we can inspect in this test)
+
+    // Need to catch the panic so that our RAIIs drop nicely.
+    let result = std::panic::catch_unwind(move || {
+        child_metric.test_get_value("store1");
+    });
+    assert!(result.is_err());
+}

toolkit/components/glean/gtest/TestFogIPC.cpp

@@ -18,7 +18,7 @@ using mozilla::ipc::ByteBuf;
 TEST(FOG, TestFlushFOGData)
 {
   // A "It doesn't explode" test.
-  std::function<void(ByteBuf &&)> resolver;
+  std::function<void(ByteBuf &&)> resolver = [](ByteBuf&& bufs) {};
   mozilla::glean::FlushFOGData(std::move(resolver));
 }
 

toolkit/components/glean/src/lib.rs

@@ -203,5 +203,9 @@ pub unsafe extern "C" fn fog_give_ipc_buf(buf: *mut u8, buf_len: usize) -> usize
 /// No ownership is transfered to Rust by this method: caller owns the memory at
 /// buf before and after this call.
 pub unsafe extern "C" fn fog_use_ipc_buf(buf: *const u8, buf_len: usize) {
-    glean::ipc::replay_from_buf(std::slice::from_raw_parts(buf, buf_len)).unwrap();
+    let slice = std::slice::from_raw_parts(buf, buf_len);
+    let _res = glean::ipc::replay_from_buf(slice);
+    /*if res.is_err() {
+        // TODO: Record the error.
+    }*/
 }

toolkit/library/gtest/rust/Cargo.toml

@@ -32,6 +32,7 @@ wasm_library_sandboxing = ["gkrust-shared/wasm_library_sandboxing"]
 webgpu = ["gkrust-shared/webgpu"]
 remote_agent = ["gkrust-shared/remote"]
 glean = ["gkrust-shared/glean"]
+glean_with_gecko = ["gkrust-shared/glean_with_gecko"]
 rust_fxa_client = ["gkrust-shared/rust_fxa_client"]
 
 [dependencies]