From 4cb45b19b900e6b5c522adda33e1005c0976dc5d Mon Sep 17 00:00:00 2001 From: jrconlin Date: Wed, 26 Apr 2023 15:44:57 -0700 Subject: [PATCH 1/6] bug: Add back missing metrics A number of metrics were accidentally dropped during the code merge and refactor. --- autoendpoint/src/extractors/notification.rs | 4 ++++ autoendpoint/src/routers/webpush.rs | 6 ++++++ autoendpoint/src/routes/webpush.rs | 7 +++++-- autopush-common/src/db/dynamodb/mod.rs | 11 +++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/autoendpoint/src/extractors/notification.rs b/autoendpoint/src/extractors/notification.rs index a7b745549..c381060f9 100644 --- a/autoendpoint/src/extractors/notification.rs +++ b/autoendpoint/src/extractors/notification.rs @@ -148,6 +148,10 @@ impl Notification { message_id.encrypt(fernet) } + pub fn has_topic(&self) -> bool { + self.headers.topic.is_some() + } + /// Serialize the notification for delivery to the connection server. Some /// fields in `autopush_common`'s `Notification` are marked with /// `#[serde(skip_serializing)]` so they are not shown to the UA. These diff --git a/autoendpoint/src/routers/webpush.rs b/autoendpoint/src/routers/webpush.rs index 2ac6aae7f..ab575378b 100644 --- a/autoendpoint/src/routers/webpush.rs +++ b/autoendpoint/src/routers/webpush.rs @@ -65,6 +65,12 @@ impl Router for WebPushRouter { ); } Err(error) => { + if error.is_timeout() { + self.metrics.incr("error.node.timeout")?; + }; + if error.is_connect() { + self.metrics.incr("error.node.connect")?; + } debug!("Error while sending webpush notification: {}", error); self.remove_node_id(user, node_id).await? } diff --git a/autoendpoint/src/routes/webpush.rs b/autoendpoint/src/routes/webpush.rs index 834d2cb1b..2e965e841 100644 --- a/autoendpoint/src/routes/webpush.rs +++ b/autoendpoint/src/routes/webpush.rs @@ -7,12 +7,13 @@ use crate::extractors::routers::{RouterType, Routers}; use crate::server::AppState; use actix_web::web::Data; use actix_web::HttpResponse; +use cadence::CountedExt; /// Handle the `POST /wpush/{api_version}/{token}` and `POST /wpush/{token}` routes pub async fn webpush_route( notification: Notification, routers: Routers, - _app_state: Data, + app_state: Data, ) -> ApiResult { // TODO: sentry::configure_scope(|scope| { @@ -25,7 +26,9 @@ pub async fn webpush_route( RouterType::from_str(¬ification.subscription.user.router_type) .map_err(|_| ApiErrorKind::InvalidRouterType)?, ); - + if notification.has_topic() { + app_state.metrics.incr("ua.notification.topic")?; + } Ok(router.route_notification(¬ification).await?.into()) } diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index 060efd9a4..8d947e0c5 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -511,6 +511,7 @@ impl DbClient for DdbClientImpl { } async fn save_message(&self, uaid: &Uuid, message: Notification) -> DbResult<()> { + let topic = message.topic.clone(); let input = PutItemInput { item: serde_dynamodb::to_hashmap(&NotificationRecord::from_notif(uaid, message))?, table_name: self.settings.message_table.clone(), @@ -524,6 +525,13 @@ impl DbClient for DdbClientImpl { ) .await?; + let metric = self.metrics.incr_with_tags("notification.message.stored"); + if let Some(topic) = topic { + metric.with_tag("topic", topic.as_str()).send(); + } else { + metric.send(); + } + Ok(()) } @@ -543,6 +551,9 @@ impl DbClient for DdbClientImpl { retryable_delete_error(self.metrics.clone()), ) .await?; + self.metrics + .incr_with_tags("notification.message.removed") + .send(); Ok(()) } From 74b5c353e8deec2729786141ee47d919970111e5 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Wed, 26 Apr 2023 17:08:11 -0700 Subject: [PATCH 2/6] Update autoendpoint/src/routes/webpush.rs remove bad metric Co-authored-by: Philip Jenvey --- autoendpoint/src/routes/webpush.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/autoendpoint/src/routes/webpush.rs b/autoendpoint/src/routes/webpush.rs index 2e965e841..49b417b4c 100644 --- a/autoendpoint/src/routes/webpush.rs +++ b/autoendpoint/src/routes/webpush.rs @@ -26,9 +26,6 @@ pub async fn webpush_route( RouterType::from_str(¬ification.subscription.user.router_type) .map_err(|_| ApiErrorKind::InvalidRouterType)?, ); - if notification.has_topic() { - app_state.metrics.incr("ua.notification.topic")?; - } Ok(router.route_notification(¬ification).await?.into()) } From 075ed6be59b33fbf90b8f37b8a19363e38bd38d0 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Wed, 26 Apr 2023 17:08:34 -0700 Subject: [PATCH 3/6] Update autopush-common/src/db/dynamodb/mod.rs Co-authored-by: Philip Jenvey --- autopush-common/src/db/dynamodb/mod.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index 8d947e0c5..33f38004b 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -525,13 +525,8 @@ impl DbClient for DdbClientImpl { ) .await?; - let metric = self.metrics.incr_with_tags("notification.message.stored"); - if let Some(topic) = topic { - metric.with_tag("topic", topic.as_str()).send(); - } else { - metric.send(); - } - + self.metrics.incr_with_tags("notification.message.stored") + .with_tag("topic", message.topic.is_some().to_string()).send() Ok(()) } From ed58f140f4a7e7a504741068c884dd902942c8c1 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Wed, 26 Apr 2023 17:08:45 -0700 Subject: [PATCH 4/6] Update autopush-common/src/db/dynamodb/mod.rs Co-authored-by: Philip Jenvey --- autopush-common/src/db/dynamodb/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index 33f38004b..73b8cdaa1 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -547,7 +547,7 @@ impl DbClient for DdbClientImpl { ) .await?; self.metrics - .incr_with_tags("notification.message.removed") + .incr_with_tags("notification.message.deleted") .send(); Ok(()) } From 62b2e3a2a3b225173467eed2a673e9bc97243e64 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Thu, 27 Apr 2023 09:44:38 -0700 Subject: [PATCH 5/6] f fix suggested changes --- autoendpoint/src/routes/webpush.rs | 3 +-- autopush-common/src/db/dynamodb/mod.rs | 8 +++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/autoendpoint/src/routes/webpush.rs b/autoendpoint/src/routes/webpush.rs index 49b417b4c..44f0a2d8b 100644 --- a/autoendpoint/src/routes/webpush.rs +++ b/autoendpoint/src/routes/webpush.rs @@ -7,13 +7,12 @@ use crate::extractors::routers::{RouterType, Routers}; use crate::server::AppState; use actix_web::web::Data; use actix_web::HttpResponse; -use cadence::CountedExt; /// Handle the `POST /wpush/{api_version}/{token}` and `POST /wpush/{token}` routes pub async fn webpush_route( notification: Notification, routers: Routers, - app_state: Data, + _app_state: Data, ) -> ApiResult { // TODO: sentry::configure_scope(|scope| { diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index 73b8cdaa1..6f20813fe 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -511,7 +511,7 @@ impl DbClient for DdbClientImpl { } async fn save_message(&self, uaid: &Uuid, message: Notification) -> DbResult<()> { - let topic = message.topic.clone(); + let topic = message.topic.clone().is_some().to_string(); let input = PutItemInput { item: serde_dynamodb::to_hashmap(&NotificationRecord::from_notif(uaid, message))?, table_name: self.settings.message_table.clone(), @@ -525,8 +525,10 @@ impl DbClient for DdbClientImpl { ) .await?; - self.metrics.incr_with_tags("notification.message.stored") - .with_tag("topic", message.topic.is_some().to_string()).send() + self.metrics + .incr_with_tags("notification.message.stored") + .with_tag("topic", &topic) + .send(); Ok(()) } From 91151fa6b9cf5fceacbfb090748431da267fc03b Mon Sep 17 00:00:00 2001 From: jrconlin Date: Thu, 27 Apr 2023 10:42:22 -0700 Subject: [PATCH 6/6] f r's --- autopush-common/src/db/dynamodb/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index 6f20813fe..a2ac9d6b8 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -511,7 +511,7 @@ impl DbClient for DdbClientImpl { } async fn save_message(&self, uaid: &Uuid, message: Notification) -> DbResult<()> { - let topic = message.topic.clone().is_some().to_string(); + let topic = message.topic.is_some().to_string(); let input = PutItemInput { item: serde_dynamodb::to_hashmap(&NotificationRecord::from_notif(uaid, message))?, table_name: self.settings.message_table.clone(),