bug 1677456 - Timing Distribution Impl for RLB

CHANGELOG.md

@@ -5,7 +5,7 @@
 * Rust
   * Introduce the String List metric type in the RLB. [#1380](https://github.com/mozilla/glean/pull/1380).
   * Introduce the `Datetime` metric type in the RLB [#1384](https://github.com/mozilla/glean/pull/1384).
-  * Introduce the `CustomDistribution` metric type in the RLB.
+  * Introduce the `CustomDistribution` and `TimingDistribution` metric type in the RLB [#1394](https://github.com/mozilla/glean/pull/1394).
 
 # v33.8.0 (2020-12-10)
 

docs/user/metrics/timing_distribution.md

@@ -260,6 +260,42 @@ Assert.Equal(0, Pages.pageLoad.TestGetNumRecordedErrors(ErrorType.InvalidValue))
 
 </div>
 
+<div data-lang="Rust" class="tab">
+
+```rust
+use glean_metrics;
+
+fn on_page_start() {
+    self.timer_id = Pages.page_load.start();
+}
+
+fn on_page_loaded() {
+    pages::page_load.stop_and_accumulate(self.timer_id);
+}
+```
+
+There are test APIs available too.
+
+```rust
+use glean::ErrorType;
+use glean_metrics;
+
+// Was anything recorded?
+assert!(pages::page_load.test_get_value(None).is_some());
+
+// Assert no errors were recorded.
+let errors = [
+    ErrorType::InvalidValue,
+    ErrorType::InvalidState,
+    ErrorType::InvalidOverflow
+];
+for error in errors {
+    assert_eq!(0, pages::page_load.test_get_num_recorded_errors(error));
+}
+```
+
+</div>
+
 {{#include ../../tab_footer.md}}
 
 ## Limits
@@ -274,6 +310,11 @@ Assert.Equal(0, Pages.pageLoad.TestGetNumRecordedErrors(ErrorType.InvalidValue))
 
   * On Python 3.7 and later, [`time.monotonic_ns()`](https://docs.python.org/3/library/time.html#time.monotonic_ns) is used.  On earlier versions of Python, [`time.monotonics()`](https://docs.python.org/3/library/time.html#time.monotonic) is used, which is not guaranteed to have nanosecond resolution.
 
+  * In Rust,
+    [`time::precise_time_ns()`](https://docs.rs/time/0.1.42/time/fn.precise_time_ns.html)
+    is used.
+
+
 * The maximum timing value that will be recorded depends on the `time_unit` parameter:
 
   - `nanosecond`: 1ns <= x <= 10 minutes

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

@@ -57,7 +57,7 @@ impl glean_core::traits::CustomDistribution for CustomDistributionMetric {
         &self,
         ping_name: S,
     ) -> Option<DistributionData> {
-        dispatcher::block_on_queue();
+        crate::block_on_dispatcher();
 
         let queried_ping_name = ping_name
             .into()
@@ -71,7 +71,7 @@ impl glean_core::traits::CustomDistribution for CustomDistributionMetric {
         error: ErrorType,
         ping_name: S,
     ) -> i32 {
-        dispatcher::block_on_queue();
+        crate::block_on_dispatcher();
 
         crate::with_glean_mut(|glean| {
             glean_core::test_get_num_recorded_errors(&glean, self.0.meta(), error, ping_name.into())

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

@@ -17,6 +17,7 @@ mod recorded_experiment_data;
 mod string;
 mod string_list;
 mod timespan;
+mod timing_distribution;
 mod uuid;
 
 pub use self::uuid::UuidMetric;
@@ -33,3 +34,4 @@ pub use recorded_experiment_data::RecordedExperimentData;
 pub use string::StringMetric;
 pub use string_list::StringListMetric;
 pub use timespan::TimespanMetric;
+pub use timing_distribution::TimingDistributionMetric;

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

@@ -0,0 +1,93 @@
+// 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, RwLock};
+
+use glean_core::metrics::{DistributionData, MetricType, TimeUnit, TimerId};
+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.
+
+/// This implements the developer-facing API for recording timing distribution 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 TimingDistributionMetric(
+    pub(crate) Arc<RwLock<glean_core::metrics::TimingDistributionMetric>>,
+);
+
+impl TimingDistributionMetric {
+    /// The public constructor used by automatically generated metrics.
+    pub fn new(meta: glean_core::CommonMetricData, time_unit: TimeUnit) -> Self {
+        Self(Arc::new(RwLock::new(
+            glean_core::metrics::TimingDistributionMetric::new(meta, time_unit),
+        )))
+    }
+}
+
+#[inherent(pub)]
+impl glean_core::traits::TimingDistribution for TimingDistributionMetric {
+    fn start(&mut self) -> TimerId {
+        let start_time = time::precise_time_ns();
+        self.0.write().unwrap().set_start(start_time)
+    }
+
+    fn stop_and_accumulate(&mut self, id: TimerId) {
+        let stop_time = time::precise_time_ns();
+        let metric = Arc::clone(&self.0);
+        dispatcher::launch(move || {
+            crate::with_glean(|glean| {
+                metric
+                    .write()
+                    .unwrap()
+                    .set_stop_and_accumulate(glean, id, stop_time)
+            })
+        });
+    }
+
+    fn cancel(&mut self, id: TimerId) {
+        let metric = Arc::clone(&self.0);
+        dispatcher::launch(move || metric.write().unwrap().cancel(id));
+    }
+
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(
+        &self,
+        ping_name: S,
+    ) -> Option<DistributionData> {
+        crate::block_on_dispatcher();
+
+        let inner = self.0.read().unwrap();
+        let queried_ping_name = ping_name
+            .into()
+            .unwrap_or_else(|| &inner.meta().send_in_pings[0]);
+
+        crate::with_glean(|glean| inner.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.read().unwrap().meta(),
+                error,
+                ping_name.into(),
+            )
+            .unwrap_or(0)
+        })
+    }
+}

glean-core/src/traits/timing_distribution.rs

@@ -4,6 +4,7 @@
 
 use crate::metrics::DistributionData;
 use crate::metrics::TimerId;
+use crate::ErrorType;
 
 /// A description for the [`TimingDistributionMetric`](crate::metrics::TimingDistributionMetric) type.
 ///
@@ -13,23 +14,19 @@ pub trait TimingDistribution {
     /// Starts tracking time for the provided metric.
     ///
     /// This records an error if it’s already tracking time (i.e.
-    /// [`set_start`](TimingDistribution::set_start) was already called with no corresponding
-    /// [`set_stop_and_accumulate`](TimingDistribution::set_stop_and_accumulate)): in that case the
+    /// [`start`](TimingDistribution::start) was already called with no corresponding
+    /// [`stop_and_accumulate`](TimingDistribution::stop_and_accumulate)): in that case the
     /// original start time will be preserved.
     ///
-    /// # Arguments
-    ///
-    /// * `start_time` - Timestamp in nanoseconds.
-    ///
     /// # Returns
     ///
     /// A unique [`TimerId`] for the new timer.
-    fn set_start(&mut self, start_time: u64);
+    fn start(&mut self) -> TimerId;
 
     /// Stops tracking time for the provided metric and associated timer id.
     ///
     /// Adds a count to the corresponding bucket in the timing distribution.
-    /// This will record an error if no [`set_start`](TimingDistribution::set_start) was
+    /// This will record an error if no [`start`](TimingDistribution::start) was
     /// called.
     ///
     /// # Arguments
@@ -37,11 +34,10 @@ pub trait TimingDistribution {
     /// * `id` - The [`TimerId`] to associate with this timing. This allows
     ///   for concurrent timing of events associated with different ids to the
     ///   same timespan metric.
-    /// * `stop_time` - Timestamp in nanoseconds.
-    fn set_stop_and_accumulate(&mut self, id: TimerId, stop_time: u64);
+    fn stop_and_accumulate(&mut self, id: TimerId);
 
-    /// Aborts a previous [`set_start`](TimingDistribution::set_start) call. No
-    /// error is recorded if no [`set_start`](TimingDistribution::set_start) was
+    /// Aborts a previous [`start`](TimingDistribution::start) call. No
+    /// error is recorded if no [`start`](TimingDistribution::start) was
     /// called.
     ///
     /// # Arguments
@@ -51,34 +47,6 @@ pub trait TimingDistribution {
     ///   same timing distribution metric.
     fn cancel(&mut self, id: TimerId);
 
-    /// Accumulates the provided signed samples in the metric.
-    ///
-    /// This is required so that the platform-specific code can provide us with
-    /// 64 bit signed integers if no `u64` comparable type is available. This
-    /// will take care of filtering and reporting errors for any provided negative
-    /// sample.
-    ///
-    /// Please note that this assumes that the provided samples are already in
-    /// the "unit" declared by the instance of the implementing metric type
-    /// (e.g. if the implementing class is a [TimingDistribution] and the
-    /// instance this method was called on is using second, then `samples` are
-    /// assumed to be in that unit).
-    ///
-    /// # Arguments
-    ///
-    /// * `samples` - The vector holding the samples to be recorded by the metric.
-    ///
-    /// ## Notes
-    ///
-    /// Discards any negative value in `samples` and report an
-    /// [`ErrorType::InvalidValue`](crate::ErrorType::InvalidValue) for each of
-    /// them.
-    ///
-    /// Reports an
-    /// [`ErrorType::InvalidOverflow`](crate::ErrorType::InvalidOverflow) error
-    /// for samples that are longer than `MAX_SAMPLE_TIME`.
-    fn accumulate_samples_signed(&mut self, samples: Vec<i64>);
-
     /// **Exported for test purposes.**
     ///
     /// Gets the currently stored value as an integer.
@@ -96,16 +64,20 @@ pub trait TimingDistribution {
 
     /// **Exported for test purposes.**
     ///
-    /// Gets the currently-stored histogram as a JSON String of the serialized value.
-    ///
-    /// This doesn't clear the stored value.
+    /// Gets the number of recorded errors for the given 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`.
-    fn test_get_value_as_json_string<'a, S: Into<Option<&'a str>>>(
+    ///
+    /// # Returns
+    ///
+    /// The number of errors recorded.
+    fn test_get_num_recorded_errors<'a, S: Into<Option<&'a str>>>(
         &self,
+        error: ErrorType,
         ping_name: S,
-    ) -> Option<String>;
+    ) -> i32;
 }