From 940e0ea0b195a11271037536f0d348bd260cc51e Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Mon, 29 Apr 2024 17:07:55 -0700 Subject: [PATCH] feat: remove the unused metrics module cleanup a couple TODOs: return WS's protocol error code for actix-ws ProtocolErrors Closes SYNC-4249 --- autoconnect/autoconnect-web/src/dockerflow.rs | 14 +- autoconnect/autoconnect-web/src/lib.rs | 1 - autoconnect/autoconnect-web/src/metrics.rs | 182 ------------------ .../autoconnect-ws-sm/src/error.rs | 3 - autoconnect/autoconnect-ws/src/error.rs | 3 +- 5 files changed, 4 insertions(+), 199 deletions(-) delete mode 100644 autoconnect/autoconnect-web/src/metrics.rs diff --git a/autoconnect/autoconnect-web/src/dockerflow.rs b/autoconnect/autoconnect-web/src/dockerflow.rs index b886a9fb7..1bbcb245f 100644 --- a/autoconnect/autoconnect-web/src/dockerflow.rs +++ b/autoconnect/autoconnect-web/src/dockerflow.rs @@ -26,10 +26,8 @@ pub fn config(config: &mut web::ServiceConfig) { } /// Handle the `/health` and `/__heartbeat__` routes -// note, this changes to `blocks_in_conditions` for 1.76+ -#[allow(clippy::blocks_in_conditions)] pub async fn health_route(state: Data) -> Json { - let status = if state + let healthy = state .db .health_check() .await @@ -37,15 +35,9 @@ pub async fn health_route(state: Data) -> Json { error!("Autoconnect Health Error: {:?}", e); e }) - .is_ok() - { - "OK" - } else { - "ERROR" - }; - //TODO: query local state and report results + .is_ok(); Json(json!({ - "status": status, + "status": if healthy { "OK" } else { "ERROR" }, "version": env!("CARGO_PKG_VERSION"), })) } diff --git a/autoconnect/autoconnect-web/src/lib.rs b/autoconnect/autoconnect-web/src/lib.rs index 9cc2be767..5e74bcc57 100644 --- a/autoconnect/autoconnect-web/src/lib.rs +++ b/autoconnect/autoconnect-web/src/lib.rs @@ -7,7 +7,6 @@ extern crate slog_scope; pub mod dockerflow; pub mod error; -pub mod metrics; pub mod routes; #[cfg(test)] mod test; diff --git a/autoconnect/autoconnect-web/src/metrics.rs b/autoconnect/autoconnect-web/src/metrics.rs deleted file mode 100644 index 8590060df..000000000 --- a/autoconnect/autoconnect-web/src/metrics.rs +++ /dev/null @@ -1,182 +0,0 @@ -// TODO: Convert autopush-common::metrics to this? - -use std::net::UdpSocket; -use std::sync::Arc; -use std::time::Instant; - -use actix_web::{web::Data, HttpRequest}; -use cadence::{ - BufferedUdpMetricSink, CountedExt, Metric, MetricError, NopMetricSink, QueuingMetricSink, - StatsdClient, Timed, -}; - -use actix_web::HttpMessage; -use autoconnect_settings::{AppState, Settings}; -use autopush_common::tags::Tags; - -#[derive(Debug, Clone)] -pub struct MetricTimer { - pub label: String, - pub start: Instant, - pub tags: Tags, -} - -#[derive(Debug, Clone)] -pub struct Metrics { - client: Option>, - timer: Option, - tags: Option, -} - -impl Drop for Metrics { - fn drop(&mut self) { - let tags = self.tags.clone().unwrap_or_default(); - if let Some(client) = self.client.as_ref() { - if let Some(timer) = self.timer.as_ref() { - let lapse = (Instant::now() - timer.start).as_millis() as u64; - trace!( - "⌚ Ending timer at nanos: {:?} : {:?}", - &timer.label, - lapse; - tags - ); - let mut tagged = client.time_with_tags(&timer.label, lapse); - // Include any "hard coded" tags. - // tagged = tagged.with_tag("version", env!("CARGO_PKG_VERSION")); - let tags = timer.tags.tags.clone(); - let keys = tags.keys(); - for tag in keys { - tagged = tagged.with_tag(tag, tags.get(tag).unwrap()) - } - match tagged.try_send() { - Err(e) => { - // eat the metric, but log the error - warn!("⚠️ Metric {} error: {:?} ", &timer.label, e); - } - Ok(v) => { - trace!("⌚ {:?}", v.as_metric_str()); - } - } - } - } - } -} - -impl From<&HttpRequest> for Metrics { - fn from(req: &HttpRequest) -> Self { - let exts = req.extensions(); - let def_tags = Tags::from_request_head(req.head()); - let tags = exts.get::().unwrap_or(&def_tags); - Metrics { - client: Some(metrics_from_req(req)), - tags: Some(tags.clone()), - timer: None, - } - } -} - -impl From for Metrics { - fn from(client: StatsdClient) -> Self { - Metrics { - client: Some(Arc::new(client)), - tags: None, - timer: None, - } - } -} - -impl From<&actix_web::web::Data> for Metrics { - fn from(state: &actix_web::web::Data) -> Self { - Metrics { - client: Some(state.metrics.clone()), - tags: None, - timer: None, - } - } -} - -impl Metrics { - #![allow(unused)] // TODO: Start using metrics - - pub fn sink() -> StatsdClient { - StatsdClient::builder("", NopMetricSink).build() - } - - pub fn noop() -> Self { - Self { - client: Some(Arc::new(Self::sink())), - timer: None, - tags: None, - } - } - - pub fn start_timer(&mut self, label: &str, tags: Option) { - let mut mtags = self.tags.clone().unwrap_or_default(); - if let Some(t) = tags { - mtags.extend(t.tags) - } - - trace!("⌚ Starting timer... {:?}", &label; &mtags); - self.timer = Some(MetricTimer { - label: label.to_owned(), - start: Instant::now(), - tags: mtags, - }); - } - - // increment a counter with no tags data. - pub fn incr(self, label: &str) { - self.incr_with_tags(label, None) - } - - pub fn incr_with_tags(self, label: &str, tags: Option) { - if let Some(client) = self.client.as_ref() { - let mut tagged = client.incr_with_tags(label); - let mut mtags = self.tags.clone().unwrap_or_default(); - if let Some(t) = tags { - mtags.tags.extend(t.tags) - } - let tag_keys = mtags.tags.keys(); - for key in tag_keys.clone() { - // REALLY wants a static here, or at least a well defined ref. - tagged = tagged.with_tag(key, mtags.tags.get(key).unwrap()); - } - // Include any "hard coded" tags. - // incr = incr.with_tag("version", env!("CARGO_PKG_VERSION")); - match tagged.try_send() { - Err(e) => { - // eat the metric, but log the error - warn!("⚠️ Metric {} error: {:?}", label, e; mtags); - } - Ok(v) => trace!("☑️ {:?}", v.as_metric_str()), - } - } - } -} - -pub fn metrics_from_req(req: &HttpRequest) -> Arc { - req.app_data::>() - .expect("Could not get state in metrics_from_req") - .metrics - .clone() -} - -/// Create a cadence StatsdClient from the given options -pub fn metrics_from_settings(settings: &Settings) -> Result { - let builder = if let Some(statsd_host) = settings.statsd_host.as_ref() { - let socket = UdpSocket::bind("0.0.0.0:0")?; - socket.set_nonblocking(true)?; - - let host = (statsd_host.as_str(), settings.statsd_port); - let udp_sink = BufferedUdpMetricSink::from(host, socket)?; - let sink = QueuingMetricSink::from(udp_sink); - StatsdClient::builder(settings.statsd_label.as_ref(), sink) - } else { - StatsdClient::builder(settings.statsd_label.as_ref(), NopMetricSink) - }; - Ok(builder - .with_error_handler(|err| { - warn!("⚠️ Metric send error: {:?}", err); - }) - .build()) -} diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs index 815723205..c4f9becf2 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs @@ -40,8 +40,6 @@ where impl SMError { pub fn close_code(&self) -> actix_ws::CloseCode { match self.kind { - // TODO: applicable here? - //SMErrorKind::InvalidMessage(_) => CloseCode::Invalid, SMErrorKind::UaidReset => CloseCode::Normal, _ => CloseCode::Error, } @@ -70,7 +68,6 @@ impl ReportableError for SMError { } fn metric_label(&self) -> Option<&'static str> { - // TODO: match &self.kind { SMErrorKind::Database(e) => e.metric_label(), SMErrorKind::MakeEndpoint(e) => e.metric_label(), diff --git a/autoconnect/autoconnect-ws/src/error.rs b/autoconnect/autoconnect-ws/src/error.rs index f62b44ed3..a914d8cdd 100644 --- a/autoconnect/autoconnect-ws/src/error.rs +++ b/autoconnect/autoconnect-ws/src/error.rs @@ -43,8 +43,7 @@ impl WSError { pub fn close_code(&self) -> actix_ws::CloseCode { match &self.kind { WSErrorKind::SM(e) => e.close_code(), - // TODO: applicable here? - //WSErrorKind::Protocol(_) => CloseCode::Protocol, + WSErrorKind::Protocol(_) => CloseCode::Protocol, WSErrorKind::UnsupportedMessage(_) => CloseCode::Unsupported, _ => CloseCode::Error, }