feat: fix high cardinality metrics tags (#1437)

Closes #1436

syncserver/src/db/mysql/models.rs

@@ -31,7 +31,6 @@ use super::{
 };
 use crate::db;
 use crate::server::metrics::Metrics;
-use crate::web::tags::Tags;
 
 pub type Result<T> = std::result::Result<T, DbError>;
 type Conn = PooledConnection<ConnectionManager<MysqlConnection>>;
@@ -402,11 +401,9 @@ impl MysqlDb {
                 collection_id,
             })?;
             if usage.total_bytes >= self.quota.size as usize {
-                let mut tags = Tags::default();
-                tags.tags
-                    .insert("collection".to_owned(), bso.collection.clone());
-                self.metrics
-                    .incr_with_tags("storage.quota.at_limit", Some(tags));
+                let mut tags = HashMap::default();
+                tags.insert("collection".to_owned(), bso.collection.clone());
+                self.metrics.incr_with_tags("storage.quota.at_limit", tags);
                 if self.quota.enforced {
                     return Err(DbErrorKind::Quota.into());
                 } else {

syncserver/src/db/spanner/batch.rs

@@ -18,7 +18,6 @@ use uuid::Uuid;
 
 use super::models::{Result, SpannerDb, PRETOUCH_TS};
 use super::support::{as_type, null_value, struct_type_field, IntoSpannerValue};
-use crate::web::tags::Tags;
 
 pub async fn create_async(
     db: &SpannerDb,
@@ -286,8 +285,8 @@ pub async fn do_append_async(
     let mut existing = HashSet::new();
     let mut collisions = HashSet::new();
     let mut count_collisions = 0;
-    let mut tags = Tags::default();
-    tags.tags.insert(
+    let mut tags = HashMap::new();
+    tags.insert(
         "collection".to_owned(),
         db.get_collection_name(collection_id)
             .await
@@ -330,7 +329,7 @@ pub async fn do_append_async(
     db.metrics.count_with_tags(
         "storage.spanner.batch.pre-existing",
         existing.len() as i64,
-        Some(tags.clone()),
+        tags.clone(),
     );
 
     // Approach 1:
@@ -398,7 +397,7 @@ pub async fn do_append_async(
         db.metrics.count_with_tags(
             "storage.spanner.batch.collisions",
             count_collisions,
-            Some(tags.clone()),
+            tags.clone(),
         );
     }
 
@@ -463,7 +462,7 @@ pub async fn do_append_async(
         db.metrics.count_with_tags(
             "storage.spanner.batch.insert",
             count_inserts as i64,
-            Some(tags.clone()),
+            tags.clone(),
         );
     }
 

syncserver/src/db/spanner/models.rs

@@ -31,7 +31,7 @@ use syncserver_db_common::{
 };
 use syncstorage_settings::Quota;
 
-use crate::{db::spanner::now, server::metrics::Metrics, web::tags::Tags};
+use crate::{db::spanner::now, server::metrics::Metrics};
 
 use super::{
     batch,
@@ -1044,11 +1044,10 @@ impl SpannerDb {
             .execute_dml_async(&self.conn)
             .await?;
         if affected_rows > 0 {
-            let mut tags = Tags::default();
-            tags.tags
-                .insert("collection".to_string(), params.collection);
+            let mut tags = HashMap::default();
+            tags.insert("collection".to_string(), params.collection);
             self.metrics
-                .incr_with_tags("storage.spanner.delete_collection", Some(tags));
+                .incr_with_tags("storage.spanner.delete_collection", tags);
             self.erect_tombstone(&params.user_id).await
         } else {
             self.get_storage_timestamp(params.user_id).await
@@ -1109,9 +1108,8 @@ impl SpannerDb {
                 .execute_dml_async(&self.conn)
                 .await?;
         } else {
-            let mut tags = Tags::default();
-            tags.tags
-                .insert("collection".to_owned(), collection.to_owned());
+            let mut tags = HashMap::default();
+            tags.insert("collection".to_owned(), collection.to_owned());
             self.metrics
                 .clone()
                 .start_timer("storage.quota.init_totals", Some(tags));
@@ -1187,11 +1185,10 @@ impl SpannerDb {
         .param_types(sqlparam_types)
         .execute_dml_async(&self.conn)
         .await?;
-        let mut tags = Tags::default();
-        tags.tags
-            .insert("collection".to_string(), params.collection.clone());
+        let mut tags = HashMap::default();
+        tags.insert("collection".to_string(), params.collection.clone());
         self.metrics
-            .incr_with_tags("self.storage.delete_bsos", Some(tags));
+            .incr_with_tags("self.storage.delete_bsos", tags);
         self.update_user_collection_quotas(&params.user_id, collection_id)
             .await
     }
@@ -1635,11 +1632,9 @@ impl SpannerDb {
 
     pub fn quota_error(&self, collection: &str) -> DbError {
         // return the over quota error.
-        let mut tags = Tags::default();
-        tags.tags
-            .insert("collection".to_owned(), collection.to_owned());
-        self.metrics
-            .incr_with_tags("storage.quota.at_limit", Some(tags));
+        let mut tags = HashMap::default();
+        tags.insert("collection".to_owned(), collection.to_owned());
+        self.metrics.incr_with_tags("storage.quota.at_limit", tags);
         DbErrorKind::Quota.into()
     }
 

syncserver/src/db/transaction.rs

@@ -1,8 +1,7 @@
-use std::cell::RefMut;
 use std::future::Future;
 
 use actix_http::http::{HeaderValue, Method, StatusCode};
-use actix_http::{Error, Extensions};
+use actix_http::Error;
 use actix_web::dev::{Payload, PayloadStream};
 use actix_web::http::header;
 use actix_web::web::Data;
@@ -19,7 +18,7 @@ use crate::server::ServerState;
 use crate::web::extractors::{
     BsoParam, CollectionParam, HawkIdentifier, PreConditionHeader, PreConditionHeaderOpt,
 };
-use crate::web::tags::Tags;
+use crate::web::tags::Taggable;
 
 #[derive(Clone)]
 pub struct DbTransactionPool {
@@ -31,18 +30,16 @@ pub struct DbTransactionPool {
     precondition: PreConditionHeaderOpt,
 }
 
-fn set_extra(exts: &mut RefMut<'_, Extensions>, connection_info: ConnectionInfo) {
-    let mut tags = Tags::default();
-    tags.add_extra("connection_age", &connection_info.age.to_string());
-    tags.add_extra(
-        "spanner_connection_age",
-        &connection_info.spanner_age.to_string(),
+fn set_extra(req: &HttpRequest, connection_info: ConnectionInfo) {
+    req.add_extra("connection_age".to_owned(), connection_info.age.to_string());
+    req.add_extra(
+        "spanner_connection_age".to_owned(),
+        connection_info.spanner_age.to_string(),
     );
-    tags.add_extra(
-        "spanner_connection_idle",
-        &connection_info.spanner_idle.to_string(),
+    req.add_extra(
+        "spanner_connection_idle".to_owned(),
+        connection_info.spanner_idle.to_string(),
     );
-    tags.commit(exts);
 }
 
 impl DbTransactionPool {
@@ -73,7 +70,7 @@ impl DbTransactionPool {
         // Handle lock error
         if let Err(e) = result {
             // Update the extra info fields.
-            set_extra(&mut request.extensions_mut(), db.get_connection_info());
+            set_extra(&request, db.get_connection_info());
             db.rollback().await?;
             return Err(e.into());
         }
@@ -127,7 +124,7 @@ impl DbTransactionPool {
         let check_precondition = move |db: Box<dyn Db<'a>>| {
             async move {
                 // set the extra information for all requests so we capture default err handlers.
-                set_extra(&mut mreq.extensions_mut(), db.get_connection_info());
+                set_extra(&mreq, db.get_connection_info());
                 let resource_ts = db
                     .extract_resource(
                         self.user_id.clone(),
@@ -235,7 +232,11 @@ impl FromRequest for DbTransactionPool {
                 Ok(v) => v.map(|collection| collection.collection),
                 Err(e) => {
                     // Semi-example to show how to use metrics inside of middleware.
-                    Metrics::from(state.as_ref()).incr("sync.error.collectionParam");
+                    // `Result::unwrap` is safe to use here, since Metrics::extract can never fail
+                    Metrics::extract(&req)
+                        .await
+                        .unwrap()
+                        .incr("sync.error.collectionParam");
                     warn!("⚠️ CollectionParam err: {:?}", e);
                     return Err(e);
                 }