From e918f350552f4175a2cd778f330e8a768b83ba8c Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 29 Jan 2024 13:52:37 +0100 Subject: [PATCH] Clean up Metrics usage a bit Instead of defining a custom wrapper type to add global tags, use the built in cadence functionality to do so. --- crates/symbolicator-service/src/metrics.rs | 133 ++++++------------ .../symbolicator-service/src/utils/futures.rs | 14 +- 2 files changed, 52 insertions(+), 95 deletions(-) diff --git a/crates/symbolicator-service/src/metrics.rs b/crates/symbolicator-service/src/metrics.rs index aecf05b06..0283f5db3 100644 --- a/crates/symbolicator-service/src/metrics.rs +++ b/crates/symbolicator-service/src/metrics.rs @@ -1,52 +1,17 @@ //! Provides access to the metrics sytem. use std::collections::BTreeMap; use std::net::ToSocketAddrs; -use std::ops::Deref; use std::sync::OnceLock; -use cadence::{Metric, MetricBuilder, StatsdClient, UdpMetricSink}; +use cadence::{StatsdClient, UdpMetricSink}; -static METRICS_CLIENT: OnceLock = OnceLock::new(); - -thread_local! { - static CURRENT_CLIENT: Option<&'static MetricsClient> = METRICS_CLIENT.get(); -} +static METRICS_CLIENT: OnceLock = OnceLock::new(); /// The metrics prelude that is necessary to use the client. pub mod prelude { pub use cadence::prelude::*; } -#[derive(Debug)] -pub struct MetricsClient { - /// The raw statsd client. - pub statsd_client: StatsdClient, - - /// A collection of tags and values that will be sent with every metric. - tags: BTreeMap, -} - -impl MetricsClient { - #[inline(always)] - pub fn send_metric<'a, T>(&'a self, mut metric: MetricBuilder<'a, '_, T>) - where - T: Metric + From, - { - for (tag, value) in self.tags.iter() { - metric = metric.with_tag(tag, value); - } - metric.send() - } -} - -impl Deref for MetricsClient { - type Target = StatsdClient; - - fn deref(&self) -> &Self::Target { - &self.statsd_client - } -} - /// Tell the metrics system to report to statsd. pub fn configure_statsd(prefix: &str, host: A, tags: BTreeMap) { let addrs: Vec<_> = host.to_socket_addrs().unwrap().collect(); @@ -56,33 +21,27 @@ pub fn configure_statsd(prefix: &str, host: A, tags: BTreeMap< let socket = std::net::UdpSocket::bind("0.0.0.0:0").unwrap(); socket.set_nonblocking(true).unwrap(); let sink = UdpMetricSink::from(&addrs[..], socket).unwrap(); - let statsd_client = StatsdClient::from_sink(prefix, sink); + let mut builder = StatsdClient::builder(prefix, sink); + for (key, value) in tags { + builder = builder.with_tag(key, value) + } + let client = builder.build(); - METRICS_CLIENT - .set(MetricsClient { - statsd_client, - tags, - }) - .unwrap(); + METRICS_CLIENT.set(client).unwrap(); } -/// Invoke a callback with the current statsd client. +/// Invoke a callback with the current [`StatsdClient`]. /// -/// If statsd is not configured the callback is not invoked. For the most part -/// the [`metric!`](crate::metric) macro should be used instead. +/// If no [`StatsdClient`] is configured the callback is not invoked. +/// For the most part the [`metric!`](crate::metric) macro should be used instead. #[inline(always)] -pub fn with_client(f: F) -> R +pub fn with_client(f: F) where - F: FnOnce(&MetricsClient) -> R, - R: Default, + F: FnOnce(&StatsdClient), { - CURRENT_CLIENT.with(|client| { - if let Some(client) = client { - f(client) - } else { - Default::default() - } - }) + if let Some(client) = METRICS_CLIENT.get() { + f(client) + } } /// Emits a metric. @@ -92,63 +51,63 @@ macro_rules! metric { (counter($id:expr) += $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.count_with_tags($id, $value) - $(.with_tag($k, $v))* - ); - }) + client + .count_with_tags($id, $value) + $(.with_tag($k, $v))* + .send(); + }); }}; (counter($id:expr) -= $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.count_with_tags($id, -$value) - $(.with_tag($k, $v))* - ); - }) + client + .count_with_tags($id, -$value) + $(.with_tag($k, $v))* + .send(); + }); }}; // gauges (gauge($id:expr) = $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.gauge_with_tags($id, $value) - $(.with_tag($k, $v))* - ); - }) + client + .gauge_with_tags($id, $value) + $(.with_tag($k, $v))* + .send(); + }); }}; // timers (timer($id:expr) = $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.time_with_tags($id, $value) - $(.with_tag($k, $v))* - ); - }) + client + .time_with_tags($id, $value) + $(.with_tag($k, $v))* + .send(); + }); }}; // we use statsd timers to send things such as filesizes as well. (time_raw($id:expr) = $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.time_with_tags($id, $value) - $(.with_tag($k, $v))* - ); - }) + client + .time_with_tags($id, $value) + $(.with_tag($k, $v))* + .send(); + }); }}; // histograms (histogram($id:expr) = $value:expr $(, $k:expr => $v:expr)* $(,)?) => {{ use $crate::metrics::prelude::*; $crate::metrics::with_client(|client| { - client.send_metric( - client.histogram_with_tags($id, $value) - $(.with_tag($k, $v))* - ); - }) + client + .histogram_with_tags($id, $value) + $(.with_tag($k, $v))* + .send(); + }); }}; } diff --git a/crates/symbolicator-service/src/utils/futures.rs b/crates/symbolicator-service/src/utils/futures.rs index 7606a89ef..391dfe0b2 100644 --- a/crates/symbolicator-service/src/utils/futures.rs +++ b/crates/symbolicator-service/src/utils/futures.rs @@ -101,11 +101,10 @@ impl<'a> MeasureGuard<'a> { /// metric. pub fn start(&mut self) { metrics::with_client(|client| { - let metric = client + client .time_with_tags("futures.wait_time", self.creation_time.elapsed()) - .with_tag("task_name", self.task_name); - - client.send_metric(metric); + .with_tag("task_name", self.task_name) + .send(); }) } @@ -122,12 +121,11 @@ impl Drop for MeasureGuard<'_> { MeasureState::Done(status) => status, }; metrics::with_client(|client| { - let metric = client + client .time_with_tags("futures.done", self.creation_time.elapsed()) .with_tag("task_name", self.task_name) - .with_tag("status", status); - - client.send_metric(metric); + .with_tag("status", status) + .send(); }) } }