From b2c7c320816f5aca6fa5030670ba61b31826527d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jordan=20gonz=C3=A1lez?= <30836115+duncanista@users.noreply.github.com> Date: Thu, 25 Sep 2025 16:02:31 -0400 Subject: [PATCH] Revert "cherry pick: No longer launch Go-based agent for compatibility/OTLP/AAP config (#817)" This reverts commit 2396c4fe102677179c834c2dd65cb5b2ea79ca8f. --- bottlecap/src/bin/bottlecap/main.rs | 81 +++---- bottlecap/src/config/mod.rs | 216 +++++++----------- .../src/lifecycle/invocation/processor.rs | 13 +- bottlecap/src/metrics/enhanced/constants.rs | 2 - bottlecap/src/metrics/enhanced/lambda.rs | 86 +------ bottlecap/src/otlp/mod.rs | 6 +- bottlecap/src/proxy/interceptor.rs | 5 +- 7 files changed, 124 insertions(+), 285 deletions(-) diff --git a/bottlecap/src/bin/bottlecap/main.rs b/bottlecap/src/bin/bottlecap/main.rs index 5028b2194..f2417ad6d 100644 --- a/bottlecap/src/bin/bottlecap/main.rs +++ b/bottlecap/src/bin/bottlecap/main.rs @@ -44,7 +44,6 @@ use bottlecap::{ }, logger, logs::{agent::LogsAgent, flusher::LogsFlusher}, - metrics::enhanced::lambda::Lambda as enhanced_metrics, otlp::{agent::Agent as OtlpAgent, should_enable_otlp_agent}, proxy::{interceptor, should_start_proxy}, secrets::decrypt, @@ -85,7 +84,10 @@ use dogstatsd::{ metric::{EMPTY_TAGS, SortedTags}, }; use reqwest::Client; -use std::{collections::hash_map, env, path::Path, sync::Arc}; +use std::{ + collections::hash_map, env, os::unix::process::CommandExt, path::Path, process::Command, + sync::Arc, +}; use tokio::time::{Duration, Instant}; use tokio::{sync::Mutex as TokioMutex, sync::mpsc::Sender, task::JoinHandle}; use tokio_util::sync::CancellationToken; @@ -292,7 +294,14 @@ fn load_configs(start_time: Instant) -> (AwsConfig, Arc) { let aws_config = AwsConfig::from_env(start_time); let lambda_directory: String = env::var("LAMBDA_TASK_ROOT").unwrap_or_else(|_| "/var/task".to_string()); - let config = Arc::new(config::get_config(Path::new(&lambda_directory))); + let config = match config::get_config(Path::new(&lambda_directory)) { + Ok(config) => Arc::new(config), + Err(_e) => { + let err = Command::new("/opt/datadog-agent-go").exec(); + panic!("Error starting the extension: {err:?}"); + } + }; + (aws_config, config) } @@ -389,29 +398,19 @@ async fn extension_loop_active( ); start_dogstatsd_aggregator(metrics_aggr_service); - let metrics_intake_url = create_metrics_intake_url_prefix(config); let metrics_flushers = Arc::new(TokioMutex::new(start_metrics_flushers( Arc::clone(&api_key_factory), &metrics_aggr_handle, - &metrics_intake_url, config, ))); - // Create lambda enhanced metrics instance once - let lambda_enhanced_metrics = - enhanced_metrics::new(metrics_aggr_handle.clone(), Arc::clone(config)); - - // Send config issue metrics - let config_issues = config::inspect_config(config); - send_config_issue_metric(&config_issues, &lambda_enhanced_metrics); - let propagator = Arc::new(DatadogCompositePropagator::new(Arc::clone(config))); // Lifecycle Invocation Processor let invocation_processor = Arc::new(TokioMutex::new(InvocationProcessor::new( Arc::clone(&tags_provider), Arc::clone(config), Arc::clone(&aws_config), - lambda_enhanced_metrics, + metrics_aggr_handle.clone(), Arc::clone(&propagator), ))); // AppSec processor (if enabled) @@ -918,33 +917,33 @@ fn start_logs_agent( (logs_agent_channel, logs_flusher) } -fn create_metrics_intake_url_prefix(config: &Config) -> MetricsIntakeUrlPrefix { - if !config.dd_url.is_empty() { +fn start_metrics_flushers( + api_key_factory: Arc, + metrics_aggr_handle: &MetricsAggregatorHandle, + config: &Arc, +) -> Vec { + 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).expect("can't parse DD_DD_URL prefix") + 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).expect("can't parse DD_URL prefix") + 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).expect("can't parse site prefix") - } -} - -fn start_metrics_flushers( - api_key_factory: Arc, - metrics_aggr_handle: &MetricsAggregatorHandle, - metrics_intake_url: &MetricsIntakeUrlPrefix, - config: &Arc, -) -> Vec { - let mut flushers = Vec::new(); + MetricsIntakeUrlPrefix::new(Some(metrics_site), None) + }; let flusher_config = MetricsFlusherConfig { api_key_factory, aggregator_handle: metrics_aggr_handle.clone(), - metrics_intake_url_prefix: metrics_intake_url.clone(), + metrics_intake_url_prefix: metrics_intake_url.expect("can't parse site or override"), https_proxy: config.proxy_https.clone(), timeout: Duration::from_secs(config.flush_timeout), retry_strategy: DsdRetryStrategy::Immediate(3), @@ -1079,28 +1078,6 @@ fn start_trace_agent( ) } -/// Sends metrics indicating issue with configuration. -/// -/// # Arguments -/// * `issue_reasons` - Vector of messages describing the issue with the configurations -/// * `lambda_enhanced_metrics` - The lambda enhanced metrics instance -fn send_config_issue_metric(issue_reasons: &[String], lambda_enhanced_metrics: &enhanced_metrics) { - if issue_reasons.is_empty() { - return; - } - let now = std::time::UNIX_EPOCH - .elapsed() - .expect("can't poll clock") - .as_secs() - .try_into() - .unwrap_or_default(); - - // Setup a separate metric for each config issue reason - for issue_reason in issue_reasons { - lambda_enhanced_metrics.set_config_load_issue_metric(now, issue_reason); - } -} - fn start_dogstatsd_aggregator(aggr_service: MetricsAggregatorService) { tokio::spawn(async move { aggr_service.run().await; diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index adcb54586..027830ee9 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -459,14 +459,11 @@ impl Default for Config { } } -fn log_unsupported_reason(reason: &str) { - error!("Found unsupported config: {reason} is no longer available."); +fn log_fallback_reason(reason: &str) { + println!("{{\"DD_EXTENSION_FALLBACK_REASON\":\"{reason}\"}}"); } -#[must_use = "Unsupported reasons should be processed to emit appropriate metrics"] -pub fn inspect_config(config: &Config) -> Vec { - let mut unsupported_reasons = Vec::new(); - +fn fallback(config: &Config) -> Result<(), ConfigError> { // Customer explicitly opted out of the Next Gen extension let opted_out = match config.extension_version.as_deref() { Some("compatibility") => true, @@ -475,18 +472,21 @@ pub fn inspect_config(config: &Config) -> Vec { }; if opted_out { - let reason = "extension_version"; - log_unsupported_reason(reason); - unsupported_reasons.push(reason.to_string()); + log_fallback_reason("extension_version"); + return Err(ConfigError::UnsupportedField( + "extension_version".to_string(), + )); } // ASM / .NET // todo(duncanista): Remove once the .NET runtime is fixed if config.serverless_appsec_enabled && has_dotnet_binary() { - let reason = "serverless_appsec_enabled_dotnet"; - log_unsupported_reason(reason); - unsupported_reasons.push(reason.to_string()); + log_fallback_reason("serverless_appsec_enabled_dotnet"); + return Err(ConfigError::UnsupportedField( + "serverless_appsec_enabled_dotnet".to_string(), + )); } + // OTLP let has_otlp_config = config .otlp_config_receiver_protocols_grpc_endpoint @@ -518,22 +518,25 @@ pub fn inspect_config(config: &Config) -> Vec { || config.otlp_config_logs_enabled; if has_otlp_config { - let reason = "otel"; - log_unsupported_reason(reason); - unsupported_reasons.push(reason.to_string()); + log_fallback_reason("otel"); + return Err(ConfigError::UnsupportedField("otel".to_string())); } - unsupported_reasons + Ok(()) } #[allow(clippy::module_name_repetitions)] -#[must_use = "configuration must be used to initialize the application"] -pub fn get_config(config_directory: &Path) -> Config { +pub fn get_config(config_directory: &Path) -> Result { let path: std::path::PathBuf = config_directory.join("datadog.yaml"); let mut config_builder = ConfigBuilder::default() .add_source(Box::new(YamlConfigSource { path })) .add_source(Box::new(EnvConfigSource)); - config_builder.build() + + let config = config_builder.build(); + + fallback(&config)?; + + Ok(config) } #[inline] @@ -742,98 +745,19 @@ pub mod tests { }; #[test] - fn test_baseline_case() { + fn test_reject_on_opted_out() { figment::Jail::expect_with(|jail| { jail.clear_env(); - let _config = get_config(Path::new("")); + jail.set_env("DD_EXTENSION_VERSION", "compatibility"); + let config = get_config(Path::new("")).expect_err("should reject unknown fields"); + assert_eq!( + config, + ConfigError::UnsupportedField("extension_version".to_string()) + ); Ok(()) }); } - #[test] - fn test_inspect_config() { - struct TestCase { - name: &'static str, - env_vars: Vec<(&'static str, &'static str)>, - yaml_content: Option<&'static str>, - expected_reasons: Vec<&'static str>, - } - - let test_cases = vec![ - TestCase { - name: "default config - no unsupported reasons", - env_vars: vec![], - yaml_content: None, - expected_reasons: vec![], - }, - TestCase { - name: "extension_version compatibility - should discover", - env_vars: vec![("DD_EXTENSION_VERSION", "compatibility")], - yaml_content: None, - expected_reasons: vec!["extension_version"], - }, - TestCase { - name: "otlp config enabled - should discover", - env_vars: vec![( - "DD_OTLP_CONFIG_RECEIVER_PROTOCOLS_GRPC_ENDPOINT", - "localhost:4317", - )], - yaml_content: None, - expected_reasons: vec!["otel"], - }, - TestCase { - name: "multiple unsupported reasons", - env_vars: vec![ - ("DD_EXTENSION_VERSION", "compatibility"), - ("DD_OTLP_CONFIG_METRICS_ENABLED", "true"), - ], - yaml_content: None, - expected_reasons: vec!["extension_version", "otel"], - }, - TestCase { - name: "otlp config via yaml - should discover", - env_vars: vec![], - yaml_content: Some( - r" - otlp_config: - receiver: - protocols: - grpc: - endpoint: localhost:4317 - ", - ), - expected_reasons: vec!["otel"], - }, - ]; - - for test_case in test_cases { - figment::Jail::expect_with(|jail| { - jail.clear_env(); - - // Set environment variables - for (key, value) in &test_case.env_vars { - jail.set_env(key, value); - } - - // Create YAML file if provided - if let Some(yaml_content) = test_case.yaml_content { - jail.create_file("datadog.yaml", yaml_content)?; - } - - let config = get_config(Path::new("")); - let unsupported_reasons = inspect_config(&config); - - assert_eq!( - unsupported_reasons, test_case.expected_reasons, - "Test case '{}' failed: expected {:?}, got {:?}", - test_case.name, test_case.expected_reasons, unsupported_reasons - ); - - Ok(()) - }); - } - } - #[test] fn test_fallback_on_otel() { figment::Jail::expect_with(|jail| { @@ -843,7 +767,8 @@ pub mod tests { "localhost:4138", ); - let _config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect_err("should reject unknown fields"); + assert_eq!(config, ConfigError::UnsupportedField("otel".to_string())); Ok(()) }); } @@ -863,7 +788,8 @@ pub mod tests { ", )?; - let _config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect_err("should reject unknown fields"); + assert_eq!(config, ConfigError::UnsupportedField("otel".to_string())); Ok(()) }); } @@ -873,7 +799,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.logs_config_logs_dd_url, "https://http-intake.logs.datadoghq.com".to_string() @@ -891,7 +817,7 @@ pub mod tests { "agent-http-intake-pci.logs.datadoghq.com:443", ); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.logs_config_logs_dd_url, "agent-http-intake-pci.logs.datadoghq.com:443".to_string() @@ -906,7 +832,7 @@ pub mod tests { jail.clear_env(); jail.set_env("DD_APM_DD_URL", "https://trace-pci.agent.datadoghq.com"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.apm_dd_url, "https://trace-pci.agent.datadoghq.com/api/v0.2/traces".to_string() @@ -921,7 +847,7 @@ pub mod tests { jail.clear_env(); jail.set_env("DD_DD_URL", "custom_proxy:3128"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.dd_url, "custom_proxy:3128".to_string()); Ok(()) }); @@ -933,7 +859,7 @@ pub mod tests { jail.clear_env(); jail.set_env("DD_URL", "custom_proxy:3128"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.url, "custom_proxy:3128".to_string()); Ok(()) }); @@ -944,12 +870,27 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.dd_url, String::new()); Ok(()) }); } + #[test] + fn test_allowed_but_disabled() { + figment::Jail::expect_with(|jail| { + jail.clear_env(); + jail.set_env( + "DD_OTLP_CONFIG_RECEIVER_PROTOCOLS_GRPC_ENDPOINT", + "localhost:4138", + ); + + let config = get_config(Path::new("")).expect_err("should reject unknown fields"); + assert_eq!(config, ConfigError::UnsupportedField("otel".to_string())); + Ok(()) + }); + } + #[test] fn test_precedence() { figment::Jail::expect_with(|jail| { @@ -961,7 +902,7 @@ pub mod tests { ", )?; jail.set_env("DD_SITE", "datad0g.com"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.site, "datad0g.com"); Ok(()) }); @@ -977,7 +918,7 @@ pub mod tests { r" ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.site, "datadoghq.com"); Ok(()) }); @@ -988,7 +929,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_SITE", "datadoghq.eu"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.site, "datadoghq.eu"); Ok(()) }); @@ -999,7 +940,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_LOG_LEVEL", "TRACE"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.log_level, LogLevel::Trace); Ok(()) }); @@ -1009,7 +950,7 @@ pub mod tests { fn test_parse_default() { figment::Jail::expect_with(|jail| { jail.clear_env(); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config, Config { @@ -1033,7 +974,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_PROXY_HTTPS", "my-proxy:3128"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.proxy_https, Some("my-proxy:3128".to_string())); Ok(()) }); @@ -1049,7 +990,7 @@ pub mod tests { "NO_PROXY", "127.0.0.1,localhost,172.16.0.0/12,us-east-1.amazonaws.com,datadoghq.eu", ); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse noproxy"); assert_eq!(config.proxy_https, None); Ok(()) }); @@ -1067,7 +1008,7 @@ pub mod tests { ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse weird proxy config"); assert_eq!(config.proxy_https, Some("my-proxy:3128".to_string())); Ok(()) }); @@ -1087,7 +1028,7 @@ pub mod tests { ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse weird proxy config"); assert_eq!(config.proxy_https, None); // Assertion to ensure config.site runs before proxy // because we chenck that noproxy contains the site @@ -1101,7 +1042,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_SERVERLESS_FLUSH_STRATEGY", "end"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.serverless_flush_strategy, FlushStrategy::End); Ok(()) }); @@ -1112,7 +1053,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_SERVERLESS_FLUSH_STRATEGY", "periodically,100000"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.serverless_flush_strategy, FlushStrategy::Periodically(PeriodicStrategy { interval: 100_000 }) @@ -1126,7 +1067,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_SERVERLESS_FLUSH_STRATEGY", "invalid_strategy"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.serverless_flush_strategy, FlushStrategy::Default); Ok(()) }); @@ -1140,7 +1081,7 @@ pub mod tests { "DD_SERVERLESS_FLUSH_STRATEGY", "periodically,invalid_interval", ); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.serverless_flush_strategy, FlushStrategy::Default); Ok(()) }); @@ -1153,7 +1094,7 @@ pub mod tests { jail.set_env("DD_VERSION", "123"); jail.set_env("DD_ENV", "123456890"); jail.set_env("DD_SERVICE", "123456"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.version.expect("failed to parse DD_VERSION"), "123"); assert_eq!(config.env.expect("failed to parse DD_ENV"), "123456890"); assert_eq!( @@ -1183,7 +1124,7 @@ pub mod tests { pattern: exclude-me-yaml ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.logs_config_processing_rules, Some(vec![ProcessingRule { @@ -1212,7 +1153,7 @@ pub mod tests { pattern: exclude ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.logs_config_processing_rules, Some(vec![ProcessingRule { @@ -1241,7 +1182,7 @@ pub mod tests { repl: 'REDACTED' ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); let rule = parse_rules_from_string( r#"[ {"name": "*", "pattern": "foo", "repl": "REDACTED"} @@ -1272,7 +1213,7 @@ pub mod tests { repl: 'REDACTED-YAML' ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); let rule = parse_rules_from_string( r#"[ {"name": "*", "pattern": "foo", "repl": "REDACTED-ENV"} @@ -1299,7 +1240,7 @@ pub mod tests { remove_paths_with_digits: true ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert!(config.apm_config_obfuscation_http_remove_query_string,); assert!(config.apm_config_obfuscation_http_remove_paths_with_digits,); Ok(()) @@ -1314,7 +1255,7 @@ pub mod tests { "datadog,tracecontext,b3,b3multi", ); jail.set_env("DD_EXTENSION_VERSION", "next"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); let expected_styles = vec![ TracePropagationStyle::Datadog, @@ -1333,7 +1274,7 @@ pub mod tests { figment::Jail::expect_with(|jail| { jail.clear_env(); jail.set_env("DD_TRACE_PROPAGATION_STYLE_EXTRACT", "datadog"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!( config.trace_propagation_style, @@ -1358,7 +1299,8 @@ pub mod tests { "DD_APM_REPLACE_TAGS", r#"[{"name":"resource.name","pattern":"(.*)/(foo[:%].+)","repl":"$1/{foo}"}]"#, ); - let _config = get_config(Path::new("")); + let config = get_config(Path::new("")); + assert!(config.is_ok()); Ok(()) }); } @@ -1371,7 +1313,7 @@ pub mod tests { jail.set_env("DD_ENHANCED_METRICS", "1"); jail.set_env("DD_LOGS_CONFIG_USE_COMPRESSION", "TRUE"); jail.set_env("DD_CAPTURE_LAMBDA_PAYLOAD", "0"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert!(config.serverless_logs_enabled); assert!(config.enhanced_metrics); assert!(config.logs_config_use_compression); @@ -1395,7 +1337,7 @@ pub mod tests { jail.set_env("DD_SITE", "us5.datadoghq.com"); jail.set_env("DD_API_KEY", "env-api-key"); jail.set_env("DD_FLUSH_TIMEOUT", "10"); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert_eq!(config.site, "us5.datadoghq.com"); assert_eq!(config.api_key, "env-api-key"); diff --git a/bottlecap/src/lifecycle/invocation/processor.rs b/bottlecap/src/lifecycle/invocation/processor.rs index 28d9fd37f..02a7a5589 100644 --- a/bottlecap/src/lifecycle/invocation/processor.rs +++ b/bottlecap/src/lifecycle/invocation/processor.rs @@ -90,7 +90,7 @@ impl Processor { tags_provider: Arc, config: Arc, aws_config: Arc, - enhanced_metrics: EnhancedMetrics, + metrics_aggregator: dogstatsd::aggregator_service::AggregatorHandle, propagator: Arc, ) -> Self { let resource = tags_provider @@ -107,7 +107,7 @@ impl Processor { context_buffer: ContextBuffer::default(), inferrer: SpanInferrer::new(Arc::clone(&config)), propagator, - enhanced_metrics, + enhanced_metrics: EnhancedMetrics::new(metrics_aggregator, Arc::clone(&config)), aws_config, tracer_detected: false, runtime: None, @@ -989,15 +989,8 @@ mod tests { tokio::spawn(service.run()); - let enhanced_metrics = EnhancedMetrics::new(handle, Arc::clone(&config)); let propagator = Arc::new(DatadogCompositePropagator::new(Arc::clone(&config))); - Processor::new( - tags_provider, - config, - aws_config, - enhanced_metrics, - propagator, - ) + Processor::new(tags_provider, config, aws_config, handle, propagator) } #[test] diff --git a/bottlecap/src/metrics/enhanced/constants.rs b/bottlecap/src/metrics/enhanced/constants.rs index 110c0b618..44666910f 100644 --- a/bottlecap/src/metrics/enhanced/constants.rs +++ b/bottlecap/src/metrics/enhanced/constants.rs @@ -45,8 +45,6 @@ pub const THREADS_USE_METRIC: &str = "aws.lambda.enhanced.threads_use"; pub const SHUTDOWNS_METRIC: &str = "aws.lambda.enhanced.shutdowns"; //pub const ASM_INVOCATIONS_METRIC: &str = "aws.lambda.enhanced.asm.invocations"; pub const UNUSED_INIT: &str = "aws.lambda.enhanced.unused_init"; -pub const DATADOG_SERVERLESS_EXTENSION_FAILOVER_CONFIG_ISSUE_METRIC: &str = - "datadog.serverless.extension.failover"; pub const ENHANCED_METRICS_ENV_VAR: &str = "DD_ENHANCED_METRICS"; // Monitoring interval for tmp, fd, and threads metrics diff --git a/bottlecap/src/metrics/enhanced/lambda.rs b/bottlecap/src/metrics/enhanced/lambda.rs index 347d06b60..42abfc6a6 100644 --- a/bottlecap/src/metrics/enhanced/lambda.rs +++ b/bottlecap/src/metrics/enhanced/lambda.rs @@ -69,29 +69,18 @@ impl Lambda { .insert(String::from("runtime"), runtime.to_string()); } - fn tags_to_sorted_tags(tags: &HashMap) -> Option { - let vec_tags: Vec = tags.iter().map(|(k, v)| format!("{k}:{v}")).collect(); + fn get_dynamic_value_tags(&self) -> Option { + let vec_tags: Vec = self + .dynamic_value_tags + .iter() + .map(|(k, v)| format!("{k}:{v}")) + .collect(); let string_tags = vec_tags.join(","); SortedTags::parse(&string_tags).ok() } - fn get_dynamic_value_tags(&self) -> Option { - Self::tags_to_sorted_tags(&self.dynamic_value_tags) - } - - fn get_combined_tags(&self, additional_tags: &HashMap) -> Option { - if additional_tags.is_empty() { - return self.get_dynamic_value_tags(); - } - - let mut combined_tags = self.dynamic_value_tags.clone(); - combined_tags.extend(additional_tags.clone()); - - Self::tags_to_sorted_tags(&combined_tags) - } - pub fn increment_invocation_metric(&self, timestamp: i64) { self.increment_metric(constants::INVOCATIONS_METRIC, timestamp); } @@ -114,19 +103,6 @@ impl Lambda { self.increment_metric(constants::OUT_OF_MEMORY_METRIC, timestamp); } - /// Set up a metric tracking configuration load issue with details - pub fn set_config_load_issue_metric(&self, timestamp: i64, reason_msg: &str) { - let dynamic_tags = self.get_combined_tags(&HashMap::from([( - "reason".to_string(), - reason_msg.to_string(), - )])); - self.increment_metric_with_tags( - constants::DATADOG_SERVERLESS_EXTENSION_FAILOVER_CONFIG_ISSUE_METRIC, - timestamp, - dynamic_tags, - ); - } - pub fn set_init_duration_metric( &mut self, init_type: InitType, @@ -155,23 +131,13 @@ impl Lambda { } fn increment_metric(&self, metric_name: &str, timestamp: i64) { - self.increment_metric_with_tags(metric_name, timestamp, self.get_dynamic_value_tags()); - } - - /// Helper function to emit metric with supplied tags - fn increment_metric_with_tags( - &self, - metric_name: &str, - timestamp: i64, - tags: Option, - ) { if !self.config.enhanced_metrics { return; } let metric = Metric::new( metric_name.into(), MetricValue::distribution(1f64), - tags, + self.get_dynamic_value_tags(), Some(timestamp), ); if let Err(e) = self.aggr_handle.insert_batch(vec![metric]) { @@ -830,19 +796,9 @@ mod tests { } async fn assert_sketch(handle: &AggregatorHandle, metric_id: &str, value: f64, timestamp: i64) { - assert_sketch_with_tag(handle, metric_id, value, timestamp, None).await; - } - - async fn assert_sketch_with_tag( - handle: &AggregatorHandle, - metric_id: &str, - value: f64, - timestamp: i64, - tags: Option, - ) { let ts = (timestamp / 10) * 10; if let Some(e) = handle - .get_entry_by_id(metric_id.into(), tags, ts) + .get_entry_by_id(metric_id.into(), None, ts) .await .unwrap() { @@ -1444,30 +1400,4 @@ mod tests { .is_none() ); } - - #[tokio::test] - async fn test_set_config_load_issue_metric() { - let (metrics_aggr, my_config) = setup(); - let lambda = Lambda::new(metrics_aggr.clone(), my_config); - let now: i64 = std::time::UNIX_EPOCH - .elapsed() - .expect("unable to poll clock, unrecoverable") - .as_secs() - .try_into() - .unwrap_or_default(); - let test_reason = "test_config_issue"; - - lambda.set_config_load_issue_metric(now, test_reason); - - // Create the expected tags for the metric lookup - let expected_tags = SortedTags::parse(&format!("reason:{test_reason}")).ok(); - assert_sketch_with_tag( - &metrics_aggr, - constants::DATADOG_SERVERLESS_EXTENSION_FAILOVER_CONFIG_ISSUE_METRIC, - 1f64, - now, - expected_tags, - ) - .await; - } } diff --git a/bottlecap/src/otlp/mod.rs b/bottlecap/src/otlp/mod.rs index 4e0f4aed7..292a80180 100644 --- a/bottlecap/src/otlp/mod.rs +++ b/bottlecap/src/otlp/mod.rs @@ -37,7 +37,7 @@ mod tests { ", )?; - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); // Since the default for traces is `true`, we don't need to set it. assert!(should_enable_otlp_agent(&Arc::new(config))); @@ -55,7 +55,7 @@ mod tests { "0.0.0.0:4318", ); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); // Since the default for traces is `true`, we don't need to set it. assert!(should_enable_otlp_agent(&Arc::new(config))); @@ -74,7 +74,7 @@ mod tests { "0.0.0.0:4318", ); - let config = get_config(Path::new("")); + let config = get_config(Path::new("")).expect("should parse config"); assert!(!should_enable_otlp_agent(&Arc::new(config))); diff --git a/bottlecap/src/proxy/interceptor.rs b/bottlecap/src/proxy/interceptor.rs index fdab63d79..92bcf1f94 100644 --- a/bottlecap/src/proxy/interceptor.rs +++ b/bottlecap/src/proxy/interceptor.rs @@ -489,8 +489,7 @@ mod tests { tokio::spawn(service.run()); - let enhanced_metrics = - crate::metrics::enhanced::lambda::Lambda::new(handle, Arc::clone(&config)); + let metrics_aggregator = handle; let aws_config = Arc::new(AwsConfig { region: "us-east-1".to_string(), function_name: "arn:some-function".to_string(), @@ -504,7 +503,7 @@ mod tests { Arc::clone(&tags_provider), Arc::clone(&config), Arc::clone(&aws_config), - enhanced_metrics, + metrics_aggregator, Arc::clone(&propagator), ))); let appsec_processor = match AppSecProcessor::new(&config) {