fix(config): trim trailing slashes from DD URL environment variables

bottlecap/src/bin/bottlecap/main.rs

@@ -53,6 +53,7 @@ use bottlecap::{
         flusher::LogsFlusher,
         lambda::DurableContextUpdate,
     },
+    metrics::intake::{additional_endpoints, metrics_intake_url},
     otlp::{agent::Agent as OtlpAgent, should_enable_otlp_agent},
     proxy::{interceptor, should_start_proxy},
     secrets::decrypt,
@@ -87,10 +88,7 @@ use dogstatsd::{
     },
     api_key::ApiKeyFactory,
     constants::CONTEXTS,
-    datadog::{
-        DdDdUrl, DdUrl, MetricsIntakeUrlPrefix, MetricsIntakeUrlPrefixOverride,
-        RetryStrategy as DsdRetryStrategy, Site as MetricsSite,
-    },
+    datadog::RetryStrategy as DsdRetryStrategy,
     dogstatsd::{DogStatsD, DogStatsDConfig},
     flusher::{Flusher as MetricsFlusher, FlusherConfig as MetricsFlusherConfig},
     metric::{EMPTY_TAGS, SortedTags},
@@ -1295,60 +1293,28 @@ fn start_metrics_flushers(
 ) -> Vec<MetricsFlusher> {
     let mut flushers = Vec::new();
 
-    let metrics_intake_url = if !config.dd_url.is_empty() {
-        let dd_dd_url = DdDdUrl::new(config.dd_url.clone()).expect("can't parse DD_DD_URL");
-
-        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(None, Some(dd_dd_url));
-        MetricsIntakeUrlPrefix::new(None, prefix_override)
-    } else if !config.url.is_empty() {
-        let dd_url = DdUrl::new(config.url.clone()).expect("can't parse DD_URL");
-
-        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(Some(dd_url), None);
-        MetricsIntakeUrlPrefix::new(None, prefix_override)
-    } else {
-        // use site
-        let metrics_site = MetricsSite::new(config.site.clone()).expect("can't parse site");
-        MetricsIntakeUrlPrefix::new(Some(metrics_site), None)
-    };
+    let metrics_intake_url = metrics_intake_url(config);
 
     let flusher_config = MetricsFlusherConfig {
         api_key_factory,
         aggregator_handle: metrics_aggr_handle.clone(),
-        metrics_intake_url_prefix: metrics_intake_url.expect("can't parse site or override"),
+        metrics_intake_url_prefix: metrics_intake_url,
         client: client.clone(),
         retry_strategy: DsdRetryStrategy::Immediate(3),
         compression_level: config.metrics_config_compression_level,
     };
     flushers.push(MetricsFlusher::new(flusher_config));
 
-    for (endpoint_url, api_keys) in &config.additional_endpoints {
-        let dd_url = match DdUrl::new(endpoint_url.clone()) {
-            Ok(url) => url,
-            Err(err) => {
-                error!(
-                    "Invalid additional endpoint: {err}. Falling back to 'https://app.datadoghq.com'"
-                );
-                DdUrl::new("https://app.datadoghq.com".to_string())
-                    .expect("additional endpoint fallback URL is invalid")
-            }
+    for endpoint in additional_endpoints(config) {
+        let additional_flusher_config = MetricsFlusherConfig {
+            api_key_factory: Arc::new(ApiKeyFactory::new(&endpoint.api_key)),
+            aggregator_handle: metrics_aggr_handle.clone(),
+            metrics_intake_url_prefix: endpoint.intake_url,
+            client: client.clone(),
+            retry_strategy: DsdRetryStrategy::Immediate(3),
+            compression_level: config.metrics_config_compression_level,
         };
-        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(Some(dd_url), None);
-        let metrics_intake_url = MetricsIntakeUrlPrefix::new(None, prefix_override)
-            .expect("can't parse additional endpoint URL");
-
-        // Create a flusher for each endpoint URL and API key pair
-        for api_key in api_keys {
-            let additional_api_key_factory = Arc::new(ApiKeyFactory::new(api_key));
-            let additional_flusher_config = MetricsFlusherConfig {
-                api_key_factory: additional_api_key_factory,
-                aggregator_handle: metrics_aggr_handle.clone(),
-                metrics_intake_url_prefix: metrics_intake_url.clone(),
-                client: client.clone(),
-                retry_strategy: DsdRetryStrategy::Immediate(3),
-                compression_level: config.metrics_config_compression_level,
-            };
-            flushers.push(MetricsFlusher::new(additional_flusher_config));
-        }
+        flushers.push(MetricsFlusher::new(additional_flusher_config));
     }
     flushers
 }

bottlecap/src/config/mod.rs

@@ -216,6 +216,9 @@ impl ConfigBuilder {
                 .clone_from(&self.config.trace_propagation_style);
         }
 
+        self.config.dd_url = trim_url(&self.config.dd_url);
+        self.config.url = trim_url(&self.config.url);
+
         // If Logs URL is not set, set it to the default
         if self.config.logs_config_logs_dd_url.trim().is_empty() {
             self.config.logs_config_logs_dd_url = build_fqdn_logs(self.config.site.clone());
@@ -229,7 +232,7 @@ impl ConfigBuilder {
             self.config.apm_dd_url = trace_intake_url(self.config.site.clone().as_str());
         } else {
             // If APM URL is set, add the site to the URL
-            self.config.apm_dd_url = trace_intake_url_prefixed(self.config.apm_dd_url.as_str());
+            self.config.apm_dd_url = trace_intake_url_prefixed(&trim_url(&self.config.apm_dd_url));
         }
 
         self.config.clone()
@@ -552,6 +555,10 @@ fn build_fqdn_logs(site: String) -> String {
     format!("https://http-intake.logs.{site}")
 }
 
+fn trim_url(url: &str) -> String {
+    url.trim_end_matches('/').to_owned()
+}
+
 #[inline]
 #[must_use]
 fn logs_intake_url(url: &str) -> String {
@@ -953,6 +960,24 @@ pub mod tests {
         });
     }
 
+    #[test]
+    fn test_support_apm_dd_url_untrimmed() {
+        figment::Jail::expect_with(|jail| {
+            jail.clear_env();
+            jail.set_env(
+                "DD_APM_DD_URL",
+                "https://dr-test-failover-trace.agent.datadoghq.com/",
+            );
+
+            let config = get_config(Path::new(""));
+            assert_eq!(
+                config.apm_dd_url,
+                "https://dr-test-failover-trace.agent.datadoghq.com/api/v0.2/traces".to_string()
+            );
+            Ok(())
+        });
+    }
+
     #[test]
     fn test_support_dd_dd_url() {
         figment::Jail::expect_with(|jail| {
@@ -965,6 +990,21 @@ pub mod tests {
         });
     }
 
+    #[test]
+    fn test_support_dd_dd_url_untrimmed() {
+        figment::Jail::expect_with(|jail| {
+            jail.clear_env();
+            jail.set_env("DD_DD_URL", "https://dr-test-failover.agent.datadoghq.com/");
+
+            let config = get_config(Path::new(""));
+            assert_eq!(
+                config.dd_url,
+                "https://dr-test-failover.agent.datadoghq.com".to_string()
+            );
+            Ok(())
+        });
+    }
+
     #[test]
     fn test_support_dd_url() {
         figment::Jail::expect_with(|jail| {
@@ -977,6 +1017,21 @@ pub mod tests {
         });
     }
 
+    #[test]
+    fn test_support_dd_url_untrimmed() {
+        figment::Jail::expect_with(|jail| {
+            jail.clear_env();
+            jail.set_env("DD_URL", "https://dr-test-failover.agent.datadoghq.com/");
+
+            let config = get_config(Path::new(""));
+            assert_eq!(
+                config.url,
+                "https://dr-test-failover.agent.datadoghq.com".to_string()
+            );
+            Ok(())
+        });
+    }
+
     #[test]
     fn test_dd_dd_url_default() {
         figment::Jail::expect_with(|jail| {

bottlecap/src/metrics/intake.rs

@@ -0,0 +1,146 @@
+use crate::config::Config;
+use dogstatsd::datadog::{
+    DdDdUrl, DdUrl, MetricsIntakeUrlPrefix, MetricsIntakeUrlPrefixOverride, Site as MetricsSite,
+};
+use tracing::error;
+
+#[must_use]
+pub fn metrics_intake_url(config: &Config) -> MetricsIntakeUrlPrefix {
+    if !config.dd_url.is_empty() {
+        let dd_dd_url = DdDdUrl::new(config.dd_url.clone()).expect("can't parse DD_DD_URL");
+        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(None, Some(dd_dd_url));
+        MetricsIntakeUrlPrefix::new(None, prefix_override)
+    } else if !config.url.is_empty() {
+        let dd_url = DdUrl::new(config.url.clone()).expect("can't parse DD_URL");
+        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(Some(dd_url), None);
+        MetricsIntakeUrlPrefix::new(None, prefix_override)
+    } else {
+        let metrics_site = MetricsSite::new(config.site.clone()).expect("can't parse site");
+        MetricsIntakeUrlPrefix::new(Some(metrics_site), None)
+    }
+    .expect("can't parse site or override")
+}
+
+pub struct MetricsEndpoint {
+    pub intake_url: MetricsIntakeUrlPrefix,
+    pub api_key: String,
+}
+
+pub fn additional_endpoints(config: &Config) -> Vec<MetricsEndpoint> {
+    let mut result = Vec::new();
+    for (endpoint_url, api_keys) in &config.additional_endpoints {
+        let dd_url = match DdUrl::new(trim_url(endpoint_url)) {
+            Ok(url) => url,
+            Err(err) => {
+                error!(
+                    "Invalid additional endpoint: {err}. Falling back to 'https://app.datadoghq.com'"
+                );
+                DdUrl::new("https://app.datadoghq.com".to_string())
+                    .expect("additional endpoint fallback URL is invalid")
+            }
+        };
+        let prefix_override = MetricsIntakeUrlPrefixOverride::maybe_new(Some(dd_url), None);
+        let intake_url = MetricsIntakeUrlPrefix::new(None, prefix_override)
+            .expect("can't parse additional endpoint URL");
+        for api_key in api_keys {
+            result.push(MetricsEndpoint {
+                intake_url: intake_url.clone(),
+                api_key: api_key.clone(),
+            });
+        }
+    }
+    result
+}
+
+fn trim_url(url: &str) -> String {
+    url.trim_end_matches('/').to_owned()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{additional_endpoints, metrics_intake_url};
+    use crate::config::Config;
+    use std::collections::HashMap;
+
+    #[test]
+    fn test_dd_dd_url_takes_priority_over_dd_url() {
+        let config = Config {
+            dd_url: "https://dd-dd-url.datadoghq.com".to_string(),
+            url: "https://dd-url.datadoghq.com".to_string(),
+            ..Config::default()
+        };
+        assert_eq!(
+            metrics_intake_url(&config).to_string(),
+            "https://dd-dd-url.datadoghq.com"
+        );
+    }
+
+    #[test]
+    fn test_site_fallback() {
+        let config = Config {
+            site: "datadoghq.com".to_string(),
+            ..Config::default()
+        };
+        assert_eq!(
+            metrics_intake_url(&config).to_string(),
+            "https://api.datadoghq.com"
+        );
+    }
+
+    #[test]
+    fn test_additional_endpoints() {
+        let config = Config {
+            additional_endpoints: HashMap::from([(
+                "https://dr-failover.agent.datadoghq.com".to_string(),
+                vec!["api-key-1".to_string(), "api-key-2".to_string()],
+            )]),
+            ..Config::default()
+        };
+        let mut results = additional_endpoints(&config);
+        results.sort_by(|a, b| a.api_key.cmp(&b.api_key));
+        assert_eq!(results.len(), 2);
+        assert_eq!(
+            results[0].intake_url.to_string(),
+            "https://dr-failover.agent.datadoghq.com"
+        );
+        assert_eq!(results[0].api_key, "api-key-1");
+        assert_eq!(
+            results[1].intake_url.to_string(),
+            "https://dr-failover.agent.datadoghq.com"
+        );
+        assert_eq!(results[1].api_key, "api-key-2");
+    }
+
+    #[test]
+    fn test_additional_endpoints_trims_trailing_slash() {
+        let config = Config {
+            additional_endpoints: HashMap::from([(
+                "https://dr-failover.agent.datadoghq.com/".to_string(),
+                vec!["api-key-1".to_string()],
+            )]),
+            ..Config::default()
+        };
+        let results = additional_endpoints(&config);
+        assert_eq!(
+            results[0].intake_url.to_string(),
+            "https://dr-failover.agent.datadoghq.com"
+        );
+    }
+
+    #[test]
+    fn test_additional_endpoints_url_fallback() {
+        let config = Config {
+            additional_endpoints: HashMap::from([(
+                "not a valid url!!!".to_string(),
+                vec!["api-key-1".to_string()],
+            )]),
+            ..Config::default()
+        };
+        let results = additional_endpoints(&config);
+        assert_eq!(
+            results[0].intake_url.to_string(),
+            "https://app.datadoghq.com"
+        );
+        assert_eq!(results[0].api_key, "api-key-1");
+    }
+}

bottlecap/src/metrics/mod.rs

@@ -1 +1,2 @@
 pub mod enhanced;
+pub mod intake;