bug 1645166 - Support rate external denominators

Rates and Counters are, to the db and to our definitions files, just that:
Rates and Counters.

But to the Language Bindings they can be of a few varieties. Rates can
be RateMetric or NumeratorMetric (for the case of an external denominator).
Counters can be CounterMetric or DenominatorMetric (similarly).

This allows for custom behaviours (CounterMetric can be Labeled,
DenominatorMetric stores its numerators).

Note that Numerator, being a strict subset of Rate, has no Core impl. LBs'
NumeratorMetrics are built atop Core RateMetrics, hiding denominator access.

CHANGELOG.md

@@ -7,6 +7,8 @@
     * The new API exists for all language bindings (Kotlin, Swift, Rust, Python).
 * Python
   * Update minimal required version of `cffi` dependency to 1.13.0 ([#1520](https://github.com/mozilla/glean/pull/1520)).
+* RLB
+  * Added `rate` metric type.
 
 # v35.0.0 (2021-02-22)
 
@@ -17,8 +19,8 @@
   * The `testGetValue` APIs now include a message on the `NullPointerException` thrown when the value is missing.
   * **Breaking change:** `LEGACY_TAG_PINGS` is removed from `GleanDebugActivity` ([#1510](https://github.com/mozilla/glean/pull/1510))
 * RLB
-  * **Breaking change:** `Configuration.data_path` is now a `std::path::PathBuf`([#1493](https://github.com/mozilla/glean/pull/1493)).
-  
+  * **Breaking change:** `Configuration.data_path` is now a `std::path::PathBuf`([#1493](https://github.com/mozilla/glean/pull/1493)). 
+
 # v34.1.0 (2021-02-04)
 
 [Full changelog](https://github.com/mozilla/glean/compare/v34.0.0...v34.1.0)

docs/user/SUMMARY.md

@@ -30,6 +30,7 @@
     - [Event](user/metrics/event.md)
     - [Custom Distribution](user/metrics/custom_distribution.md)
     - [Quantity](user/metrics/quantity.md)
+    - [Rate](user/metrics/rate.md)
 - [Pings](user/pings/index.md)
     - [Ping schedules and timings overview](user/pings/ping-schedules-and-timings.md)
     - [Baseline Ping](user/pings/baseline.md)

docs/user/user/metrics/rate.md

@@ -28,24 +28,65 @@ network:
     ...
 ```
 
+### External Denominators
+
+If several rates share the same denominator
+(from our example above, maybe there are multiple rates per total connections made)
+then the denominator should be defined as a `counter` and shared between
+`rates` using the `denominator_metric` property:
+
+```YAML
+network:
+  http_connections:
+    type: counter
+    description: >
+      Total number of http connections made.
+    ...
+
+  http_connection_error:
+    type: rate
+    description: >
+      How many HTTP connections error out out of the total connections made.
+    denominator_metric: network.http_connections
+    ...
+```
+
 ## API
 
-{{#include ../../tab_header.md}}
+{{#include ../../../shared/tab_header.md}}
 
 <div data-lang="Rust" class="tab">
 
 Since a rate is two numbers, you add to each one individually:
 
-```
+```rust
 use glean_metrics;
 
-if connection_had_error:
+if connection_had_error {
     network::http_connection_error.add_to_numerator(1);
+}
 
 network::http_connection_error.add_to_denominator(1);
 ```
 
-There are test APIs available too:
+If the rate uses an external denominator,
+adding to the denominator must be done through the denominator's
+`counter` API:
+
+```rust
+use glean_metrics;
+
+if connection_had_error {
+    network::http_connection_error.add_to_numerator(1);
+}
+
+// network::http_connection_error has no `add_to_denominator` method.
+network::http_connections.add(1);
+```
+
+There are test APIs available too.
+Whether the rate has an external denominator or not,
+you can use this API to get the current value:
 
 ```rust
 use glean::ErrorType;
@@ -67,7 +108,7 @@ assert_eq!(
 
 </div>
 
-{{#include ../../tab_footer.md}}
+{{#include ../../../shared/tab_footer.md}}
 
 ## Limits
 

glean-core/rlb/src/private/denominator.rs

@@ -0,0 +1,62 @@
+// 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 std::sync::Arc;
+
+use glean_core::metrics::MetricType;
+use glean_core::ErrorType;
+use glean_core::CommonMetricData;
+
+// We need to wrap the glean-core type: otherwise if we try to implement
+// the trait for the metric in `glean_core::metrics` we hit error[E0117]:
+// only traits defined in the current crate can be implemented for arbitrary
+// types.
+
+/// Developer-facing API for recording counter metrics that are acting as
+/// external denominators for rate metrics.
+///
+/// Instances of this class type are automatically generated by the parsers
+/// at build time, allowing developers to record values that were previously
+/// registered in the metrics.yaml file.
+#[derive(Clone)]
+pub struct DenominatorMetric(pub(crate) Arc<glean_core::metrics::DenominatorMetric>);
+
+impl DenominatorMetric {
+    /// The public constructor used by automatically generated metrics.
+    pub fn new(meta: CommonMetricData, numerators: Vec<CommonMetricData>) -> Self {
+        Self(Arc::new(glean_core::metrics::DenominatorMetric::new(meta, numerators)))
+    }
+}
+
+#[inherent(pub)]
+impl glean_core::traits::Counter for DenominatorMetric {
+    fn add(&self, amount: i32) {
+        let metric = Arc::clone(&self.0);
+        crate::launch_with_glean(move |glean| metric.add(glean, amount));
+    }
+
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(&self, ping_name: S) -> Option<i32> {
+        crate::block_on_dispatcher();
+
+        let queried_ping_name = ping_name
+            .into()
+            .unwrap_or_else(|| &self.0.meta().send_in_pings[0]);
+
+        crate::with_glean(|glean| self.0.test_get_value(glean, queried_ping_name))
+    }
+
+    fn test_get_num_recorded_errors<'a, S: Into<Option<&'a str>>>(
+        &self,
+        error: ErrorType,
+        ping_name: S,
+    ) -> i32 {
+        crate::block_on_dispatcher();
+
+        crate::with_glean_mut(|glean| {
+            glean_core::test_get_num_recorded_errors(&glean, self.0.meta(), error, ping_name.into())
+                .unwrap_or(0)
+        })
+    }
+}

glean-core/rlb/src/private/mod.rs

@@ -8,9 +8,11 @@ mod boolean;
 mod counter;
 mod custom_distribution;
 mod datetime;
+mod denominator;
 mod event;
 mod labeled;
 mod memory_distribution;
+mod numerator;
 mod ping;
 mod quantity;
 mod rate;
@@ -26,9 +28,11 @@ pub use boolean::BooleanMetric;
 pub use counter::CounterMetric;
 pub use custom_distribution::CustomDistributionMetric;
 pub use datetime::{Datetime, DatetimeMetric};
+pub use denominator::DenominatorMetric;
 pub use event::EventMetric;
 pub use labeled::{AllowLabeled, LabeledMetric};
 pub use memory_distribution::MemoryDistributionMetric;
+pub use numerator::NumeratorMetric;
 pub use ping::PingType;
 pub use quantity::QuantityMetric;
 pub use rate::RateMetric;

glean-core/rlb/src/private/numerator.rs

@@ -0,0 +1,108 @@
+// 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 std::sync::Arc;
+
+use glean_core::metrics::MetricType;
+use glean_core::ErrorType;
+
+use crate::dispatcher;
+
+// We need to wrap the glean-core type: otherwise if we try to implement
+// the trait for the metric in `glean_core::metrics` we hit error[E0117]:
+// only traits defined in the current crate can be implemented for arbitrary
+// types.
+
+/// Developer-facing API for recording rate metrics with external denominators.
+///
+/// Instances of this class type are automatically generated by the parsers
+/// at build time, allowing developers to record values that were previously
+/// registered in the metrics.yaml file.
+#[derive(Clone)]
+pub struct NumeratorMetric(pub(crate) Arc<glean_core::metrics::RateMetric>);
+
+impl NumeratorMetric {
+    /// The public constructor used by automatically generated metrics.
+    pub fn new(meta: glean_core::CommonMetricData) -> Self {
+        Self(Arc::new(glean_core::metrics::RateMetric::new(meta)))
+    }
+}
+
+#[inherent(pub)]
+impl glean_core::traits::Numerator for NumeratorMetric {
+    fn add_to_numerator(&self, amount: i32) {
+        let metric = Arc::clone(&self.0);
+        dispatcher::launch(move || {
+            crate::with_glean(|glean| metric.add_to_numerator(glean, amount))
+        });
+    }
+
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(&self, ping_name: S) -> Option<(i32, i32)> {
+        crate::block_on_dispatcher();
+
+        let queried_ping_name = ping_name
+            .into()
+            .unwrap_or_else(|| &self.0.meta().send_in_pings[0]);
+
+        crate::with_glean(|glean| self.0.test_get_value(glean, queried_ping_name))
+    }
+
+    fn test_get_num_recorded_errors<'a, S: Into<Option<&'a str>>>(
+        &self,
+        error: ErrorType,
+        ping_name: S,
+    ) -> i32 {
+        crate::block_on_dispatcher();
+
+        crate::with_glean_mut(|glean| {
+            glean_core::test_get_num_recorded_errors(&glean, self.0.meta(), error, ping_name.into())
+                .unwrap_or(0)
+        })
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::common_test::{lock_test, new_glean};
+    use crate::CommonMetricData;
+
+    #[test]
+    fn numerator_smoke() {
+        let _lock = lock_test();
+        let _t = new_glean(None, true);
+
+        let metric: NumeratorMetric = NumeratorMetric::new(CommonMetricData {
+            name: "rate".into(),
+            category: "test".into(),
+            send_in_pings: vec!["test1".into()],
+            ..Default::default()
+        });
+
+        // Adding 0 doesn't error.
+        metric.add_to_numerator(0);
+
+        assert_eq!(
+            metric.test_get_num_recorded_errors(ErrorType::InvalidValue, None),
+            0
+        );
+
+        // Adding a negative value errors.
+        metric.add_to_numerator(-1);
+
+        assert_eq!(
+            metric.test_get_num_recorded_errors(ErrorType::InvalidValue, None),
+            1
+        );
+
+        // Getting the value returns 0s if that's all we have.
+        assert_eq!(metric.test_get_value(None), Some((0, 0)));
+
+        // And normal values of course work.
+        metric.add_to_numerator(22);
+
+        assert_eq!(metric.test_get_value(None), Some((22, 0)));
+    }
+}

glean-core/src/metrics/denominator.rs

@@ -0,0 +1,99 @@
+// 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 crate::error_recording::{record_error, ErrorType};
+use crate::metrics::CounterMetric;
+use crate::metrics::Metric;
+use crate::metrics::MetricType;
+use crate::metrics::RateMetric;
+use crate::storage::StorageManager;
+use crate::CommonMetricData;
+use crate::Glean;
+
+/// A Denominator metric (a kind of count shared among Rate metrics).
+///
+/// Used to count things.
+/// The value can only be incremented, not decremented.
+#[derive(Clone, Debug)]
+pub struct DenominatorMetric {
+    counter: CounterMetric,
+    numerators: Vec<RateMetric>,
+}
+
+impl MetricType for DenominatorMetric {
+    fn meta(&self) -> &CommonMetricData {
+        self.counter.meta()
+    }
+
+    fn meta_mut(&mut self) -> &mut CommonMetricData {
+        self.counter.meta_mut()
+    }
+}
+
+impl DenominatorMetric {
+    /// Creates a new denominator metric.
+    pub fn new(meta: CommonMetricData, numerators: Vec<CommonMetricData>) -> Self {
+        Self {
+            counter: CounterMetric::new(meta),
+            numerators: numerators.into_iter().map(RateMetric::new).collect(),
+        }
+    }
+
+    /// Increases the denominator by `amount`.
+    ///
+    /// # Arguments
+    ///
+    /// * `glean` - The Glean instance this metric belongs to.
+    /// * `amount` - The amount to increase by. Should be positive.
+    ///
+    /// ## Notes
+    ///
+    /// Logs an error if the `amount` is 0 or negative.
+    pub fn add(&self, glean: &Glean, amount: i32) {
+        if !self.should_record(glean) {
+            return;
+        }
+
+        if amount <= 0 {
+            record_error(
+                glean,
+                self.meta(),
+                ErrorType::InvalidValue,
+                format!("Added negative or zero value {}", amount),
+                None,
+            );
+            return;
+        }
+
+        for num in &self.numerators {
+            num.add_to_denominator(glean, amount);
+        }
+
+        glean
+            .storage()
+            .record_with(glean, self.counter.meta(), |old_value| match old_value {
+                Some(Metric::Counter(old_value)) => {
+                    Metric::Counter(old_value.saturating_add(amount))
+                }
+                _ => Metric::Counter(amount),
+            })
+    }
+
+    /// **Test-only API (exported for FFI purposes).**
+    ///
+    /// Gets the currently stored value as an integer.
+    ///
+    /// This doesn't clear the stored value.
+    pub fn test_get_value(&self, glean: &Glean, storage_name: &str) -> Option<i32> {
+        match StorageManager.snapshot_metric(
+            glean.storage(),
+            storage_name,
+            &self.meta().identifier(glean),
+            self.meta().lifetime,
+        ) {
+            Some(Metric::Counter(i)) => Some(i),
+            _ => None,
+        }
+    }
+}

glean-core/src/metrics/mod.rs

@@ -14,6 +14,7 @@ mod boolean;
 mod counter;
 mod custom_distribution;
 mod datetime;
+mod denominator;
 mod event;
 mod experiment;
 mod jwe;
@@ -41,6 +42,7 @@ pub use self::boolean::BooleanMetric;
 pub use self::counter::CounterMetric;
 pub use self::custom_distribution::CustomDistributionMetric;
 pub use self::datetime::DatetimeMetric;
+pub use self::denominator::DenominatorMetric;
 pub use self::event::EventMetric;
 pub(crate) use self::experiment::ExperimentMetric;
 pub use crate::histogram::HistogramType;

glean-core/src/traits/mod.rs

@@ -15,6 +15,7 @@ mod event;
 mod jwe;
 mod labeled;
 mod memory_distribution;
+mod numerator;
 mod ping;
 mod quantity;
 mod rate;
@@ -35,6 +36,7 @@ pub use self::event::NoExtraKeys;
 pub use self::jwe::Jwe;
 pub use self::labeled::Labeled;
 pub use self::memory_distribution::MemoryDistribution;
+pub use self::numerator::Numerator;
 pub use self::ping::Ping;
 pub use self::quantity::Quantity;
 pub use self::rate::Rate;

glean-core/src/traits/numerator.rs

@@ -0,0 +1,52 @@
+// 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 crate::ErrorType;
+
+// When changing this trait, ensure all operations are implemented in the
+// related type in `../metrics`. (Except test_get_num_errors)
+/// A description for the `NumeratorMetric` subtype of the [`RateMetric`](crate::metrics::RateMetric) type.
+pub trait Numerator {
+    /// Increases the numerator by `amount`.
+    ///
+    /// # Arguments
+    ///
+    /// * `amount` - The amount to increase by. Should be non-negative.
+    ///
+    /// ## Notes
+    ///
+    /// Logs an error if the `amount` is negative.
+    fn add_to_numerator(&self, amount: i32);
+
+    /// **Exported for test purposes.**
+    ///
+    /// Gets the currently stored value as a pair of integers.
+    ///
+    /// # Arguments
+    ///
+    /// * `ping_name` - the optional name of the ping to retrieve the metric
+    ///                 for. Defaults to the first value in `send_in_pings`.
+    ///
+    /// This doesn't clear the stored value.
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(&self, ping_name: S) -> Option<(i32, i32)>;
+
+    /// **Exported for test purposes.**
+    ///
+    /// Gets the number of recorded errors for the given metric and error type.
+    ///
+    /// # Arguments
+    ///
+    /// * `error` - The type of error
+    /// * `ping_name` - 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: ErrorType,
+        ping_name: S,
+    ) -> i32;
+}