Bug 1908397 - Suggest metrics

Defined metrics for suggest.  I used labeled timing distributions, which
is a relatively new metric that seems great for our needs.  Using these
metrics, we can track a separate timing distribution for each record
type / provider.

Updated `ingest` to return metrics.  Added `query_with_metrics`, which
returns metrics alongside the usual results. Added some classes to
record metrics as the operations execute.

Moved the top-level `fetch_suggestions` from db.rs to store.rs.  It's
now responsible for metrics, so I thought it fit better in store.

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ### Suggest
 - Added support for Fakespot suggestions.
+- Added support for recording metrics
 - Removed the `SuggestIngestionConstraints::max_suggestions` field.  No consumers were using this.
 
 ## 🦊 What's Changed 🦊

components/suggest/metrics.yaml

@@ -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 http://mozilla.org/MPL/2.0/.
+
+# This file defines the metrics that will be gathered for the Suggest
+# component.  Changes to these metrics require data review.
+#
+# We can't record metrics from Rust directly.  To work around that we:
+#   - Define the metrics in application-services
+#   - Define API calls in application-services that return the metrics
+#     alongside the normal results.
+#   - Record the metrics with Glean in the consumer code
+
+---
+$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0
+
+suggest:
+  ingest_time:
+    type: labeled_timing_distribution
+    description: Time for ingestion (excluding download time), labelled by record type
+    time_unit: microsecond
+    labels:
+      - icon
+      - data
+      - amo-suggestions
+      - pocket-suggestions
+      - yelp-suggestions
+      - mdn-suggestions
+      - weather
+      - configuration
+      - amp-mobile-suggestions
+      - fakespot-suggestions
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1908397
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1911664
+    notification_emails:
+      - [email protected]
+      - [email protected]
+    expires: "never"
+    data_sensitivity:
+      - technical
+
+  ingest_download_time:
+    type: labeled_timing_distribution
+    description: Download time for ingestion, labelled by record type
+    time_unit: microsecond
+    labels:
+      - icon
+      - data
+      - amo-suggestions
+      - pocket-suggestions
+      - yelp-suggestions
+      - mdn-suggestions
+      - weather
+      - configuration
+      - amp-mobile-suggestions
+      - fakespot-suggestions
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1908397
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1911664
+    notification_emails:
+      - [email protected]
+      - [email protected]
+    expires: "never"
+    data_sensitivity:
+      - technical
+
+  query_time:
+    type: labeled_timing_distribution
+    description: Time executing queries, labelled by provider type
+    time_unit: microsecond
+    labels:
+      - amp
+      - ampmobile
+      - wikipedia
+      - amo
+      - pocket
+      - yelp
+      - mdn
+      - weather
+      - fakespot
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1908397
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1911664
+    notification_emails:
+      - [email protected]
+      - [email protected]
+    expires: "never"
+    data_sensitivity:
+      - technical

components/suggest/src/db.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-use std::{collections::HashSet, path::Path, sync::Arc};
+use std::{path::Path, sync::Arc};
 
 use interrupt_support::{SqlInterruptHandle, SqlInterruptScope};
 use parking_lot::{Mutex, MutexGuard};
@@ -203,39 +203,6 @@ impl<'a> SuggestDao<'a> {
             .query_one::<bool>("SELECT NOT EXISTS (SELECT 1 FROM suggestions)")?)
     }
 
-    /// Fetches suggestions that match the given query from the database.
-    pub fn fetch_suggestions(&self, query: &SuggestionQuery) -> Result<Vec<Suggestion>> {
-        let unique_providers = query.providers.iter().collect::<HashSet<_>>();
-        unique_providers
-            .iter()
-            .try_fold(vec![], |mut acc, provider| {
-                let suggestions = match provider {
-                    SuggestionProvider::Amp => {
-                        self.fetch_amp_suggestions(query, AmpSuggestionType::Desktop)
-                    }
-                    SuggestionProvider::AmpMobile => {
-                        self.fetch_amp_suggestions(query, AmpSuggestionType::Mobile)
-                    }
-                    SuggestionProvider::Wikipedia => self.fetch_wikipedia_suggestions(query),
-                    SuggestionProvider::Amo => self.fetch_amo_suggestions(query),
-                    SuggestionProvider::Pocket => self.fetch_pocket_suggestions(query),
-                    SuggestionProvider::Yelp => self.fetch_yelp_suggestions(query),
-                    SuggestionProvider::Mdn => self.fetch_mdn_suggestions(query),
-                    SuggestionProvider::Weather => self.fetch_weather_suggestions(query),
-                    SuggestionProvider::Fakespot => self.fetch_fakespot_suggestions(query),
-                }?;
-                acc.extend(suggestions);
-                Ok(acc)
-            })
-            .map(|mut suggestions| {
-                suggestions.sort();
-                if let Some(limit) = query.limit.and_then(|limit| usize::try_from(limit).ok()) {
-                    suggestions.truncate(limit);
-                }
-                suggestions
-            })
-    }
-
     /// Fetches Suggestions of type Amp provider that match the given query
     pub fn fetch_amp_suggestions(
         &self,

components/suggest/src/lib.rs

@@ -10,6 +10,7 @@ mod config;
 mod db;
 mod error;
 mod keyword;
+mod metrics;
 pub mod pocket;
 mod provider;
 mod query;
@@ -23,8 +24,9 @@ mod yelp;
 
 pub use config::{SuggestGlobalConfig, SuggestProviderConfig};
 pub use error::SuggestApiError;
+pub use metrics::{LabeledTimingSample, SuggestIngestionMetrics};
 pub use provider::SuggestionProvider;
-pub use query::SuggestionQuery;
+pub use query::{QueryWithMetricsResult, SuggestionQuery};
 pub use store::{InterruptKind, SuggestIngestionConstraints, SuggestStore, SuggestStoreBuilder};
 pub use suggestion::{raw_suggestion_url_matches, Suggestion};
 

components/suggest/src/metrics.rs

@@ -0,0 +1,97 @@
+/* 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 http://mozilla.org/MPL/2.0/. */
+
+use std::time::Instant;
+
+/// Single sample for a Glean labeled_timing_distribution
+pub struct LabeledTimingSample {
+    pub label: String,
+    pub value: u64, // time in microseconds
+}
+
+impl LabeledTimingSample {
+    fn new(label: String, value: u64) -> Self {
+        Self { label, value }
+    }
+}
+
+/// Ingestion metrics
+///
+/// These are recorded during [crate::Store::ingest] and returned to the consumer to record.
+#[derive(Default)]
+pub struct SuggestIngestionMetrics {
+    pub ingestion_times: Vec<LabeledTimingSample>,
+    pub download_times: Vec<LabeledTimingSample>,
+}
+
+impl SuggestIngestionMetrics {
+    /// Wraps each iteration in `ingest` and records the time for it.
+    ///
+    /// Passes the closure a DownloadTimer.  Use this to measure the times for all
+    /// downloads that happen during the ingest
+    pub fn measure_ingest<F, T>(&mut self, record_type: impl Into<String>, operation: F) -> T
+    where
+        F: FnOnce(&mut DownloadTimer) -> T,
+    {
+        let timer = Instant::now();
+        let record_type = record_type.into();
+        let mut download_metrics = DownloadTimer::default();
+        let result = operation(&mut download_metrics);
+        let elapsed = timer.elapsed().as_micros() as u64;
+        self.ingestion_times.push(LabeledTimingSample::new(
+            record_type.clone(),
+            elapsed - download_metrics.total_time,
+        ));
+        self.download_times.push(LabeledTimingSample::new(
+            record_type,
+            download_metrics.total_time,
+        ));
+        result
+    }
+}
+
+/// Records download times for a single loop in ingest
+///
+/// [Self::measure_download] can be called multiple times.  [DownloadTimer] will track the total
+/// time for all calls.
+#[derive(Default)]
+pub struct DownloadTimer {
+    total_time: u64,
+}
+
+impl DownloadTimer {
+    pub fn measure_download<F, T>(&mut self, operation: F) -> T
+    where
+        F: FnOnce() -> T,
+    {
+        let timer = Instant::now();
+        let result = operation();
+        self.total_time += timer.elapsed().as_micros() as u64;
+        result
+    }
+}
+
+/// Query metrics
+///
+/// These are recorded during [crate::Store::query] and returned to the consumer to record.
+#[derive(Default)]
+pub struct SuggestQueryMetrics {
+    pub times: Vec<LabeledTimingSample>,
+}
+
+impl SuggestQueryMetrics {
+    pub fn measure_query<F, T>(&mut self, provider: impl Into<String>, operation: F) -> T
+    where
+        F: FnOnce() -> T,
+    {
+        let provider = provider.into();
+        let timer = Instant::now();
+        // Make sure the compiler doesn't reorder/inline in a way that invalidates this
+        // measurement.
+        let result = std::hint::black_box(operation());
+        let elapsed = timer.elapsed().as_micros() as u64;
+        self.times.push(LabeledTimingSample::new(provider, elapsed));
+        result
+    }
+}

components/suggest/src/provider.rs

@@ -3,6 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+use std::fmt;
+
 use rusqlite::{
     types::{FromSql, FromSqlError, FromSqlResult, ToSql, ToSqlOutput, ValueRef},
     Result as RusqliteResult,
@@ -25,6 +27,22 @@ pub enum SuggestionProvider {
     Fakespot = 9,
 }
 
+impl fmt::Display for SuggestionProvider {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Amp => write!(f, "amp"),
+            Self::Wikipedia => write!(f, "wikipedia"),
+            Self::Amo => write!(f, "amo"),
+            Self::Pocket => write!(f, "pocket"),
+            Self::Yelp => write!(f, "yelp"),
+            Self::Mdn => write!(f, "mdn"),
+            Self::Weather => write!(f, "weather"),
+            Self::AmpMobile => write!(f, "ampmobile"),
+            Self::Fakespot => write!(f, "fakespot"),
+        }
+    }
+}
+
 impl FromSql for SuggestionProvider {
     fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
         let v = value.as_i64()?;

components/suggest/src/query.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use crate::SuggestionProvider;
+use crate::{LabeledTimingSample, Suggestion, SuggestionProvider};
 
 /// A query for suggestions to show in the address bar.
 #[derive(Clone, Debug, Default)]
@@ -12,6 +12,11 @@ pub struct SuggestionQuery {
     pub limit: Option<i32>,
 }
 
+pub struct QueryWithMetricsResult {
+    pub suggestions: Vec<Suggestion>,
+    pub query_times: Vec<LabeledTimingSample>,
+}
+
 impl SuggestionQuery {
     // Builder style methods for creating queries (mostly used by the test code)