From 6472129d59c9c7565635c36436566ff038829bdc Mon Sep 17 00:00:00 2001 From: Jordan Gonzalez <30836115+duncanista@users.noreply.github.com> Date: Wed, 4 Feb 2026 17:35:54 -0500 Subject: [PATCH 1/2] refactor(flusher): use &self instead of &mut self for flush methods The Flusher methods (get_dd_api, flush, flush_metrics) previously required &mut self, forcing callers to wrap the flusher in a Mutex. Since tokio::sync::OnceCell::get_or_init() only requires &self, we can change all methods to use &self, enabling lock-free usage. Also fixes test assertions: 404 is a permanent error (4xx) that should not be retried, so flush_metrics correctly returns None. --- crates/datadog-serverless-compat/src/main.rs | 4 ++-- crates/dogstatsd/src/flusher.rs | 22 +++++++++++--------- crates/dogstatsd/tests/integration_test.rs | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/crates/datadog-serverless-compat/src/main.rs b/crates/datadog-serverless-compat/src/main.rs index f763627..a13e753 100644 --- a/crates/datadog-serverless-compat/src/main.rs +++ b/crates/datadog-serverless-compat/src/main.rs @@ -143,7 +143,7 @@ pub async fn main() { } }); - let (mut metrics_flusher, _aggregator_handle) = if dd_use_dogstatsd { + let (metrics_flusher, _aggregator_handle) = if dd_use_dogstatsd { debug!("Starting dogstatsd"); let (_, metrics_flusher, aggregator_handle) = start_dogstatsd( dd_dogstatsd_port, @@ -167,7 +167,7 @@ pub async fn main() { loop { flush_interval.tick().await; - if let Some(metrics_flusher) = metrics_flusher.as_mut() { + if let Some(metrics_flusher) = metrics_flusher.as_ref() { debug!("Flushing dogstatsd metrics"); metrics_flusher.flush().await; } diff --git a/crates/dogstatsd/src/flusher.rs b/crates/dogstatsd/src/flusher.rs index 0de79c1..03da626 100644 --- a/crates/dogstatsd/src/flusher.rs +++ b/crates/dogstatsd/src/flusher.rs @@ -51,7 +51,7 @@ impl Flusher { } } - async fn get_dd_api(&mut self) -> &Option { + async fn get_dd_api(&self) -> &Option { self.dd_api .get_or_init(|| async { let api_key = self.api_key_factory.get_api_key().await; @@ -76,7 +76,7 @@ impl Flusher { /// Flush metrics from the aggregator pub async fn flush( - &mut self, + &self, ) -> Option<( Vec, Vec, @@ -96,7 +96,7 @@ impl Flusher { /// Flush given batch of metrics pub async fn flush_metrics( - &mut self, + &self, series: Vec, distributions: Vec, ) -> Option<( @@ -272,7 +272,7 @@ mod tests { None, ); - let mut flusher = Flusher::new(FlusherConfig { + let flusher = Flusher::new(FlusherConfig { api_key_factory: Arc::new(api_key_factory), aggregator_handle: handle, metrics_intake_url_prefix: MetricsIntakeUrlPrefix::new( @@ -319,7 +319,7 @@ mod tests { let api_key_factory = ApiKeyFactory::new("test-api-key"); - let mut flusher = Flusher::new(FlusherConfig { + let flusher = Flusher::new(FlusherConfig { api_key_factory: Arc::new(api_key_factory), aggregator_handle: handle, metrics_intake_url_prefix: MetricsIntakeUrlPrefix::new( @@ -351,8 +351,9 @@ mod tests { // Test with series but empty distributions let result = flusher.flush_metrics(series, Vec::new()).await; - // Should attempt to flush and return Some with failed metrics (since we're not mocking the API) - assert!(result.is_some()); + // The request will fail with 404 (no mock server), which is a permanent 4xx error. + // Permanent errors are not retried, so None is returned (success from retry perspective). + assert!(result.is_none()); } #[tokio::test] @@ -369,7 +370,7 @@ mod tests { let api_key_factory = ApiKeyFactory::new("test-api-key"); - let mut flusher = Flusher::new(FlusherConfig { + let flusher = Flusher::new(FlusherConfig { api_key_factory: Arc::new(api_key_factory), aggregator_handle: handle, metrics_intake_url_prefix: MetricsIntakeUrlPrefix::new( @@ -398,7 +399,8 @@ mod tests { // Test with distributions but empty series let result = flusher.flush_metrics(Vec::new(), distributions).await; - // Should attempt to flush and return Some with failed metrics (since we're not mocking the API) - assert!(result.is_some()); + // The request will fail with 404 (no mock server), which is a permanent 4xx error. + // Permanent errors are not retried, so None is returned (success from retry perspective). + assert!(result.is_none()); } } diff --git a/crates/dogstatsd/tests/integration_test.rs b/crates/dogstatsd/tests/integration_test.rs index 65d919c..9824a63 100644 --- a/crates/dogstatsd/tests/integration_test.rs +++ b/crates/dogstatsd/tests/integration_test.rs @@ -47,7 +47,7 @@ async fn dogstatsd_server_ships_series() { let api_key_factory = ApiKeyFactory::new("mock-api-key"); - let mut metrics_flusher = Flusher::new(FlusherConfig { + let metrics_flusher = Flusher::new(FlusherConfig { api_key_factory: Arc::new(api_key_factory), aggregator_handle: handle.clone(), metrics_intake_url_prefix: MetricsIntakeUrlPrefix::new( From a71be1be4e636cbdef0dde557e7328edf274210a Mon Sep 17 00:00:00 2001 From: Jordan Gonzalez <30836115+duncanista@users.noreply.github.com> Date: Wed, 4 Feb 2026 22:21:44 -0500 Subject: [PATCH 2/2] revert tests update --- crates/dogstatsd/src/flusher.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/dogstatsd/src/flusher.rs b/crates/dogstatsd/src/flusher.rs index 03da626..e4fbe4b 100644 --- a/crates/dogstatsd/src/flusher.rs +++ b/crates/dogstatsd/src/flusher.rs @@ -351,9 +351,8 @@ mod tests { // Test with series but empty distributions let result = flusher.flush_metrics(series, Vec::new()).await; - // The request will fail with 404 (no mock server), which is a permanent 4xx error. - // Permanent errors are not retried, so None is returned (success from retry perspective). - assert!(result.is_none()); + // Should attempt to flush and return Some with failed metrics (since we're not mocking the API) + assert!(result.is_some()); } #[tokio::test] @@ -399,8 +398,7 @@ mod tests { // Test with distributions but empty series let result = flusher.flush_metrics(Vec::new(), distributions).await; - // The request will fail with 404 (no mock server), which is a permanent 4xx error. - // Permanent errors are not retried, so None is returned (success from retry perspective). - assert!(result.is_none()); + // Should attempt to flush and return Some with failed metrics (since we're not mocking the API) + assert!(result.is_some()); } }