Bug 1672706 - Take the metric ID for a UUID metric r=chutten

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

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

@@ -6,12 +6,9 @@ use std::sync::Arc;
 
 use uuid::Uuid;
 
-use super::CommonMetricData;
+use super::{CommonMetricData, MetricId};
 
-use crate::{
-    dispatcher,
-    ipc::{need_ipc, MetricId},
-};
+use crate::{dispatcher, ipc::need_ipc};
 
 /// A UUID metric.
 ///
@@ -25,18 +22,26 @@ pub enum UuidMetric {
 #[derive(Clone, Debug)]
 pub struct UuidMetricImpl(pub(crate) glean_core::metrics::UuidMetric);
 #[derive(Debug)]
-pub struct UuidMetricIpc(MetricId);
+pub struct UuidMetricIpc;
 
 impl UuidMetric {
     /// Create a new UUID metric.
-    pub fn new(meta: CommonMetricData) -> Self {
+    pub fn new(_id: MetricId, meta: CommonMetricData) -> Self {
         if need_ipc() {
-            UuidMetric::Child(UuidMetricIpc(MetricId::new(meta)))
+            UuidMetric::Child(UuidMetricIpc)
         } else {
             UuidMetric::Parent(Arc::new(UuidMetricImpl::new(meta)))
         }
     }
 
+    #[cfg(test)]
+    pub(crate) fn child_metric(&self) -> Self {
+        match self {
+            UuidMetric::Parent(_) => UuidMetric::Child(UuidMetricIpc),
+            UuidMetric::Child(_) => panic!("Can't get a child metric from a child metric"),
+        }
+    }
+
     /// Set to the specified value.
     ///
     /// ## Arguments
@@ -120,3 +125,54 @@ impl UuidMetricImpl {
         crate::with_glean(move |glean| self.0.test_get_value(glean, storage_name))
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::{common_test::*, ipc, metrics};
+
+    #[test]
+    fn sets_uuid_value() {
+        let _lock = lock_test();
+
+        let metric = &metrics::test_only_ipc::a_uuid;
+        let expected = Uuid::new_v4();
+        metric.set(expected.clone());
+
+        assert_eq!(expected, metric.test_get_value("store1").unwrap());
+    }
+
+    #[test]
+    fn uuid_ipc() {
+        // UuidMetric doesn't support IPC.
+        let _lock = lock_test();
+
+        let parent_metric = &metrics::test_only_ipc::a_uuid;
+        let expected = Uuid::new_v4();
+        parent_metric.set(expected.clone());
+
+        {
+            let child_metric = parent_metric.child_metric();
+
+            // Instrumentation calls do not panic.
+            child_metric.set(Uuid::new_v4());
+
+            // (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());
+        }
+
+        assert!(ipc::replay_from_buf(&ipc::take_buf().unwrap()).is_ok());
+
+        assert_eq!(
+            expected,
+            parent_metric.test_get_value("store1").unwrap(),
+            "UUID metrics should only work in the parent process"
+        );
+    }
+}

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

@@ -1,61 +0,0 @@
-// 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/.
-
-use uuid::Uuid;
-
-mod common;
-use common::*;
-
-use fog::ipc;
-use fog::private::{CommonMetricData, Lifetime, UuidMetric};
-
-#[test]
-fn sets_uuid_value() {
-    let _lock = lock_test();
-    let _t = setup_glean(None);
-    let store_names: Vec<String> = vec!["store1".into()];
-
-    let metric = UuidMetric::new(CommonMetricData {
-        name: "uuid_metric".into(),
-        category: "telemetry".into(),
-        send_in_pings: store_names.clone(),
-        disabled: false,
-        lifetime: Lifetime::Ping,
-        ..Default::default()
-    });
-
-    let expected = Uuid::new_v4();
-    metric.set(expected.clone());
-
-    assert_eq!(expected, metric.test_get_value("store1").unwrap());
-}
-
-#[test]
-fn uuid_ipc() {
-    // UuidMetric doesn't support IPC.
-    let _lock = lock_test();
-    let _t = setup_glean(None);
-    let store_names: Vec<String> = vec!["store1".into()];
-    let _raii = ipc::test_set_need_ipc(true);
-    let child_metric = UuidMetric::new(CommonMetricData {
-        name: "uuid_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(Uuid::new_v4());
-
-    // (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/test_metrics.yaml

@@ -197,3 +197,21 @@ test_only.ipc:
       - store1
     no_lint:
       - COMMON_PREFIX
+  a_uuid:
+    type: uuid
+    description: |
+      This is a test-only metric.
+      Just recording some unique identifiers.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1646165
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1646165#c1
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    send_in_pings:
+      - store1
+    no_lint:
+      - COMMON_PREFIX