Bug 1704846 - Add Quantity metric type to FOG (Rust) r=janerik

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

toolkit/components/glean/api/src/ffi/mod.rs

@@ -20,13 +20,13 @@ pub unsafe extern "C" fn fog_counter_add(id: u32, amount: i32) {
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_counter_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(COUNTER_MAP, id, metric, test_has!(metric, storage_name))
+pub unsafe extern "C" fn fog_counter_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(COUNTER_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_counter_test_get_value(id: u32, storage_name: &nsACString) -> i32 {
-    with_metric!(COUNTER_MAP, id, metric, test_get!(metric, storage_name))
+pub unsafe extern "C" fn fog_counter_test_get_value(id: u32, ping_name: &nsACString) -> i32 {
+    with_metric!(COUNTER_MAP, id, metric, test_get!(metric, ping_name))
 }
 
 #[no_mangle]
@@ -55,23 +55,23 @@ pub unsafe extern "C" fn fog_timespan_set_raw(id: u32, duration: u32) {
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_timespan_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(TIMESPAN_MAP, id, metric, test_has!(metric, storage_name))
+pub unsafe extern "C" fn fog_timespan_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(TIMESPAN_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_timespan_test_get_value(id: u32, storage_name: &nsACString) -> u64 {
-    with_metric!(TIMESPAN_MAP, id, metric, test_get!(metric, storage_name))
+pub unsafe extern "C" fn fog_timespan_test_get_value(id: u32, ping_name: &nsACString) -> u64 {
+    with_metric!(TIMESPAN_MAP, id, metric, test_get!(metric, ping_name))
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_boolean_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(BOOLEAN_MAP, id, metric, test_has!(metric, storage_name))
+pub unsafe extern "C" fn fog_boolean_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(BOOLEAN_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fog_boolean_test_get_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(BOOLEAN_MAP, id, metric, test_get!(metric, storage_name))
+pub unsafe extern "C" fn fog_boolean_test_get_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(BOOLEAN_MAP, id, metric, test_get!(metric, ping_name))
 }
 
 #[no_mangle]
@@ -84,17 +84,17 @@ pub extern "C" fn fog_boolean_set(id: u32, value: bool) {
 // generate the rest of the functions, or something.
 
 #[no_mangle]
-pub extern "C" fn fog_string_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(STRING_MAP, id, metric, test_has!(metric, storage_name))
+pub extern "C" fn fog_string_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(STRING_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
 pub extern "C" fn fog_string_test_get_value(
     id: u32,
-    storage_name: &nsACString,
+    ping_name: &nsACString,
     value: &mut nsACString,
 ) {
-    let val = with_metric!(STRING_MAP, id, metric, test_get!(metric, storage_name));
+    let val = with_metric!(STRING_MAP, id, metric, test_get!(metric, ping_name));
     value.assign(&val);
 }
 
@@ -106,17 +106,17 @@ pub extern "C" fn fog_string_set(id: u32, value: &nsACString) {
 // String List Functions:
 
 #[no_mangle]
-pub extern "C" fn fog_string_list_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(STRING_LIST_MAP, id, metric, test_has!(metric, storage_name))
+pub extern "C" fn fog_string_list_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(STRING_LIST_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
 pub extern "C" fn fog_string_list_test_get_value(
     id: u32,
-    storage_name: &nsACString,
+    ping_name: &nsACString,
     value: &mut ThinVec<nsCString>,
 ) {
-    let val = with_metric!(STRING_LIST_MAP, id, metric, test_get!(metric, storage_name));
+    let val = with_metric!(STRING_LIST_MAP, id, metric, test_get!(metric, ping_name));
     for v in val {
         value.push(v.into());
     }
@@ -134,17 +134,13 @@ pub extern "C" fn fog_string_list_set(id: u32, value: &ThinVec<nsCString>) {
 }
 
 #[no_mangle]
-pub extern "C" fn fog_uuid_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(UUID_MAP, id, metric, test_has!(metric, storage_name))
+pub extern "C" fn fog_uuid_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(UUID_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
-pub extern "C" fn fog_uuid_test_get_value(
-    id: u32,
-    storage_name: &nsACString,
-    value: &mut nsACString,
-) {
-    let uuid = with_metric!(UUID_MAP, id, metric, test_get!(metric, storage_name)).to_string();
+pub extern "C" fn fog_uuid_test_get_value(id: u32, ping_name: &nsACString, value: &mut nsACString) {
+    let uuid = with_metric!(UUID_MAP, id, metric, test_get!(metric, ping_name)).to_string();
     value.assign(&uuid);
 }
 
@@ -161,17 +157,17 @@ pub extern "C" fn fog_uuid_generate_and_set(id: u32) {
 }
 
 #[no_mangle]
-pub extern "C" fn fog_datetime_test_has_value(id: u32, storage_name: &nsACString) -> bool {
-    with_metric!(DATETIME_MAP, id, metric, test_has!(metric, storage_name))
+pub extern "C" fn fog_datetime_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(DATETIME_MAP, id, metric, test_has!(metric, ping_name))
 }
 
 #[no_mangle]
 pub extern "C" fn fog_datetime_test_get_value(
     id: u32,
-    storage_name: &nsACString,
+    ping_name: &nsACString,
     value: &mut nsACString,
 ) {
-    let val = with_metric!(DATETIME_MAP, id, metric, test_get!(metric, storage_name));
+    let val = with_metric!(DATETIME_MAP, id, metric, test_get!(metric, ping_name));
     value.assign(&val.to_rfc3339());
 }
 
@@ -196,22 +192,19 @@ pub extern "C" fn fog_datetime_set(
 }
 
 #[no_mangle]
-pub extern "C" fn fog_memory_distribution_test_has_value(
-    id: u32,
-    storage_name: &nsACString,
-) -> bool {
+pub extern "C" fn fog_memory_distribution_test_has_value(id: u32, ping_name: &nsACString) -> bool {
     with_metric!(
         MEMORY_DISTRIBUTION_MAP,
         id,
         metric,
-        test_has!(metric, storage_name)
+        test_has!(metric, ping_name)
     )
 }
 
 #[no_mangle]
 pub extern "C" fn fog_memory_distribution_test_get_value(
     id: u32,
-    storage_name: &nsACString,
+    ping_name: &nsACString,
     sum: &mut u64,
     buckets: &mut ThinVec<u64>,
     counts: &mut ThinVec<u64>,
@@ -220,7 +213,7 @@ pub extern "C" fn fog_memory_distribution_test_get_value(
         MEMORY_DISTRIBUTION_MAP,
         id,
         metric,
-        test_get!(metric, storage_name)
+        test_get!(metric, ping_name)
     );
     *sum = val.sum;
     for (&bucket, &count) in val.values.iter() {
@@ -337,3 +330,18 @@ pub extern "C" fn fog_labeled_string_get(id: u32, label: &nsACString) -> u32 {
         LabeledStringMetric
     )
 }
+
+#[no_mangle]
+pub extern "C" fn fog_quantity_set(id: u32, value: i64) {
+    with_metric!(QUANTITY_MAP, id, metric, metric.set(value));
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn fog_quantity_test_has_value(id: u32, ping_name: &nsACString) -> bool {
+    with_metric!(QUANTITY_MAP, id, metric, test_has!(metric, ping_name))
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn fog_quantity_test_get_value(id: u32, ping_name: &nsACString) -> i64 {
+    with_metric!(QUANTITY_MAP, id, metric, test_get!(metric, ping_name))
+}

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

@@ -22,6 +22,7 @@ mod labeled;
 mod labeled_counter;
 mod memory_distribution;
 mod ping;
+mod quantity;
 pub(crate) mod string;
 mod string_list;
 mod timespan;
@@ -37,6 +38,7 @@ pub use self::labeled::LabeledMetric;
 pub use self::labeled_counter::LabeledCounterMetric;
 pub use self::memory_distribution::MemoryDistributionMetric;
 pub use self::ping::Ping;
+pub use self::quantity::QuantityMetric;
 pub use self::string::StringMetric;
 pub use self::string::StringMetric as LabeledStringMetric;
 pub use self::string_list::StringListMetric;

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

@@ -0,0 +1,157 @@
+// 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 inherent::inherent;
+
+use glean::traits::Quantity;
+
+use super::CommonMetricData;
+
+use crate::ipc::need_ipc;
+use crate::private::MetricId;
+
+/// A quantity metric.
+///
+/// Records a single numeric value of a specific unit.
+#[derive(Clone)]
+pub enum QuantityMetric {
+    Parent(glean::private::QuantityMetric),
+    Child(QuantityMetricIpc),
+}
+#[derive(Clone, Debug)]
+pub struct QuantityMetricIpc;
+
+impl QuantityMetric {
+    /// Create a new quantity metric.
+    pub fn new(_id: MetricId, meta: CommonMetricData) -> Self {
+        if need_ipc() {
+            QuantityMetric::Child(QuantityMetricIpc)
+        } else {
+            QuantityMetric::Parent(glean::private::QuantityMetric::new(meta))
+        }
+    }
+
+    #[cfg(test)]
+    pub(crate) fn child_metric(&self) -> Self {
+        match self {
+            QuantityMetric::Parent(_) => QuantityMetric::Child(QuantityMetricIpc),
+            QuantityMetric::Child(_) => panic!("Can't get a child metric from a child metric"),
+        }
+    }
+}
+
+#[inherent(pub)]
+impl Quantity for QuantityMetric {
+    /// Set the value. Must be non-negative.
+    ///
+    /// # Arguments
+    ///
+    /// * `value` - The value. Must be non-negative.
+    ///
+    /// ## Notes
+    ///
+    /// Logs an error if the `value` is negative.
+    fn set(&self, value: i64) {
+        match self {
+            QuantityMetric::Parent(p) => {
+                Quantity::set(&*p, value);
+            }
+            QuantityMetric::Child(_) => {
+                log::error!("Unable to set quantity metric in non-parent process. Ignoring.");
+                // TODO: Record an error.
+            }
+        }
+    }
+
+    /// **Test-only API.**
+    ///
+    /// Get the currently stored value.
+    /// This doesn't clear the stored value.
+    ///
+    /// ## Arguments
+    ///
+    /// * `ping_name` - the storage name to look into.
+    ///
+    /// ## Return value
+    ///
+    /// Returns the stored value or `None` if nothing stored.
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(&self, ping_name: S) -> Option<i64> {
+        match self {
+            QuantityMetric::Parent(p) => p.test_get_value(ping_name),
+            QuantityMetric::Child(_) => {
+                panic!("Cannot get test value for quantity metric in non-parent process!",)
+            }
+        }
+    }
+
+    /// **Test-only API.**
+    ///
+    /// Gets the number of recorded errors for the given metric and error type.
+    ///
+    /// # Arguments
+    ///
+    /// * `error` - The type of error
+    /// * `ping_name` - represents the optional name of the ping to retrieve the
+    ///   metric for. Defaults to the first value in `send_in_pings`.
+    ///
+    /// # Returns
+    ///
+    /// The number of errors reported.
+    fn test_get_num_recorded_errors<'a, S: Into<Option<&'a str>>>(
+        &self,
+        error: glean::ErrorType,
+        ping_name: S,
+    ) -> i32 {
+        match self {
+            QuantityMetric::Parent(p) => {
+                p.test_get_num_recorded_errors(error, ping_name)
+            }
+            QuantityMetric::Child(_) => panic!(
+                "Cannot get the number of recorded errors for quantity metric in non-parent process!"
+            ),
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use crate::{common_test::*, ipc, metrics};
+
+    #[test]
+    fn sets_quantity_metric() {
+        let _lock = lock_test();
+
+        let metric = &metrics::test_only_ipc::a_quantity;
+        metric.set(14);
+
+        assert_eq!(14, metric.test_get_value("store1").unwrap());
+    }
+
+    #[test]
+    fn quantity_ipc() {
+        // QuantityMetric doesn't support IPC.
+        let _lock = lock_test();
+
+        let parent_metric = &metrics::test_only_ipc::a_quantity;
+
+        parent_metric.set(15);
+
+        {
+            let child_metric = parent_metric.child_metric();
+
+            // scope for need_ipc RAII
+            let _raii = ipc::test_set_need_ipc(true);
+
+            // Instrumentation calls do not panic.
+            child_metric.set(30);
+
+            // (They also shouldn't do anything,
+            // but that's not something we can inspect in this test)
+        }
+
+        assert!(ipc::replay_from_buf(&ipc::take_buf().unwrap()).is_ok());
+
+        assert_eq!(15, parent_metric.test_get_value(None).unwrap());
+    }
+}

toolkit/components/glean/docs/dev/new_metric_types.md

@@ -48,6 +48,18 @@ we split the metric into three pieces:
     * If it does support operations in non-parent processes it stores the
       `MetricId` that identifies this particular metric in a cross-process fashion.
 
+## Mirrors
+
+FOG can mirror Glean metrics to Telemetry probes via the
+[Glean Interface For Firefox Telemetry](../user/gifft.md).
+
+Can this metric type be mirrored?
+Should it be mirrored?
+
+If so, add an appropriate Telemetry probe for it to mirror to,
+documenting the compatibility in
+[the GIFFT docs](../user/gifft.md).
+
 ## Rust
 
 FOG uses the Rust Language Binding APIs (the `glean` crate) with a layer of IPC on top.

toolkit/components/glean/test_metrics.yaml

@@ -516,3 +516,23 @@ test_only.ipc:
     no_lint:
       - COMMON_PREFIX
     telemetry_mirror: TELEMETRY_TEST_KEYED_UNSIGNED_INT
+  a_quantity:
+    type: quantity
+    unit: squad
+    description: |
+      This is a test-only metric.
+      Just quantifying things.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1704846
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1704846#c1
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    send_in_pings:
+      - store1
+    no_lint:
+      - COMMON_PREFIX
+    telemetry_mirror: TELEMETRY_TEST_MIRROR_FOR_QUANTITY

toolkit/components/telemetry/Scalars.yaml

@@ -7597,6 +7597,15 @@ telemetry.test:
     products: ["firefox", "thunderbird"]
     record_in_processes: ["all"]
 
+  mirror_for_quantity:
+    bug_numbers: [1704846]
+    description: Test only. This is a mirror probe for a Glean quantity metric.
+    expires: never
+    kind: uint
+    notification_emails: [glean-team@mozilla.com]
+    products: ["firefox", "thunderbird"]
+    record_in_processes: ["all"]
+
 
 # The following section contains counters for corruption of internal omni jar files.
 corroborate: