Bug 1677454: Implement the string list type in the RLB (#1380)

* Bug 1677454: Implement the string list type in the RLB

* Test and fix up doc examples

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 [Full changelog](https://github.com/mozilla/glean/compare/v33.8.0...main)
 
+* Rust
+  * Introduce the String List metric type in the RLB. [#1380](https://github.com/mozilla/glean/pull/1380)
+
 # v33.8.0 (2020-12-10)
 
 [Full changelog](https://github.com/mozilla/glean/compare/v33.7.0...v33.8.0)

docs/user/metrics/string_list.md

@@ -150,6 +150,43 @@ Assert.Equal(
 ```
 </div>
 
+<div data-lang="Rust" class="tab">
+
+```Rust
+use glean_metrics;
+
+// Add them one at a time
+engines.iter().for_each(|x|
+  search::engines.add(x)
+);
+
+// Set them in one go
+search::engines.set(engines)
+```
+
+There are test APIs available too:
+
+```Rust
+use glean::ErrorType;
+use glean_metrics;
+
+// Was anything recorded?
+assert!(search::engines.test_get_value(None).is_some());
+// Does it have the expected value?
+// IMPORTANT: It may have been truncated -- see "Limits" below
+assert_eq!(
+    vec!["Google".to_string(), "DuckDuckGo".to_string()], 
+    search::engines.test_get_value(None)
+);
+// Were any of the values too long, and thus an error was recorded?
+assert_eq!(
+    0, 
+    search::engines.test_get_num_recorded_errors(ErrorType::InvalidValue)
+);
+```
+
+</div>
+
 {{#include ../../tab_footer.md}}
 
 ## Limits
@@ -172,3 +209,4 @@ Assert.Equal(
 
 * [Kotlin API docs](../../../javadoc/glean/mozilla.telemetry.glean.private/-string-list-metric-type/index.html)
 * [Python API docs](../../../python/glean/metrics/string_list.html)
+* [Rust API docs](../../../docs/glean/private/struct.StringListMetric.html)

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

@@ -13,6 +13,7 @@ mod ping;
 mod quantity;
 mod recorded_experiment_data;
 mod string;
+mod string_list;
 mod timespan;
 mod uuid;
 
@@ -26,4 +27,5 @@ pub use ping::PingType;
 pub use quantity::QuantityMetric;
 pub use recorded_experiment_data::RecordedExperimentData;
 pub use string::StringMetric;
+pub use string_list::StringListMetric;
 pub use timespan::TimespanMetric;

glean-core/rlb/src/private/string_list.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.
+
+/// This implements the developer-facing API for recording string list 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 StringListMetric(pub(crate) Arc<glean_core::metrics::StringListMetric>);
+
+impl StringListMetric {
+    /// The public constructor used by automatically generated metrics.
+    pub fn new(meta: glean_core::CommonMetricData) -> Self {
+        Self(Arc::new(glean_core::metrics::StringListMetric::new(meta)))
+    }
+}
+
+#[inherent(pub)]
+impl glean_core::traits::StringList for StringListMetric {
+    fn add<S: Into<String>>(&self, value: S) {
+        let metric = Arc::clone(&self.0);
+        let new_value = value.into();
+        dispatcher::launch(move || crate::with_glean(|glean| metric.add(glean, new_value)));
+    }
+
+    fn set(&self, value: Vec<String>) {
+        let metric = Arc::clone(&self.0);
+        dispatcher::launch(move || crate::with_glean(|glean| metric.set(glean, value)));
+    }
+
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(&self, ping_name: S) -> Option<Vec<String>> {
+        dispatcher::block_on_queue();
+
+        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 {
+        dispatcher::block_on_queue();
+
+        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, ErrorType};
+
+    #[test]
+    fn string_list_metric_docs() {
+        let _lock = lock_test();
+        let _t = new_glean(None, true);
+
+        let engine_metric: StringListMetric = StringListMetric::new(CommonMetricData {
+            name: "event".into(),
+            category: "test".into(),
+            send_in_pings: vec!["test1".into()],
+            ..Default::default()
+        });
+
+        let engines: Vec<String> = vec!["Google".to_string(), "DuckDuckGo".to_string()];
+
+        // Add them one at a time
+        engines.iter().for_each(|x| engine_metric.add(x));
+
+        // Set them in one go
+        engine_metric.set(engines);
+
+        assert!(engine_metric.test_get_value(None).is_some());
+
+        assert_eq!(
+            vec!["Google".to_string(), "DuckDuckGo".to_string()],
+            engine_metric.test_get_value(None).unwrap()
+        );
+
+        assert_eq!(
+            0,
+            engine_metric.test_get_num_recorded_errors(ErrorType::InvalidValue, None)
+        );
+    }
+}

glean-core/src/traits/string_list.rs

@@ -2,6 +2,8 @@
 // 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;
+
 /// A description for the [`StringListMetric`](crate::metrics::StringListMetric) type.
 ///
 /// When changing this trait, make sure all the operations are
@@ -45,17 +47,20 @@ pub trait StringList {
 
     /// **Exported for test purposes.**
     ///
-    /// Gets the currently-stored values as a JSON String of the format
-    /// ["string1", "string2", ...]
-    ///
-    /// 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;
 }