From 67186854025a93c8868c202910431d1d645bbe82 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Mon, 25 Sep 2023 15:35:57 -0700 Subject: [PATCH] feat: emit a tag in autoconnect's metrics (#435) to distinguish its metrics from the legacy version and consolidate autoendpoint's metrics builder w/ autopush_common's SYNC-3893 --- .../autoconnect-settings/src/app_state.rs | 16 ++++--- autoconnect/autoconnect-settings/src/lib.rs | 3 +- autoconnect/autoconnect-web/src/error.rs | 5 +- autoendpoint/src/metrics.rs | 48 ++++++------------- autopush-common/src/metrics.rs | 31 ++++++------ 5 files changed, 45 insertions(+), 58 deletions(-) diff --git a/autoconnect/autoconnect-settings/src/app_state.rs b/autoconnect/autoconnect-settings/src/app_state.rs index 16b8a05d..139f39d2 100644 --- a/autoconnect/autoconnect-settings/src/app_state.rs +++ b/autoconnect/autoconnect-settings/src/app_state.rs @@ -11,10 +11,7 @@ use autoconnect_common::{ registry::ClientRegistry, }; use autopush_common::db::{client::DbClient, dynamodb::DdbClientImpl, DbSettings, StorageType}; -use autopush_common::{ - errors::{ApcErrorKind, Result}, - metrics::new_metrics, -}; +use autopush_common::errors::{ApcErrorKind, Result}; use crate::{Settings, ENV_PREFIX}; @@ -59,10 +56,15 @@ impl AppState { }) .collect(); let fernet = MultiFernet::new(fernets); - let metrics = Arc::new(new_metrics( - settings.statsd_host.clone(), + let metrics = autopush_common::metrics::builder( + &settings.statsd_label, + &settings.statsd_host, settings.statsd_port, - )?); + )? + // Temporary tag to distinguish from the legacy autopush(connect) + .with_tag("autoconnect", "true") + .build(); + let metrics = Arc::new(metrics); let db_settings = DbSettings { dsn: settings.db_dsn.clone(), diff --git a/autoconnect/autoconnect-settings/src/lib.rs b/autoconnect/autoconnect-settings/src/lib.rs index 90266274..a2164f3f 100644 --- a/autoconnect/autoconnect-settings/src/lib.rs +++ b/autoconnect/autoconnect-settings/src/lib.rs @@ -122,7 +122,8 @@ impl Default for Settings { endpoint_port: 8082, crypto_key: format!("[{}]", Fernet::generate_key()), statsd_host: Some("localhost".to_owned()), - statsd_label: ENV_PREFIX.to_owned(), + // Matches the legacy value + statsd_label: "autopush".to_owned(), statsd_port: 8125, db_dsn: None, db_settings: "".to_owned(), diff --git a/autoconnect/autoconnect-web/src/error.rs b/autoconnect/autoconnect-web/src/error.rs index aee27862..09929a74 100644 --- a/autoconnect/autoconnect-web/src/error.rs +++ b/autoconnect/autoconnect-web/src/error.rs @@ -24,8 +24,9 @@ impl ResponseError for ApiError { } fn error_response(&self) -> HttpResponse { - HttpResponse::build(self.status_code()).json(json!({ - "code": self.status_code().as_u16(), + let code = self.status_code(); + HttpResponse::build(code).json(json!({ + "code": code.as_u16(), "errno": self.errno(), "error": self.to_string(), })) diff --git a/autoendpoint/src/metrics.rs b/autoendpoint/src/metrics.rs index c3b0fcb0..edd328bf 100644 --- a/autoendpoint/src/metrics.rs +++ b/autoendpoint/src/metrics.rs @@ -1,22 +1,13 @@ -// TODO: Consolidate with autopush_common::metrics - -use std::net::UdpSocket; -use std::sync::Arc; -use std::time::Instant; - -use actix_web::{web::Data, FromRequest, HttpMessage, HttpRequest}; -use cadence::{ - BufferedUdpMetricSink, CountedExt, Metric, MetricError, NopMetricSink, QueuingMetricSink, - StatsdClient, Timed, -}; - -use crate::error::ApiError; -use crate::server::AppState; -use crate::settings::Settings; -use actix_web::dev::Payload; -use autopush_common::tags::Tags; +use std::{sync::Arc, time::Instant}; + +use actix_web::{dev::Payload, web::Data, FromRequest, HttpMessage, HttpRequest}; +use cadence::{CountedExt, Metric, MetricError, NopMetricSink, StatsdClient, Timed}; use futures::future; +use autopush_common::tags::Tags; + +use crate::{error::ApiError, server::AppState, settings::Settings}; + #[derive(Debug, Clone)] pub struct MetricTimer { pub label: String, @@ -175,20 +166,11 @@ pub fn metrics_from_req(req: &HttpRequest) -> Arc { /// 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()) + let client = autopush_common::metrics::builder( + &settings.statsd_label, + &settings.statsd_host, + settings.statsd_port, + )? + .build(); + Ok(client) } diff --git a/autopush-common/src/metrics.rs b/autopush-common/src/metrics.rs index 833e9bd7..679c6288 100644 --- a/autopush-common/src/metrics.rs +++ b/autopush-common/src/metrics.rs @@ -1,26 +1,27 @@ //! Metrics tie-ins -// TODO: consolidate this with autoendpoint::metrics - use std::net::UdpSocket; -use cadence::{BufferedUdpMetricSink, NopMetricSink, QueuingMetricSink, StatsdClient}; - -use crate::errors::Result; +use cadence::{ + BufferedUdpMetricSink, MetricError, NopMetricSink, QueuingMetricSink, StatsdClient, + StatsdClientBuilder, +}; -/// Create a cadence StatsdClient from the given options -pub fn new_metrics(host: Option, port: u16) -> Result { - let builder = if let Some(statsd_host) = host.as_ref() { +/// Create a cadence StatsdClientBuilder from the given options +pub fn builder( + prefix: &str, + host: &Option, + port: u16, +) -> Result { + let builder = if let Some(host) = host { let socket = UdpSocket::bind("0.0.0.0:0")?; socket.set_nonblocking(true)?; - let host = (statsd_host.as_str(), port); - let udp_sink = BufferedUdpMetricSink::from(host, socket)?; + let addr = (host.as_str(), port); + let udp_sink = BufferedUdpMetricSink::from(addr, socket)?; let sink = QueuingMetricSink::from(udp_sink); - StatsdClient::builder("autopush", sink) + StatsdClient::builder(prefix, sink) } else { - StatsdClient::builder("autopush", NopMetricSink) + StatsdClient::builder(prefix, NopMetricSink) }; - Ok(builder - .with_error_handler(|err| error!("Metrics send error: {}", err)) - .build()) + Ok(builder.with_error_handler(|err| warn!("⚠️ Metric send error: {:?}", err))) }