From 42c6b77ed436d41cf6f34ed72fbd5d44b7b7f424 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Thu, 2 Jul 2026 18:03:38 -0400 Subject: [PATCH 01/13] feat(traces): rescue errored traces via agent-side error sampler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the lambda_extension_compute_stats path, the extension drops every trace marked P0 (priority <= 0) after computing its stats. This adds an error sampler that gives those dropped traces a second look: errored traces are kept (rescued) up to DD_APM_ERROR_TPS traces/sec (default 10), distributed fairly across trace signatures, with _dd.errors_sr stamped on the rescued root span. Non-errored P0 traces are still dropped, and stats still count all traces. This guarantees error visibility even under aggressive sampling. Ports the Go trace agent's ScoreSampler/ErrorTPS behavior via the shared, dependency-free datadog-agent-trace-sampler crate. New config: - DD_APM_ERROR_TPS (default 10.0; 0 disables the rescue) - DD_APM_EXTRA_SAMPLE_RATE (default 1.0) 🤖 --- bottlecap/Cargo.lock | 12 +- bottlecap/Cargo.toml | 7 +- bottlecap/src/bin/bottlecap/main.rs | 8 + bottlecap/src/config/mod.rs | 58 ++++++ .../src/lifecycle/invocation/processor.rs | 4 + bottlecap/src/traces/trace_processor.rs | 194 +++++++++++++++++- bottlecap/tests/apm_integration_test.rs | 5 + 7 files changed, 280 insertions(+), 8 deletions(-) diff --git a/bottlecap/Cargo.lock b/bottlecap/Cargo.lock index 7e010800c..da02835c3 100644 --- a/bottlecap/Cargo.lock +++ b/bottlecap/Cargo.lock @@ -485,6 +485,7 @@ dependencies = [ "chrono", "cookie", "datadog-agent-config", + "datadog-agent-trace-sampler", "datadog-fips", "datadog-opentelemetry", "datadog-protos", @@ -809,7 +810,7 @@ dependencies = [ [[package]] name = "datadog-agent-config" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=d0c7f44191445e20d309734675d5e8b91d2a5d51#d0c7f44191445e20d309734675d5e8b91d2a5d51" +source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" dependencies = [ "datadog-opentelemetry", "dogstatsd", @@ -824,10 +825,15 @@ dependencies = [ "tracing", ] +[[package]] +name = "datadog-agent-trace-sampler" +version = "0.1.0" +source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" + [[package]] name = "datadog-fips" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=d0c7f44191445e20d309734675d5e8b91d2a5d51#d0c7f44191445e20d309734675d5e8b91d2a5d51" +source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" dependencies = [ "reqwest", "rustls", @@ -976,7 +982,7 @@ dependencies = [ [[package]] name = "dogstatsd" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=d0c7f44191445e20d309734675d5e8b91d2a5d51#d0c7f44191445e20d309734675d5e8b91d2a5d51" +source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" dependencies = [ "datadog-protos", "ddsketch-agent", diff --git a/bottlecap/Cargo.toml b/bottlecap/Cargo.toml index 563545feb..a4b5da378 100644 --- a/bottlecap/Cargo.toml +++ b/bottlecap/Cargo.toml @@ -82,9 +82,10 @@ libdd-trace-normalization = { git = "https://github.com/DataDog/libdatadog", rev libdd-trace-obfuscation = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } libdd-trace-stats = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } datadog-opentelemetry = { git = "https://github.com/DataDog/dd-trace-rs", rev = "50bfea8755b75e448a80ac04d53fa7edd414eefe", default-features = false, features = ["_unstable_propagation"] } -dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "d0c7f44191445e20d309734675d5e8b91d2a5d51", default-features = false } -datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "d0c7f44191445e20d309734675d5e8b91d2a5d51", default-features = false } -datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "d0c7f44191445e20d309734675d5e8b91d2a5d51", default-features = false } +dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } +datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } +datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } +datadog-agent-trace-sampler = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } libddwaf = { version = "1.28.1", git = "https://github.com/DataDog/libddwaf-rust", rev = "d1534a158d976bd4f747bf9fcc58e0712d2d17fc", default-features = false, features = ["serde"] } [dev-dependencies] diff --git a/bottlecap/src/bin/bottlecap/main.rs b/bottlecap/src/bin/bottlecap/main.rs index 39f352149..ee48950ee 100644 --- a/bottlecap/src/bin/bottlecap/main.rs +++ b/bottlecap/src/bin/bottlecap/main.rs @@ -1162,6 +1162,14 @@ fn start_trace_agent( let trace_processor = Arc::new(trace_processor::ServerlessTraceProcessor { obfuscation_config: Arc::new(obfuscation_config), + error_sampler: Arc::new(std::sync::Mutex::new( + datadog_agent_trace_sampler::ErrorsSampler::new( + datadog_agent_trace_sampler::ErrorSamplerConfig { + target_tps: config.ext.apm_error_tps, + extra_sample_rate: config.ext.apm_extra_sample_rate, + }, + ), + )), }); let (span_dedup_service, span_dedup_handle) = span_dedup_service::DedupService::new(); diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index db0a1f192..3c959a477 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -66,6 +66,17 @@ pub struct LambdaConfig { pub capture_lambda_payload: bool, pub capture_lambda_payload_max_depth: u32, pub lambda_extension_compute_stats: bool, + + /// `DD_APM_ERROR_TPS` — target error traces per second rescued by the + /// agent-side error sampler when the extension computes stats. Matches the + /// Go trace agent's `apm_config.errors_per_second`. Default 10.0; `0` + /// disables the sampler (no rescue). + pub apm_error_tps: f64, + /// `DD_APM_EXTRA_SAMPLE_RATE` — extra raw sampling rate applied on top of + /// the computed error-sampler rate. Matches the Go trace agent's + /// `apm_config.extra_sample_rate`. Default 1.0. + pub apm_extra_sample_rate: f64, + pub span_dedup_timeout: Option, pub api_key_secret_reload_interval: Option, pub serverless_appsec_enabled: bool, @@ -95,6 +106,8 @@ impl Default for LambdaConfig { capture_lambda_payload: false, capture_lambda_payload_max_depth: 10, lambda_extension_compute_stats: false, + apm_error_tps: 10.0, + apm_extra_sample_rate: 1.0, span_dedup_timeout: None, api_key_secret_reload_interval: None, serverless_appsec_enabled: false, @@ -153,6 +166,15 @@ pub struct LambdaConfigSource { #[serde(deserialize_with = "deser_opt_bool")] pub lambda_extension_compute_stats: Option, + /// `DD_APM_ERROR_TPS` — error sampler target traces/sec. Flat env/YAML key + /// (`apm_error_tps`), unlike the Go agent's nested `apm_config.errors_per_second`. + #[serde(deserialize_with = "deser_opt_lossless")] + pub apm_error_tps: Option, + /// `DD_APM_EXTRA_SAMPLE_RATE` — error sampler extra sample rate. Flat + /// env/YAML key (`apm_extra_sample_rate`). + #[serde(deserialize_with = "deser_opt_lossless")] + pub apm_extra_sample_rate: Option, + #[serde(deserialize_with = "deser_dur_secs_ignore_zero")] pub span_dedup_timeout: Option, #[serde(deserialize_with = "deser_dur_secs_ignore_zero")] @@ -199,6 +221,8 @@ impl DatadogConfigExtension for LambdaConfig { capture_lambda_payload, capture_lambda_payload_max_depth, lambda_extension_compute_stats, + apm_error_tps, + apm_extra_sample_rate, serverless_appsec_enabled, appsec_waf_timeout, api_security_enabled, @@ -233,6 +257,7 @@ impl DatadogConfigExtension for LambdaConfig { #[cfg_attr(coverage_nightly, coverage(off))] // Test modules skew coverage metrics #[cfg(test)] #[allow(clippy::unwrap_used)] +#[allow(clippy::float_cmp)] // exact, representable config values (10.0, 1.0, ...) parsed deterministically mod lambda_config_tests { use datadog_agent_config::{ Config as UpstreamConfig, flush_strategy::PeriodicStrategy, get_config_with_extension, @@ -536,6 +561,39 @@ mod lambda_config_tests { assert!(!config.ext.lambda_extension_compute_stats); } + // ---- error sampler (apm_error_tps / apm_extra_sample_rate) ---- + + #[test] + fn apm_error_tps_defaults_to_ten() { + let config = load(|_| Ok(())); + assert_eq!(config.ext.apm_error_tps, 10.0); + assert_eq!(config.ext.apm_extra_sample_rate, 1.0); + } + + #[test] + fn apm_error_tps_from_env() { + let config = load(|jail| { + jail.set_env("DD_APM_ERROR_TPS", "25"); + jail.set_env("DD_APM_EXTRA_SAMPLE_RATE", "0.5"); + Ok(()) + }); + assert_eq!(config.ext.apm_error_tps, 25.0); + assert_eq!(config.ext.apm_extra_sample_rate, 0.5); + } + + #[test] + fn apm_error_tps_from_yaml() { + let config = load(|jail| { + jail.create_file( + "datadog.yaml", + "apm_error_tps: 0\napm_extra_sample_rate: 2\n", + )?; + Ok(()) + }); + assert_eq!(config.ext.apm_error_tps, 0.0); + assert_eq!(config.ext.apm_extra_sample_rate, 2.0); + } + // ---- Duration fields ---- #[test] diff --git a/bottlecap/src/lifecycle/invocation/processor.rs b/bottlecap/src/lifecycle/invocation/processor.rs index 307484b56..c878d3810 100644 --- a/bottlecap/src/lifecycle/invocation/processor.rs +++ b/bottlecap/src/lifecycle/invocation/processor.rs @@ -1918,6 +1918,7 @@ mod tests { appsec: None, processor: Arc::new(trace_processor::ServerlessTraceProcessor { obfuscation_config: Arc::new(ObfuscationConfig::new().expect("Failed to create ObfuscationConfig")), + error_sampler: trace_processor::default_error_sampler(), }), trace_tx: tokio::sync::mpsc::channel(1).0, stats_generator: Arc::new(StatsGenerator::new(stats_concentrator_handle)), @@ -2028,6 +2029,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: trace_processor::default_error_sampler(), }), trace_tx: tokio::sync::mpsc::channel(1).0, stats_generator: Arc::new(StatsGenerator::new(stats_concentrator_handle)), @@ -2667,6 +2669,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: trace_processor::default_error_sampler(), }), trace_tx: tokio::sync::mpsc::channel(1).0, stats_generator: Arc::new(StatsGenerator::new(stats_concentrator_handle)), @@ -3176,6 +3179,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: trace_processor::default_error_sampler(), }), trace_tx, stats_generator: Arc::new(StatsGenerator::new(stats_concentrator_handle)), diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index f16eee7bc..aea4b08eb 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -15,6 +15,7 @@ use crate::traces::{ LAMBDA_RUNTIME_URL_PREFIX, LAMBDA_STATSD_URL_PREFIX, }; use async_trait::async_trait; +use datadog_agent_trace_sampler::{ErrorsSampler, SampleDecision, SpanView, TraceView}; use libdd_common::Endpoint; use libdd_trace_obfuscation::obfuscate::obfuscate_span; use libdd_trace_obfuscation::obfuscation_config; @@ -62,6 +63,67 @@ impl StatsComputedBy { #[allow(clippy::module_name_repetitions)] pub struct ServerlessTraceProcessor { pub obfuscation_config: Arc, + /// Agent-side error sampler. On the `lambda_extension_compute_stats` path, + /// errored chunks that would be dropped (priority <= 0) get a second look + /// and are rescued up to `apm_error_tps` traces/sec. Shared across + /// invocations (std Mutex, not tokio: consulted from the synchronous + /// `process_traces`) so its rolling-window rate limiter accumulates. + pub error_sampler: Arc>, +} + +impl ServerlessTraceProcessor { + /// Consult the error sampler for a chunk that would otherwise be dropped + /// (priority <= 0). Returns `true` to keep (rescue) the chunk. Only errored + /// traces are candidates; on a keep, stamps `_dd.errors_sr` on the root span. + fn rescue_error_chunk(&self, chunk: &mut pb::TraceChunk, env: &str, now_secs: i64) -> bool { + let Ok(root_idx) = trace_utils::get_root_span_index(&chunk.spans) else { + return false; // can't identify a root span; drop as before + }; + // Only errored traces are rescue candidates (matches the Go agent, which + // only routes error traces through the ErrorTPS ScoreSampler). + if chunk.spans.get(root_idx).map_or(0, |s| s.error) == 0 { + return false; + } + + // Build the borrow-only views in a scope so they (and their immutable + // borrow of chunk.spans) drop before the `_dd.errors_sr` mutation below. + let decision = { + let views: Vec = chunk + .spans + .iter() + .map(|s| SpanView { + service: &s.service, + name: &s.name, + resource: &s.resource, + error: s.error != 0, + http_status_code: s.meta.get("http.status_code").map(String::as_str), + error_type: s.meta.get("error.type").map(String::as_str), + }) + .collect(); + let root = &chunk.spans[root_idx]; + let trace = TraceView { + env, + trace_id: root.trace_id, + root_index: root_idx, + root_global_sample_rate: root.metrics.get("_sample_rate").copied().unwrap_or(1.0), + spans: &views, + }; + match self.error_sampler.lock() { + Ok(mut sampler) => sampler.sample(now_secs, &trace), + Err(_) => return false, // poisoned lock: fail safe to the drop + } + }; + + match decision { + SampleDecision::Keep { errors_sr } => { + if let Some(root) = chunk.spans.get_mut(root_idx) { + root.metrics.insert("_dd.errors_sr".to_string(), errors_sr); + } + true + } + SampleDecision::Drop => false, + } + } } struct ChunkProcessor { @@ -445,9 +507,22 @@ impl TraceProcessor for ServerlessTraceProcessor { if config.ext.lambda_extension_compute_stats && let TracerPayloadCollection::V07(ref mut tracer_payloads) = payload { + let env = config.env.as_deref().unwrap_or_default(); + let now_secs: i64 = std::time::UNIX_EPOCH + .elapsed() + .expect("unable to poll clock, unrecoverable") + .as_secs() + .try_into() + .unwrap_or_default(); for tp in tracer_payloads.iter_mut() { - tp.chunks.retain(|chunk| { - chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 + tp.chunks.retain_mut(|chunk| { + // Explicit keeps and "no priority set" pass through unchanged. + if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { + return true; + } + // Otherwise this chunk would be dropped; give errored ones a + // second look via the error sampler (rescue within budget). + self.rescue_error_chunk(chunk, env, now_secs) }); } tracer_payloads.retain(|tp| !tp.chunks.is_empty()); @@ -598,6 +673,16 @@ impl SendingTraceProcessor { } } +/// Default error sampler for constructing `ServerlessTraceProcessor` in tests +/// (sampler behavior itself is unit-tested in the `datadog-agent-trace-sampler` +/// crate; these call sites just need a value). +#[cfg(test)] +pub(crate) fn default_error_sampler() -> Arc> { + Arc::new(std::sync::Mutex::new(ErrorsSampler::new( + datadog_agent_trace_sampler::ErrorSamplerConfig::default(), + ))) +} + #[cfg(test)] mod tests { use std::{ @@ -723,6 +808,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }; let config = create_test_config(); let tags_provider = create_tags_provider(config.clone()); @@ -1201,6 +1287,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }; let header_tags = tracer_header_tags::TracerHeaderTags { @@ -1272,6 +1359,105 @@ mod tests { ); } + /// On the compute-stats path, the error sampler rescues errored chunks that + /// would otherwise be dropped (priority <= 0), stamping `_dd.errors_sr`, + /// while non-errored P0 chunks are still dropped. + #[test] + fn test_error_sampler_rescues_errored_p0_chunks() { + use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig; + + let config = Arc::new(Config { + apm_dd_url: "https://trace.agent.datadoghq.com".to_string(), + ext: crate::config::LambdaConfig { + lambda_extension_compute_stats: true, + ..Default::default() + }, + ..Config::default() + }); + let tags_provider = Arc::new(Provider::new( + config.clone(), + "lambda".to_string(), + &std::collections::HashMap::from([( + "function_arn".to_string(), + "test-arn".to_string(), + )]), + )); + let processor = ServerlessTraceProcessor { + obfuscation_config: Arc::new( + ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), + ), + error_sampler: default_error_sampler(), + }; + + let header_tags = tracer_header_tags::TracerHeaderTags { + lang: "rust", + lang_version: "1.0", + lang_interpreter: "", + lang_vendor: "", + tracer_version: "1.0", + container_id: "", + client_computed_top_level: false, + client_computed_stats: false, + dropped_p0_traces: 0, + dropped_p0_spans: 0, + }; + + let make_span = |trace_id: u64, priority: f64, error: i32| -> pb::Span { + let mut metrics = HashMap::new(); + metrics.insert("_sampling_priority_v1".to_string(), priority); + pb::Span { + trace_id, + span_id: trace_id, + parent_id: 0, + error, + metrics, + service: "svc".to_string(), + name: "op".to_string(), + resource: "res".to_string(), + ..Default::default() + } + }; + + // trace 1: kept normally (priority 1). trace 2: errored P0 (rescued). + // trace 3: non-errored P0 (dropped). + let traces = vec![ + vec![make_span(1, 1.0, 0)], + vec![make_span(2, 0.0, 1)], + vec![make_span(3, 0.0, 0)], + ]; + + let (payload_info, _stats) = + processor.process_traces(config, tags_provider, header_tags, traces, 0, None); + let payload_info = payload_info.expect("expected Some payload"); + let backend_send_data = payload_info.builder.build(); + let TracerPayloadCollection::V07(backend_payloads) = backend_send_data.get_payloads() + else { + panic!("expected V07"); + }; + + let kept: Vec = backend_payloads + .iter() + .flat_map(|tp| tp.chunks.iter()) + .flat_map(|c| c.spans.iter()) + .map(|s| s.trace_id) + .collect(); + assert_eq!(kept.len(), 2, "kept normal trace + rescued errored trace"); + assert!(kept.contains(&1), "priority-1 trace kept"); + assert!(kept.contains(&2), "errored P0 trace rescued"); + assert!(!kept.contains(&3), "non-errored P0 trace dropped"); + + let rescued_root = backend_payloads + .iter() + .flat_map(|tp| tp.chunks.iter()) + .flat_map(|c| c.spans.iter()) + .find(|s| s.trace_id == 2) + .expect("rescued trace present"); + assert!( + rescued_root.metrics.contains_key("_dd.errors_sr"), + "_dd.errors_sr stamped on rescued root" + ); + } + /// Verifies that `process_traces` returns `None` for the backend payload when all /// traces are sampled out and `lambda_extension_compute_stats` is true. #[test] @@ -1298,6 +1484,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }; let header_tags = tracer_header_tags::TracerHeaderTags { lang: "rust", @@ -1379,6 +1566,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }; let header_tags = tracer_header_tags::TracerHeaderTags { lang: "rust", @@ -1487,6 +1675,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }; let header_tags = tracer_header_tags::TracerHeaderTags { lang: "rust", @@ -1858,6 +2047,7 @@ mod tests { obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: default_error_sampler(), }), trace_tx, stats_generator: Arc::new(StatsGenerator::new(concentrator_handle.clone())), diff --git a/bottlecap/tests/apm_integration_test.rs b/bottlecap/tests/apm_integration_test.rs index cfb89f244..2f1429d0b 100644 --- a/bottlecap/tests/apm_integration_test.rs +++ b/bottlecap/tests/apm_integration_test.rs @@ -339,6 +339,11 @@ async fn run_processor_pipeline_with_traces( obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), + error_sampler: Arc::new(std::sync::Mutex::new( + datadog_agent_trace_sampler::ErrorsSampler::new( + datadog_agent_trace_sampler::ErrorSamplerConfig::default(), + ), + )), }), trace_tx, stats_generator: Arc::new(StatsGenerator::new(concentrator_handle.clone())), From b724eb504a9d03152a45977f89b5aeda0474cfa3 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 4 Aug 2026 13:25:28 -0400 Subject: [PATCH 02/13] Update LICENSE-3rdparty.csv for datadog-agent-trace-sampler --- bottlecap/LICENSE-3rdparty.csv | 1 + 1 file changed, 1 insertion(+) diff --git a/bottlecap/LICENSE-3rdparty.csv b/bottlecap/LICENSE-3rdparty.csv index 9da9c4542..bddc9f975 100644 --- a/bottlecap/LICENSE-3rdparty.csv +++ b/bottlecap/LICENSE-3rdparty.csv @@ -44,6 +44,7 @@ crossbeam-utils,https://github.com/crossbeam-rs/crossbeam,MIT OR Apache-2.0,The crypto-common,https://github.com/RustCrypto/traits,MIT OR Apache-2.0,RustCrypto Developers ctor,https://github.com/mmastrac/rust-ctor,Apache-2.0 OR MIT,Matt Mastracci datadog-agent-config,https://github.com/DataDog/serverless-components,Apache-2.0,The datadog-agent-config Authors +datadog-agent-trace-sampler,https://github.com/DataDog/serverless-components,Apache-2.0,The datadog-agent-trace-sampler Authors datadog-fips,https://github.com/DataDog/serverless-components,Apache-2.0,The datadog-fips Authors datadog-opentelemetry,https://github.com/DataDog/dd-trace-rs/tree/main/datadog-opentelemetry,Apache-2.0,Datadog Inc. datadog-protos,https://github.com/DataDog/saluki,Apache-2.0,The datadog-protos Authors From 1e010eb5b2a88c94d9e3e597fc5f1a40c66602eb Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 4 Aug 2026 14:11:28 -0400 Subject: [PATCH 03/13] fix(traces): rescue traces with an error on any span, not just the root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Errored traces were only rescued from a drop decision when the root span itself carried the error, so a trace whose failure happened deeper (for example a failed downstream call that the handler caught) was still dropped. Now an error anywhere in the trace makes it a rescue candidate, matching the Datadog Agent. 🤖 --- bottlecap/src/traces/trace_processor.rs | 88 ++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index aea4b08eb..79a356cfe 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -80,8 +80,9 @@ impl ServerlessTraceProcessor { return false; // can't identify a root span; drop as before }; // Only errored traces are rescue candidates (matches the Go agent, which - // only routes error traces through the ErrorTPS ScoreSampler). - if chunk.spans.get(root_idx).map_or(0, |s| s.error) == 0 { + // only routes error traces through the ErrorTPS ScoreSampler). An error + // anywhere in the chunk counts, not just on the root span. + if !chunk.spans.iter().any(|s| s.error != 0) { return false; } @@ -1458,6 +1459,89 @@ mod tests { ); } + /// An error on a child span (root not errored) still makes the chunk a rescue + /// candidate, matching the Go agent's `traceContainsError`. + #[test] + fn test_error_sampler_rescues_chunk_with_errored_child_span() { + use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig; + + let config = Arc::new(Config { + apm_dd_url: "https://trace.agent.datadoghq.com".to_string(), + ext: crate::config::LambdaConfig { + lambda_extension_compute_stats: true, + ..Default::default() + }, + ..Config::default() + }); + let tags_provider = Arc::new(Provider::new( + config.clone(), + "lambda".to_string(), + &std::collections::HashMap::from([( + "function_arn".to_string(), + "test-arn".to_string(), + )]), + )); + let processor = ServerlessTraceProcessor { + obfuscation_config: Arc::new( + ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), + ), + error_sampler: default_error_sampler(), + }; + + let header_tags = tracer_header_tags::TracerHeaderTags { + lang: "rust", + lang_version: "1.0", + lang_interpreter: "", + lang_vendor: "", + tracer_version: "1.0", + container_id: "", + client_computed_top_level: false, + client_computed_stats: false, + dropped_p0_traces: 0, + dropped_p0_spans: 0, + }; + + let make_span = |span_id: u64, parent_id: u64, error: i32| -> pb::Span { + let mut metrics = HashMap::new(); + metrics.insert("_sampling_priority_v1".to_string(), 0.0); + pb::Span { + trace_id: 1, + span_id, + parent_id, + error, + metrics, + service: "svc".to_string(), + name: "op".to_string(), + resource: "res".to_string(), + ..Default::default() + } + }; + + // P0 trace whose root is fine but whose child failed (e.g. a caught + // downstream call): still an error trace, so it must be rescued. + let traces = vec![vec![make_span(1, 0, 0), make_span(2, 1, 1)]]; + + let (payload_info, _stats) = + processor.process_traces(config, tags_provider, header_tags, traces, 0, None); + let payload_info = payload_info.expect("errored-child P0 trace rescued"); + let backend_send_data = payload_info.builder.build(); + let TracerPayloadCollection::V07(backend_payloads) = backend_send_data.get_payloads() + else { + panic!("expected V07"); + }; + + let root = backend_payloads + .iter() + .flat_map(|tp| tp.chunks.iter()) + .flat_map(|c| c.spans.iter()) + .find(|s| s.span_id == 1) + .expect("rescued trace present"); + assert!( + root.metrics.contains_key("_dd.errors_sr"), + "_dd.errors_sr stamped on rescued root" + ); + } + /// Verifies that `process_traces` returns `None` for the backend payload when all /// traces are sampled out and `lambda_extension_compute_stats` is true. #[test] From b3366cb67552183dc5a936cb7f2ac6bf60879a5f Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 4 Aug 2026 14:12:51 -0400 Subject: [PATCH 04/13] fix(traces): honor explicit trace drops instead of rescuing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Traces dropped on purpose, either by a tracer sampling rule or by an explicit MANUAL_DROP, were being fed to the error sampler and could be sent to Datadog anyway when they contained an error. Only traces dropped by automatic sampling are now rescue candidates, matching the Datadog Agent. 🤖 --- bottlecap/src/traces/trace_processor.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index 79a356cfe..2415ebaec 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -64,7 +64,7 @@ impl StatsComputedBy { pub struct ServerlessTraceProcessor { pub obfuscation_config: Arc, /// Agent-side error sampler. On the `lambda_extension_compute_stats` path, - /// errored chunks that would be dropped (priority <= 0) get a second look + /// errored chunks that would be dropped (`AutoDrop`) get a second look /// and are rescued up to `apm_error_tps` traces/sec. Shared across /// invocations (std Mutex, not tokio: consulted from the synchronous /// `process_traces`) so its rolling-window rate limiter accumulates. @@ -73,7 +73,7 @@ pub struct ServerlessTraceProcessor { impl ServerlessTraceProcessor { /// Consult the error sampler for a chunk that would otherwise be dropped - /// (priority <= 0). Returns `true` to keep (rescue) the chunk. Only errored + /// (`AutoDrop`). Returns `true` to keep (rescue) the chunk. Only errored /// traces are candidates; on a keep, stamps `_dd.errors_sr` on the root span. fn rescue_error_chunk(&self, chunk: &mut pb::TraceChunk, env: &str, now_secs: i64) -> bool { let Ok(root_idx) = trace_utils::get_root_span_index(&chunk.spans) else { @@ -503,8 +503,8 @@ impl TraceProcessor for ServerlessTraceProcessor { // Remove sampled-out chunks so they won't be sent to Datadog. // Sampled-out chunks are preserved in payloads_for_stats above so their // stats are still counted. SamplerPriority::None (-128) means no explicit priority - // was set and the trace is kept; drop priorities are SamplerPriority::AutoDrop (0) - // and UserDrop (-1, not represented in SamplerPriority). + // was set and the trace is kept. Only SamplerPriority::AutoDrop (0) chunks are + // rescue candidates; negative priorities are explicit drops and are honored. if config.ext.lambda_extension_compute_stats && let TracerPayloadCollection::V07(ref mut tracer_payloads) = payload { @@ -521,8 +521,13 @@ impl TraceProcessor for ServerlessTraceProcessor { if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { return true; } - // Otherwise this chunk would be dropped; give errored ones a - // second look via the error sampler (rescue within budget). + // A negative priority is an explicit drop (a tracer sampling rule + // or MANUAL_DROP): honor it and never rescue, as the Agent does. + if chunk.priority < 0 { + return false; + } + // AutoDrop (0): give errored chunks a second look via the error + // sampler (rescue within budget). self.rescue_error_chunk(chunk, env, now_secs) }); } @@ -1361,8 +1366,8 @@ mod tests { } /// On the compute-stats path, the error sampler rescues errored chunks that - /// would otherwise be dropped (priority <= 0), stamping `_dd.errors_sr`, - /// while non-errored P0 chunks are still dropped. + /// would otherwise be dropped (`AutoDrop`), stamping `_dd.errors_sr`, while + /// non-errored P0 chunks and explicit user drops are still dropped. #[test] fn test_error_sampler_rescues_errored_p0_chunks() { use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig; @@ -1420,11 +1425,12 @@ mod tests { }; // trace 1: kept normally (priority 1). trace 2: errored P0 (rescued). - // trace 3: non-errored P0 (dropped). + // trace 3: non-errored P0 (dropped). trace 4: errored user drop (dropped). let traces = vec![ vec![make_span(1, 1.0, 0)], vec![make_span(2, 0.0, 1)], vec![make_span(3, 0.0, 0)], + vec![make_span(4, -1.0, 1)], ]; let (payload_info, _stats) = @@ -1446,6 +1452,7 @@ mod tests { assert!(kept.contains(&1), "priority-1 trace kept"); assert!(kept.contains(&2), "errored P0 trace rescued"); assert!(!kept.contains(&3), "non-errored P0 trace dropped"); + assert!(!kept.contains(&4), "errored user-drop trace not rescued"); let rescued_root = backend_payloads .iter() From a09a48a18f1ca2cda85c53ebb638b482dafb513c Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Tue, 4 Aug 2026 14:16:27 -0400 Subject: [PATCH 05/13] fix(traces): key error sampling on the env reported by the tracer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error sampler's per-signature rate limits were keyed on the extension's own DD_ENV, so when that was unset every trace shared one empty env and distinct services competed for the same budget. The env the tracer reported with the trace is now used instead, matching the Datadog Agent. 🤖 --- bottlecap/src/traces/trace_processor.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index 2415ebaec..3906f1097 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -508,7 +508,6 @@ impl TraceProcessor for ServerlessTraceProcessor { if config.ext.lambda_extension_compute_stats && let TracerPayloadCollection::V07(ref mut tracer_payloads) = payload { - let env = config.env.as_deref().unwrap_or_default(); let now_secs: i64 = std::time::UNIX_EPOCH .elapsed() .expect("unable to poll clock, unrecoverable") @@ -516,6 +515,9 @@ impl TraceProcessor for ServerlessTraceProcessor { .try_into() .unwrap_or_default(); for tp in tracer_payloads.iter_mut() { + // The sampler keys its per-signature rate limits on the env the + // tracer reported for this payload, as the Agent does. + let env = tp.env.as_str(); tp.chunks.retain_mut(|chunk| { // Explicit keeps and "no priority set" pass through unchanged. if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { From e379d954304e306e7bf3388a82af5453bced3d82 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Thu, 6 Aug 2026 16:17:55 -0400 Subject: [PATCH 06/13] fix(traces): keep error sampling alive after a panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A panic while the error sampler's lock was held poisoned the mutex, which silently disabled error-trace rescue for the rest of the sandbox's life. Recover through poisoning instead: the sampler holds only rolling-window counters, so a partially updated bucket costs far less than losing the feature entirely. 🤖 --- bottlecap/src/traces/trace_processor.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index 3906f1097..eec7d7e9d 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -109,10 +109,14 @@ impl ServerlessTraceProcessor { root_global_sample_rate: root.metrics.get("_sample_rate").copied().unwrap_or(1.0), spans: &views, }; - match self.error_sampler.lock() { - Ok(mut sampler) => sampler.sample(now_secs, &trace), - Err(_) => return false, // poisoned lock: fail safe to the drop - } + // Recover through poisoning: the sampler holds only rolling-window + // counters, so a partially updated bucket is far cheaper than + // disabling error rescue for the rest of the sandbox's life. + let mut sampler = self + .error_sampler + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + sampler.sample(now_secs, &trace) }; match decision { From 20affae61194d6e694277a1a8c33ded5650e5065 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Thu, 6 Aug 2026 16:25:28 -0400 Subject: [PATCH 07/13] fast path when error sampler is disabled Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- bottlecap/src/traces/trace_processor.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index eec7d7e9d..b559391d7 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -532,8 +532,9 @@ impl TraceProcessor for ServerlessTraceProcessor { if chunk.priority < 0 { return false; } - // AutoDrop (0): give errored chunks a second look via the error - // sampler (rescue within budget). + if config.ext.apm_error_tps <= 0.0 { + return false; + } self.rescue_error_chunk(chunk, env, now_secs) }); } From 328cf0523517d0964dc2d67dfb0c813e82756d5b Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Thu, 6 Aug 2026 17:53:31 -0400 Subject: [PATCH 08/13] refactor(traces): gate error rescue on the sampler's own disabled flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The disabled check re-derived "sampler is off" from apm_error_tps at the call site, a second definition of a condition the sampler already knows. It now asks the sampler via the new is_disabled(), hoisted above the payload loop so it costs one lock per flush instead of a check per chunk. Repins the four serverless-components deps to pick up is_disabled(), which also brings in non-finite client sample rate handling. 🤖 --- bottlecap/Cargo.lock | 8 ++++---- bottlecap/Cargo.toml | 8 ++++---- bottlecap/src/traces/trace_processor.rs | 14 +++++++++++++- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/bottlecap/Cargo.lock b/bottlecap/Cargo.lock index da02835c3..20e5b04c2 100644 --- a/bottlecap/Cargo.lock +++ b/bottlecap/Cargo.lock @@ -810,7 +810,7 @@ dependencies = [ [[package]] name = "datadog-agent-config" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" +source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" dependencies = [ "datadog-opentelemetry", "dogstatsd", @@ -828,12 +828,12 @@ dependencies = [ [[package]] name = "datadog-agent-trace-sampler" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" +source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" [[package]] name = "datadog-fips" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" +source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" dependencies = [ "reqwest", "rustls", @@ -982,7 +982,7 @@ dependencies = [ [[package]] name = "dogstatsd" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=3759fae9ba4afb2abbb886fc5e596dec55e3c83b#3759fae9ba4afb2abbb886fc5e596dec55e3c83b" +source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" dependencies = [ "datadog-protos", "ddsketch-agent", diff --git a/bottlecap/Cargo.toml b/bottlecap/Cargo.toml index a4b5da378..ef993d9e3 100644 --- a/bottlecap/Cargo.toml +++ b/bottlecap/Cargo.toml @@ -82,10 +82,10 @@ libdd-trace-normalization = { git = "https://github.com/DataDog/libdatadog", rev libdd-trace-obfuscation = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } libdd-trace-stats = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } datadog-opentelemetry = { git = "https://github.com/DataDog/dd-trace-rs", rev = "50bfea8755b75e448a80ac04d53fa7edd414eefe", default-features = false, features = ["_unstable_propagation"] } -dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } -datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } -datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } -datadog-agent-trace-sampler = { git = "https://github.com/DataDog/serverless-components", rev = "3759fae9ba4afb2abbb886fc5e596dec55e3c83b", default-features = false } +dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } +datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } +datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } +datadog-agent-trace-sampler = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } libddwaf = { version = "1.28.1", git = "https://github.com/DataDog/libddwaf-rust", rev = "d1534a158d976bd4f747bf9fcc58e0712d2d17fc", default-features = false, features = ["serde"] } [dev-dependencies] diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index b559391d7..cf62b76fe 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -518,6 +518,14 @@ impl TraceProcessor for ServerlessTraceProcessor { .as_secs() .try_into() .unwrap_or_default(); + // Ask the sampler itself whether it is disabled, rather than + // re-deriving that from config: one lock here instead of per chunk, + // and no second definition of "disabled" to keep in sync. + let rescue_enabled = !self + .error_sampler + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .is_disabled(); for tp in tracer_payloads.iter_mut() { // The sampler keys its per-signature rate limits on the env the // tracer reported for this payload, as the Agent does. @@ -532,9 +540,13 @@ impl TraceProcessor for ServerlessTraceProcessor { if chunk.priority < 0 { return false; } - if config.ext.apm_error_tps <= 0.0 { + // A disabled sampler drops every chunk; skip building views + // for a decision that is already known. + if !rescue_enabled { return false; } + // AutoDrop (0): give errored chunks a second look via the error + // sampler (rescue within budget). self.rescue_error_chunk(chunk, env, now_secs) }); } From f118cf4e48fb82dc3a29746c157d94ff84759d37 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 7 Aug 2026 18:49:45 -0400 Subject: [PATCH 09/13] feat(traces): default error sampler to AlwaysKeep mode Bump the serverless-components pin to 54e570ae, which adds the dual-mode ErrorSamplerMode (AlwaysKeep | RateLimited) to datadog-agent-trace-sampler and makes `mode` a required field on ErrorSamplerConfig. The rev also carries a fix for non-finite client sample rates on the error sample rate path. Hardcode the production sampler to AlwaysKeep: Lambda's per-invocation trace volume is low, so the RateLimited budget rarely binds, and freeze/thaw breaks its 30s wall-clock window. Wiring the mode through config is deferred to a follow-up. Correct the doc comments on apm_error_tps, apm_extra_sample_rate, and ServerlessTraceProcessor::error_sampler, which described RateLimited behavior that no longer runs in production. Refs APMSVLS-469 --- bottlecap/Cargo.lock | 8 ++++---- bottlecap/Cargo.toml | 8 ++++---- bottlecap/src/bin/bottlecap/main.rs | 5 +++++ bottlecap/src/config/mod.rs | 9 +++++++++ bottlecap/src/traces/trace_processor.rs | 10 +++++++--- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/bottlecap/Cargo.lock b/bottlecap/Cargo.lock index 20e5b04c2..c87abf658 100644 --- a/bottlecap/Cargo.lock +++ b/bottlecap/Cargo.lock @@ -810,7 +810,7 @@ dependencies = [ [[package]] name = "datadog-agent-config" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" +source = "git+https://github.com/DataDog/serverless-components?rev=54e570ae60d169acfd48800b1805a9bbdbbeafb2#54e570ae60d169acfd48800b1805a9bbdbbeafb2" dependencies = [ "datadog-opentelemetry", "dogstatsd", @@ -828,12 +828,12 @@ dependencies = [ [[package]] name = "datadog-agent-trace-sampler" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" +source = "git+https://github.com/DataDog/serverless-components?rev=54e570ae60d169acfd48800b1805a9bbdbbeafb2#54e570ae60d169acfd48800b1805a9bbdbbeafb2" [[package]] name = "datadog-fips" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" +source = "git+https://github.com/DataDog/serverless-components?rev=54e570ae60d169acfd48800b1805a9bbdbbeafb2#54e570ae60d169acfd48800b1805a9bbdbbeafb2" dependencies = [ "reqwest", "rustls", @@ -982,7 +982,7 @@ dependencies = [ [[package]] name = "dogstatsd" version = "0.1.0" -source = "git+https://github.com/DataDog/serverless-components?rev=1a2fb1894542926501ba23407c923e27a1e2d13c#1a2fb1894542926501ba23407c923e27a1e2d13c" +source = "git+https://github.com/DataDog/serverless-components?rev=54e570ae60d169acfd48800b1805a9bbdbbeafb2#54e570ae60d169acfd48800b1805a9bbdbbeafb2" dependencies = [ "datadog-protos", "ddsketch-agent", diff --git a/bottlecap/Cargo.toml b/bottlecap/Cargo.toml index ef993d9e3..9623a68ad 100644 --- a/bottlecap/Cargo.toml +++ b/bottlecap/Cargo.toml @@ -82,10 +82,10 @@ libdd-trace-normalization = { git = "https://github.com/DataDog/libdatadog", rev libdd-trace-obfuscation = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } libdd-trace-stats = { git = "https://github.com/DataDog/libdatadog", rev = "85ce322a1dcb1eda7df9bcc021223b2d1a236783", default-features = false } datadog-opentelemetry = { git = "https://github.com/DataDog/dd-trace-rs", rev = "50bfea8755b75e448a80ac04d53fa7edd414eefe", default-features = false, features = ["_unstable_propagation"] } -dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } -datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } -datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } -datadog-agent-trace-sampler = { git = "https://github.com/DataDog/serverless-components", rev = "1a2fb1894542926501ba23407c923e27a1e2d13c", default-features = false } +dogstatsd = { git = "https://github.com/DataDog/serverless-components", rev = "54e570ae60d169acfd48800b1805a9bbdbbeafb2", default-features = false } +datadog-fips = { git = "https://github.com/DataDog/serverless-components", rev = "54e570ae60d169acfd48800b1805a9bbdbbeafb2", default-features = false } +datadog-agent-config = { git = "https://github.com/DataDog/serverless-components", rev = "54e570ae60d169acfd48800b1805a9bbdbbeafb2", default-features = false } +datadog-agent-trace-sampler = { git = "https://github.com/DataDog/serverless-components", rev = "54e570ae60d169acfd48800b1805a9bbdbbeafb2", default-features = false } libddwaf = { version = "1.28.1", git = "https://github.com/DataDog/libddwaf-rust", rev = "d1534a158d976bd4f747bf9fcc58e0712d2d17fc", default-features = false, features = ["serde"] } [dev-dependencies] diff --git a/bottlecap/src/bin/bottlecap/main.rs b/bottlecap/src/bin/bottlecap/main.rs index ee48950ee..96c7721ec 100644 --- a/bottlecap/src/bin/bottlecap/main.rs +++ b/bottlecap/src/bin/bottlecap/main.rs @@ -1165,6 +1165,11 @@ fn start_trace_agent( error_sampler: Arc::new(std::sync::Mutex::new( datadog_agent_trace_sampler::ErrorsSampler::new( datadog_agent_trace_sampler::ErrorSamplerConfig { + // Hardcoded for now (config wiring via DD_APM_ERROR_SAMPLER_MODE + // is deferred): Lambda's per-invocation trace volume is low and + // freeze/thaw breaks the RateLimited mode's 30s wall-clock + // window, so AlwaysKeep is the right default. See APMSVLS-469. + mode: datadog_agent_trace_sampler::ErrorSamplerMode::AlwaysKeep, target_tps: config.ext.apm_error_tps, extra_sample_rate: config.ext.apm_extra_sample_rate, }, diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index 3c959a477..1de9eb01d 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -71,10 +71,19 @@ pub struct LambdaConfig { /// agent-side error sampler when the extension computes stats. Matches the /// Go trace agent's `apm_config.errors_per_second`. Default 10.0; `0` /// disables the sampler (no rescue). + /// + /// Note: the sampler currently runs in `AlwaysKeep` mode (hardcoded in + /// `main.rs`), where this value acts only as an on/off switch — `0` (or + /// negative) disables rescue, any positive value rescues every errored + /// chunk without a rate cap. The traces/sec budget is only enforced in + /// `RateLimited` mode. See APMSVLS-469. pub apm_error_tps: f64, /// `DD_APM_EXTRA_SAMPLE_RATE` — extra raw sampling rate applied on top of /// the computed error-sampler rate. Matches the Go trace agent's /// `apm_config.extra_sample_rate`. Default 1.0. + /// + /// Note: only meaningful in `RateLimited` mode; inert while the sampler is + /// hardcoded to `AlwaysKeep`. See APMSVLS-469. pub apm_extra_sample_rate: f64, pub span_dedup_timeout: Option, diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index cf62b76fe..657c371f8 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -64,10 +64,14 @@ impl StatsComputedBy { pub struct ServerlessTraceProcessor { pub obfuscation_config: Arc, /// Agent-side error sampler. On the `lambda_extension_compute_stats` path, - /// errored chunks that would be dropped (`AutoDrop`) get a second look - /// and are rescued up to `apm_error_tps` traces/sec. Shared across + /// errored chunks that would be dropped (`AutoDrop`) get a second look and + /// may be rescued. In the current `AlwaysKeep` mode every errored chunk is + /// rescued (`apm_error_tps` only gates on/off); the `apm_error_tps` + /// traces/sec budget applies in `RateLimited` mode. Shared across /// invocations (std Mutex, not tokio: consulted from the synchronous - /// `process_traces`) so its rolling-window rate limiter accumulates. + /// `process_traces`) so `RateLimited`'s rolling-window rate limiter + /// accumulates. `AlwaysKeep` holds no state, so the lock is uncontended + /// bookkeeping there. pub error_sampler: Arc>, } From 736e1814eb2fa662a7a0eae4426df5992a7ab016 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 7 Aug 2026 18:54:55 -0400 Subject: [PATCH 10/13] feat(config): gate error sampler behind a boolean Replace apm_error_tps and apm_extra_sample_rate with a single apm_error_sampler_enabled toggle (DD_APM_ERROR_SAMPLER_ENABLED). Both replaced knobs were introduced earlier on this branch and never released, so there is no compatibility constraint. Neither carries its advertised meaning in AlwaysKeep mode: extra_sample_rate is ignored outright, and error_tps degrades to an on/off switch, so DD_APM_ERROR_TPS=25 and =1 behave identically. Exposing a rate cap that is not enforced is worse than not exposing one. Both return, with their real semantics, when RateLimited is wired up. Default false while the feature rolls out as opt-in; the plan is to flip it once it has soaked. AlwaysKeep derives its disabled flag from target_tps <= 0.0, so the boolean maps onto 1.0 / 0.0 and the disabled path short-circuits before any SpanView is built. Refs APMSVLS-469 --- bottlecap/src/bin/bottlecap/main.rs | 12 ++++- bottlecap/src/config/mod.rs | 71 +++++++++---------------- bottlecap/src/traces/trace_processor.rs | 11 ++-- 3 files changed, 41 insertions(+), 53 deletions(-) diff --git a/bottlecap/src/bin/bottlecap/main.rs b/bottlecap/src/bin/bottlecap/main.rs index 96c7721ec..964feb4d8 100644 --- a/bottlecap/src/bin/bottlecap/main.rs +++ b/bottlecap/src/bin/bottlecap/main.rs @@ -1170,8 +1170,16 @@ fn start_trace_agent( // freeze/thaw breaks the RateLimited mode's 30s wall-clock // window, so AlwaysKeep is the right default. See APMSVLS-469. mode: datadog_agent_trace_sampler::ErrorSamplerMode::AlwaysKeep, - target_tps: config.ext.apm_error_tps, - extra_sample_rate: config.ext.apm_extra_sample_rate, + // AlwaysKeep derives its disabled flag from `target_tps <= 0.0`, + // so the boolean maps onto any positive value vs. zero. The + // magnitude is unused until RateLimited is wired up, which is + // when DD_APM_ERROR_TPS becomes meaningful and gets exposed. + target_tps: if config.ext.apm_error_sampler_enabled { + 1.0 + } else { + 0.0 + }, + extra_sample_rate: 1.0, }, ), )), diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index 1de9eb01d..4c98d1a5c 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -67,24 +67,18 @@ pub struct LambdaConfig { pub capture_lambda_payload_max_depth: u32, pub lambda_extension_compute_stats: bool, - /// `DD_APM_ERROR_TPS` — target error traces per second rescued by the - /// agent-side error sampler when the extension computes stats. Matches the - /// Go trace agent's `apm_config.errors_per_second`. Default 10.0; `0` - /// disables the sampler (no rescue). + /// `DD_APM_ERROR_SAMPLER_ENABLED` — whether the agent-side error sampler + /// rescues errored trace chunks that would otherwise be dropped, on the + /// `lambda_extension_compute_stats` path. /// - /// Note: the sampler currently runs in `AlwaysKeep` mode (hardcoded in - /// `main.rs`), where this value acts only as an on/off switch — `0` (or - /// negative) disables rescue, any positive value rescues every errored - /// chunk without a rate cap. The traces/sec budget is only enforced in - /// `RateLimited` mode. See APMSVLS-469. - pub apm_error_tps: f64, - /// `DD_APM_EXTRA_SAMPLE_RATE` — extra raw sampling rate applied on top of - /// the computed error-sampler rate. Matches the Go trace agent's - /// `apm_config.extra_sample_rate`. Default 1.0. - /// - /// Note: only meaningful in `RateLimited` mode; inert while the sampler is - /// hardcoded to `AlwaysKeep`. See APMSVLS-469. - pub apm_extra_sample_rate: f64, + /// Defaults to `false` while the feature rolls out as opt-in; the plan is + /// to flip the default to `true` once it has soaked. The sampler runs in + /// `AlwaysKeep` mode (hardcoded in `main.rs`), so this is a plain on/off + /// switch: enabled rescues every errored chunk. The Go agent's + /// rate-limiting knobs (`apm_config.errors_per_second` / + /// `extra_sample_rate`) are intentionally not exposed yet — they only have + /// meaning in `RateLimited` mode and will be added with it. See APMSVLS-469. + pub apm_error_sampler_enabled: bool, pub span_dedup_timeout: Option, pub api_key_secret_reload_interval: Option, @@ -115,8 +109,7 @@ impl Default for LambdaConfig { capture_lambda_payload: false, capture_lambda_payload_max_depth: 10, lambda_extension_compute_stats: false, - apm_error_tps: 10.0, - apm_extra_sample_rate: 1.0, + apm_error_sampler_enabled: false, span_dedup_timeout: None, api_key_secret_reload_interval: None, serverless_appsec_enabled: false, @@ -175,14 +168,10 @@ pub struct LambdaConfigSource { #[serde(deserialize_with = "deser_opt_bool")] pub lambda_extension_compute_stats: Option, - /// `DD_APM_ERROR_TPS` — error sampler target traces/sec. Flat env/YAML key - /// (`apm_error_tps`), unlike the Go agent's nested `apm_config.errors_per_second`. - #[serde(deserialize_with = "deser_opt_lossless")] - pub apm_error_tps: Option, - /// `DD_APM_EXTRA_SAMPLE_RATE` — error sampler extra sample rate. Flat - /// env/YAML key (`apm_extra_sample_rate`). - #[serde(deserialize_with = "deser_opt_lossless")] - pub apm_extra_sample_rate: Option, + /// `DD_APM_ERROR_SAMPLER_ENABLED` — toggles the agent-side error sampler. + /// Flat env/YAML key (`apm_error_sampler_enabled`). + #[serde(deserialize_with = "deser_opt_bool")] + pub apm_error_sampler_enabled: Option, #[serde(deserialize_with = "deser_dur_secs_ignore_zero")] pub span_dedup_timeout: Option, @@ -230,8 +219,7 @@ impl DatadogConfigExtension for LambdaConfig { capture_lambda_payload, capture_lambda_payload_max_depth, lambda_extension_compute_stats, - apm_error_tps, - apm_extra_sample_rate, + apm_error_sampler_enabled, serverless_appsec_enabled, appsec_waf_timeout, api_security_enabled, @@ -570,37 +558,30 @@ mod lambda_config_tests { assert!(!config.ext.lambda_extension_compute_stats); } - // ---- error sampler (apm_error_tps / apm_extra_sample_rate) ---- + // ---- error sampler (apm_error_sampler_enabled) ---- #[test] - fn apm_error_tps_defaults_to_ten() { + fn apm_error_sampler_enabled_defaults_to_false() { let config = load(|_| Ok(())); - assert_eq!(config.ext.apm_error_tps, 10.0); - assert_eq!(config.ext.apm_extra_sample_rate, 1.0); + assert!(!config.ext.apm_error_sampler_enabled); } #[test] - fn apm_error_tps_from_env() { + fn apm_error_sampler_enabled_from_env() { let config = load(|jail| { - jail.set_env("DD_APM_ERROR_TPS", "25"); - jail.set_env("DD_APM_EXTRA_SAMPLE_RATE", "0.5"); + jail.set_env("DD_APM_ERROR_SAMPLER_ENABLED", "true"); Ok(()) }); - assert_eq!(config.ext.apm_error_tps, 25.0); - assert_eq!(config.ext.apm_extra_sample_rate, 0.5); + assert!(config.ext.apm_error_sampler_enabled); } #[test] - fn apm_error_tps_from_yaml() { + fn apm_error_sampler_enabled_from_yaml() { let config = load(|jail| { - jail.create_file( - "datadog.yaml", - "apm_error_tps: 0\napm_extra_sample_rate: 2\n", - )?; + jail.create_file("datadog.yaml", "apm_error_sampler_enabled: true\n")?; Ok(()) }); - assert_eq!(config.ext.apm_error_tps, 0.0); - assert_eq!(config.ext.apm_extra_sample_rate, 2.0); + assert!(config.ext.apm_error_sampler_enabled); } // ---- Duration fields ---- diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index 657c371f8..7615f330e 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -66,12 +66,11 @@ pub struct ServerlessTraceProcessor { /// Agent-side error sampler. On the `lambda_extension_compute_stats` path, /// errored chunks that would be dropped (`AutoDrop`) get a second look and /// may be rescued. In the current `AlwaysKeep` mode every errored chunk is - /// rescued (`apm_error_tps` only gates on/off); the `apm_error_tps` - /// traces/sec budget applies in `RateLimited` mode. Shared across - /// invocations (std Mutex, not tokio: consulted from the synchronous - /// `process_traces`) so `RateLimited`'s rolling-window rate limiter - /// accumulates. `AlwaysKeep` holds no state, so the lock is uncontended - /// bookkeeping there. + /// rescued, gated only by `apm_error_sampler_enabled`; a traces/sec budget + /// applies in `RateLimited` mode. Shared across invocations (std Mutex, not + /// tokio: consulted from the synchronous `process_traces`) so + /// `RateLimited`'s rolling-window rate limiter accumulates. `AlwaysKeep` + /// holds no state, so the lock is uncontended bookkeeping there. pub error_sampler: Arc>, } From f005c419178ed4f5a1b03f391ee4090a246c0494 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 7 Aug 2026 19:16:14 -0400 Subject: [PATCH 11/13] fix(traces): keep the extension alive on a bad clock reading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build the shipped error sampler (AlwaysKeep, enabled by apm_error_sampler_enabled) in one place so tests exercise the same configuration that ships, and cover the disabled default with a test. A failed clock read no longer aborts the extension, and the clock is only read when error rescue is enabled. In AlwaysKeep mode the sampler ignores span contents, so only the root span view is built instead of one per span. 🤖 --- bottlecap/src/bin/bottlecap/main.rs | 22 +-- bottlecap/src/traces/trace_processor.rs | 246 +++++++++++++++++------- bottlecap/tests/apm_integration_test.rs | 10 +- 3 files changed, 181 insertions(+), 97 deletions(-) diff --git a/bottlecap/src/bin/bottlecap/main.rs b/bottlecap/src/bin/bottlecap/main.rs index 964feb4d8..8f76fba5d 100644 --- a/bottlecap/src/bin/bottlecap/main.rs +++ b/bottlecap/src/bin/bottlecap/main.rs @@ -1162,27 +1162,7 @@ fn start_trace_agent( let trace_processor = Arc::new(trace_processor::ServerlessTraceProcessor { obfuscation_config: Arc::new(obfuscation_config), - error_sampler: Arc::new(std::sync::Mutex::new( - datadog_agent_trace_sampler::ErrorsSampler::new( - datadog_agent_trace_sampler::ErrorSamplerConfig { - // Hardcoded for now (config wiring via DD_APM_ERROR_SAMPLER_MODE - // is deferred): Lambda's per-invocation trace volume is low and - // freeze/thaw breaks the RateLimited mode's 30s wall-clock - // window, so AlwaysKeep is the right default. See APMSVLS-469. - mode: datadog_agent_trace_sampler::ErrorSamplerMode::AlwaysKeep, - // AlwaysKeep derives its disabled flag from `target_tps <= 0.0`, - // so the boolean maps onto any positive value vs. zero. The - // magnitude is unused until RateLimited is wired up, which is - // when DD_APM_ERROR_TPS becomes meaningful and gets exposed. - target_tps: if config.ext.apm_error_sampler_enabled { - 1.0 - } else { - 0.0 - }, - extra_sample_rate: 1.0, - }, - ), - )), + error_sampler: trace_processor::new_error_sampler(config.ext.apm_error_sampler_enabled), }); let (span_dedup_service, span_dedup_handle) = span_dedup_service::DedupService::new(); diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index 7615f330e..c847b1559 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -15,7 +15,9 @@ use crate::traces::{ LAMBDA_RUNTIME_URL_PREFIX, LAMBDA_STATSD_URL_PREFIX, }; use async_trait::async_trait; -use datadog_agent_trace_sampler::{ErrorsSampler, SampleDecision, SpanView, TraceView}; +use datadog_agent_trace_sampler::{ + ErrorSamplerConfig, ErrorSamplerMode, ErrorsSampler, SampleDecision, SpanView, TraceView, +}; use libdd_common::Endpoint; use libdd_trace_obfuscation::obfuscate::obfuscate_span; use libdd_trace_obfuscation::obfuscation_config; @@ -74,7 +76,95 @@ pub struct ServerlessTraceProcessor { pub error_sampler: Arc>, } +/// Borrow-only view of a span for the error sampler. +fn span_view(span: &Span) -> SpanView<'_> { + SpanView { + service: &span.service, + name: &span.name, + resource: &span.resource, + error: span.error != 0, + http_status_code: span.meta.get("http.status_code").map(String::as_str), + error_type: span.meta.get("error.type").map(String::as_str), + } +} + +/// Builds the error sampler as the extension ships it, enabled or disabled by +/// `apm_error_sampler_enabled`. Shared by `main` and the tests so tests +/// exercise the shipping configuration. +/// +/// The mode is hardcoded for now (config wiring via `DD_APM_ERROR_SAMPLER_MODE` +/// is deferred): Lambda's per-invocation trace volume is low and freeze/thaw +/// breaks the `RateLimited` mode's 30s wall-clock window, so `AlwaysKeep` is +/// the right default. See APMSVLS-469. +#[must_use] +pub fn new_error_sampler(enabled: bool) -> Arc> { + Arc::new(std::sync::Mutex::new(ErrorsSampler::new( + ErrorSamplerConfig { + mode: ErrorSamplerMode::AlwaysKeep, + // AlwaysKeep derives its disabled flag from `target_tps <= 0.0`, so + // the boolean maps onto any positive value vs. zero. The magnitude + // is unused until RateLimited is wired up, which is when + // DD_APM_ERROR_TPS becomes meaningful and gets exposed. + target_tps: if enabled { 1.0 } else { 0.0 }, + extra_sample_rate: 1.0, + }, + ))) +} + impl ServerlessTraceProcessor { + /// Removes sampled-out chunks so they won't be sent to Datadog, then drops + /// any payload left without chunks. `SamplerPriority::None` (-128) means no + /// explicit priority was set and the trace is kept. Only + /// `SamplerPriority::AutoDrop` (0) chunks are rescue candidates; negative + /// priorities are explicit drops and are honored. + fn drop_sampled_out_chunks(&self, tracer_payloads: &mut Vec) { + // Ask the sampler itself whether it is disabled, rather than + // re-deriving that from config: one lock here instead of per chunk, + // and no second definition of "disabled" to keep in sync. + let rescue_enabled = !self + .error_sampler + .lock() + .expect("error sampler poisoned") + .is_disabled(); + // Only RateLimited's rolling window reads the clock, and only rescue + // candidates reach it. + let now_secs: i64 = if rescue_enabled { + std::time::UNIX_EPOCH + .elapsed() + .unwrap_or_default() + .as_secs() + .try_into() + .unwrap_or_default() + } else { + 0 + }; + for tp in tracer_payloads.iter_mut() { + // The sampler keys its per-signature rate limits on the env the + // tracer reported for this payload, as the Agent does. + let env = tp.env.as_str(); + tp.chunks.retain_mut(|chunk| { + // Explicit keeps and "no priority set" pass through unchanged. + if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { + return true; + } + // A negative priority is an explicit drop (a tracer sampling rule + // or MANUAL_DROP): honor it and never rescue, as the Agent does. + if chunk.priority < 0 { + return false; + } + // A disabled sampler drops every chunk; skip building views + // for a decision that is already known. + if !rescue_enabled { + return false; + } + // AutoDrop (0): give errored chunks a second look via the error + // sampler (rescue within budget). + self.rescue_error_chunk(chunk, env, now_secs) + }); + } + tracer_payloads.retain(|tp| !tp.chunks.is_empty()); + } + /// Consult the error sampler for a chunk that would otherwise be dropped /// (`AutoDrop`). Returns `true` to keep (rescue) the chunk. Only errored /// traces are candidates; on a keep, stamps `_dd.errors_sr` on the root span. @@ -92,33 +182,27 @@ impl ServerlessTraceProcessor { // Build the borrow-only views in a scope so they (and their immutable // borrow of chunk.spans) drop before the `_dd.errors_sr` mutation below. let decision = { - let views: Vec = chunk - .spans - .iter() - .map(|s| SpanView { - service: &s.service, - name: &s.name, - resource: &s.resource, - error: s.error != 0, - http_status_code: s.meta.get("http.status_code").map(String::as_str), - error_type: s.meta.get("error.type").map(String::as_str), - }) - .collect(); + let mut sampler = self.error_sampler.lock().expect("error sampler poisoned"); let root = &chunk.spans[root_idx]; + // AlwaysKeep ignores the spans apart from a bounds check on + // root_index, so only the root view is built there. RateLimited + // needs every span to compute the trace signature. + let root_view; + let all_views; + let (spans, root_index) = if matches!(*sampler, ErrorsSampler::AlwaysKeep { .. }) { + root_view = [span_view(root)]; + (&root_view[..], 0) + } else { + all_views = chunk.spans.iter().map(span_view).collect::>(); + (&all_views[..], root_idx) + }; let trace = TraceView { env, trace_id: root.trace_id, - root_index: root_idx, + root_index, root_global_sample_rate: root.metrics.get("_sample_rate").copied().unwrap_or(1.0), - spans: &views, + spans, }; - // Recover through poisoning: the sampler holds only rolling-window - // counters, so a partially updated bucket is far cheaper than - // disabling error rescue for the rest of the sandbox's life. - let mut sampler = self - .error_sampler - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); sampler.sample(now_secs, &trace) }; @@ -507,53 +591,12 @@ impl TraceProcessor for ServerlessTraceProcessor { } }; - // Remove sampled-out chunks so they won't be sent to Datadog. - // Sampled-out chunks are preserved in payloads_for_stats above so their - // stats are still counted. SamplerPriority::None (-128) means no explicit priority - // was set and the trace is kept. Only SamplerPriority::AutoDrop (0) chunks are - // rescue candidates; negative priorities are explicit drops and are honored. + // Sampled-out chunks are preserved in payloads_for_stats above, so their + // stats are still counted after they are removed here. if config.ext.lambda_extension_compute_stats && let TracerPayloadCollection::V07(ref mut tracer_payloads) = payload { - let now_secs: i64 = std::time::UNIX_EPOCH - .elapsed() - .expect("unable to poll clock, unrecoverable") - .as_secs() - .try_into() - .unwrap_or_default(); - // Ask the sampler itself whether it is disabled, rather than - // re-deriving that from config: one lock here instead of per chunk, - // and no second definition of "disabled" to keep in sync. - let rescue_enabled = !self - .error_sampler - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .is_disabled(); - for tp in tracer_payloads.iter_mut() { - // The sampler keys its per-signature rate limits on the env the - // tracer reported for this payload, as the Agent does. - let env = tp.env.as_str(); - tp.chunks.retain_mut(|chunk| { - // Explicit keeps and "no priority set" pass through unchanged. - if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { - return true; - } - // A negative priority is an explicit drop (a tracer sampling rule - // or MANUAL_DROP): honor it and never rescue, as the Agent does. - if chunk.priority < 0 { - return false; - } - // A disabled sampler drops every chunk; skip building views - // for a decision that is already known. - if !rescue_enabled { - return false; - } - // AutoDrop (0): give errored chunks a second look via the error - // sampler (rescue within budget). - self.rescue_error_chunk(chunk, env, now_secs) - }); - } - tracer_payloads.retain(|tp| !tp.chunks.is_empty()); + self.drop_sampled_out_chunks(tracer_payloads); if tracer_payloads.is_empty() { return (None, payloads_for_stats); } @@ -701,14 +744,12 @@ impl SendingTraceProcessor { } } -/// Default error sampler for constructing `ServerlessTraceProcessor` in tests +/// Enabled error sampler for constructing `ServerlessTraceProcessor` in tests /// (sampler behavior itself is unit-tested in the `datadog-agent-trace-sampler` /// crate; these call sites just need a value). #[cfg(test)] pub(crate) fn default_error_sampler() -> Arc> { - Arc::new(std::sync::Mutex::new(ErrorsSampler::new( - datadog_agent_trace_sampler::ErrorSamplerConfig::default(), - ))) + new_error_sampler(true) } #[cfg(test)] @@ -1571,6 +1612,71 @@ mod tests { ); } + /// With the error sampler disabled (the shipping default, + /// `apm_error_sampler_enabled: false`), errored P0 chunks are dropped as + /// before: no rescue, no `_dd.errors_sr`. + #[test] + fn test_disabled_error_sampler_drops_errored_p0_chunks() { + use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig; + + let config = Arc::new(Config { + apm_dd_url: "https://trace.agent.datadoghq.com".to_string(), + ext: crate::config::LambdaConfig { + lambda_extension_compute_stats: true, + ..Default::default() + }, + ..Config::default() + }); + let tags_provider = Arc::new(Provider::new( + config.clone(), + "lambda".to_string(), + &std::collections::HashMap::from([( + "function_arn".to_string(), + "test-arn".to_string(), + )]), + )); + let processor = ServerlessTraceProcessor { + obfuscation_config: Arc::new( + ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), + ), + error_sampler: new_error_sampler(false), + }; + + let header_tags = tracer_header_tags::TracerHeaderTags { + lang: "rust", + lang_version: "1.0", + lang_interpreter: "", + lang_vendor: "", + tracer_version: "1.0", + container_id: "", + client_computed_top_level: false, + client_computed_stats: false, + dropped_p0_traces: 0, + dropped_p0_spans: 0, + }; + + let mut metrics = HashMap::new(); + metrics.insert("_sampling_priority_v1".to_string(), 0.0); + let traces = vec![vec![pb::Span { + trace_id: 1, + span_id: 1, + parent_id: 0, + error: 1, + metrics, + service: "svc".to_string(), + name: "op".to_string(), + resource: "res".to_string(), + ..Default::default() + }]]; + + let (payload_info, _stats) = + processor.process_traces(config, tags_provider, header_tags, traces, 0, None); + assert!( + payload_info.is_none(), + "errored P0 trace must stay dropped when the error sampler is disabled" + ); + } + /// Verifies that `process_traces` returns `None` for the backend payload when all /// traces are sampled out and `lambda_extension_compute_stats` is true. #[test] diff --git a/bottlecap/tests/apm_integration_test.rs b/bottlecap/tests/apm_integration_test.rs index 2f1429d0b..63f06ef29 100644 --- a/bottlecap/tests/apm_integration_test.rs +++ b/bottlecap/tests/apm_integration_test.rs @@ -30,7 +30,9 @@ use bottlecap::traces::stats_generator::StatsGenerator; use bottlecap::traces::trace_aggregator::SendDataBuilderInfo; use bottlecap::traces::trace_aggregator_service::AggregatorService; use bottlecap::traces::trace_flusher::TraceFlusher; -use bottlecap::traces::trace_processor::{SendingTraceProcessor, ServerlessTraceProcessor}; +use bottlecap::traces::trace_processor::{ + SendingTraceProcessor, ServerlessTraceProcessor, new_error_sampler, +}; use dogstatsd::api_key::ApiKeyFactory; use libdd_common::Endpoint; use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig; @@ -339,11 +341,7 @@ async fn run_processor_pipeline_with_traces( obfuscation_config: Arc::new( ObfuscationConfig::new().expect("Failed to create ObfuscationConfig"), ), - error_sampler: Arc::new(std::sync::Mutex::new( - datadog_agent_trace_sampler::ErrorsSampler::new( - datadog_agent_trace_sampler::ErrorSamplerConfig::default(), - ), - )), + error_sampler: new_error_sampler(true), }), trace_tx, stats_generator: Arc::new(StatsGenerator::new(concentrator_handle.clone())), From 0ccfcaa6cb81cf9225186f3467e4f7709aeffc88 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 7 Aug 2026 19:16:14 -0400 Subject: [PATCH 12/13] chore(config): drop an unused clippy allow in the config tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The float_cmp allow no longer matches any comparison in the test module and would mask a real one. 🤖 --- bottlecap/src/config/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index 4c98d1a5c..052d010ca 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -254,7 +254,6 @@ impl DatadogConfigExtension for LambdaConfig { #[cfg_attr(coverage_nightly, coverage(off))] // Test modules skew coverage metrics #[cfg(test)] #[allow(clippy::unwrap_used)] -#[allow(clippy::float_cmp)] // exact, representable config values (10.0, 1.0, ...) parsed deterministically mod lambda_config_tests { use datadog_agent_config::{ Config as UpstreamConfig, flush_strategy::PeriodicStrategy, get_config_with_extension, From e822993f83e91535bd6c1dde18948d2121ed65d2 Mon Sep 17 00:00:00 2001 From: Lucas Pimentel Date: Fri, 7 Aug 2026 19:25:12 -0400 Subject: [PATCH 13/13] docs(traces): trim the error sampler comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Condense the doc and inline comments added with the error sampler: drop roadmap notes, references to prior behavior, and comments that restate the code they sit above. 🤖 --- bottlecap/src/config/mod.rs | 17 ++------ bottlecap/src/traces/trace_processor.rs | 57 +++++++++---------------- 2 files changed, 23 insertions(+), 51 deletions(-) diff --git a/bottlecap/src/config/mod.rs b/bottlecap/src/config/mod.rs index 052d010ca..648eff8da 100644 --- a/bottlecap/src/config/mod.rs +++ b/bottlecap/src/config/mod.rs @@ -67,17 +67,10 @@ pub struct LambdaConfig { pub capture_lambda_payload_max_depth: u32, pub lambda_extension_compute_stats: bool, - /// `DD_APM_ERROR_SAMPLER_ENABLED` — whether the agent-side error sampler - /// rescues errored trace chunks that would otherwise be dropped, on the - /// `lambda_extension_compute_stats` path. - /// - /// Defaults to `false` while the feature rolls out as opt-in; the plan is - /// to flip the default to `true` once it has soaked. The sampler runs in - /// `AlwaysKeep` mode (hardcoded in `main.rs`), so this is a plain on/off - /// switch: enabled rescues every errored chunk. The Go agent's - /// rate-limiting knobs (`apm_config.errors_per_second` / - /// `extra_sample_rate`) are intentionally not exposed yet — they only have - /// meaning in `RateLimited` mode and will be added with it. See APMSVLS-469. + /// `DD_APM_ERROR_SAMPLER_ENABLED`: rescue errored trace chunks that would + /// otherwise be dropped, on the `lambda_extension_compute_stats` path. The + /// sampler runs in `AlwaysKeep` mode, so this is a plain on/off switch: + /// enabled rescues every errored chunk. See APMSVLS-469. pub apm_error_sampler_enabled: bool, pub span_dedup_timeout: Option, @@ -168,8 +161,6 @@ pub struct LambdaConfigSource { #[serde(deserialize_with = "deser_opt_bool")] pub lambda_extension_compute_stats: Option, - /// `DD_APM_ERROR_SAMPLER_ENABLED` — toggles the agent-side error sampler. - /// Flat env/YAML key (`apm_error_sampler_enabled`). #[serde(deserialize_with = "deser_opt_bool")] pub apm_error_sampler_enabled: Option, diff --git a/bottlecap/src/traces/trace_processor.rs b/bottlecap/src/traces/trace_processor.rs index c847b1559..e8a59b43b 100644 --- a/bottlecap/src/traces/trace_processor.rs +++ b/bottlecap/src/traces/trace_processor.rs @@ -65,14 +65,10 @@ impl StatsComputedBy { #[allow(clippy::module_name_repetitions)] pub struct ServerlessTraceProcessor { pub obfuscation_config: Arc, - /// Agent-side error sampler. On the `lambda_extension_compute_stats` path, - /// errored chunks that would be dropped (`AutoDrop`) get a second look and - /// may be rescued. In the current `AlwaysKeep` mode every errored chunk is - /// rescued, gated only by `apm_error_sampler_enabled`; a traces/sec budget - /// applies in `RateLimited` mode. Shared across invocations (std Mutex, not - /// tokio: consulted from the synchronous `process_traces`) so - /// `RateLimited`'s rolling-window rate limiter accumulates. `AlwaysKeep` - /// holds no state, so the lock is uncontended bookkeeping there. + /// Rescues errored `AutoDrop` chunks on the `lambda_extension_compute_stats` + /// path. Shared across invocations so `RateLimited`'s rolling-window rate + /// limiter accumulates; a std Mutex rather than tokio because + /// `process_traces` is synchronous. pub error_sampler: Arc>, } @@ -89,22 +85,17 @@ fn span_view(span: &Span) -> SpanView<'_> { } /// Builds the error sampler as the extension ships it, enabled or disabled by -/// `apm_error_sampler_enabled`. Shared by `main` and the tests so tests -/// exercise the shipping configuration. +/// `apm_error_sampler_enabled`. /// -/// The mode is hardcoded for now (config wiring via `DD_APM_ERROR_SAMPLER_MODE` -/// is deferred): Lambda's per-invocation trace volume is low and freeze/thaw -/// breaks the `RateLimited` mode's 30s wall-clock window, so `AlwaysKeep` is -/// the right default. See APMSVLS-469. +/// The mode is hardcoded to `AlwaysKeep`: Lambda's per-invocation trace volume +/// is low, and freeze/thaw breaks `RateLimited`'s 30s wall-clock window. #[must_use] pub fn new_error_sampler(enabled: bool) -> Arc> { Arc::new(std::sync::Mutex::new(ErrorsSampler::new( ErrorSamplerConfig { mode: ErrorSamplerMode::AlwaysKeep, - // AlwaysKeep derives its disabled flag from `target_tps <= 0.0`, so - // the boolean maps onto any positive value vs. zero. The magnitude - // is unused until RateLimited is wired up, which is when - // DD_APM_ERROR_TPS becomes meaningful and gets exposed. + // `is_disabled()` is `target_tps <= 0.0`; the magnitude only + // matters in RateLimited mode. target_tps: if enabled { 1.0 } else { 0.0 }, extra_sample_rate: 1.0, }, @@ -118,9 +109,7 @@ impl ServerlessTraceProcessor { /// `SamplerPriority::AutoDrop` (0) chunks are rescue candidates; negative /// priorities are explicit drops and are honored. fn drop_sampled_out_chunks(&self, tracer_payloads: &mut Vec) { - // Ask the sampler itself whether it is disabled, rather than - // re-deriving that from config: one lock here instead of per chunk, - // and no second definition of "disabled" to keep in sync. + // Hoisted out of the loop: one lock instead of one per chunk. let rescue_enabled = !self .error_sampler .lock() @@ -143,7 +132,6 @@ impl ServerlessTraceProcessor { // tracer reported for this payload, as the Agent does. let env = tp.env.as_str(); tp.chunks.retain_mut(|chunk| { - // Explicit keeps and "no priority set" pass through unchanged. if chunk.priority > 0 || chunk.priority == SamplerPriority::None as i32 { return true; } @@ -152,13 +140,9 @@ impl ServerlessTraceProcessor { if chunk.priority < 0 { return false; } - // A disabled sampler drops every chunk; skip building views - // for a decision that is already known. if !rescue_enabled { return false; } - // AutoDrop (0): give errored chunks a second look via the error - // sampler (rescue within budget). self.rescue_error_chunk(chunk, env, now_secs) }); } @@ -170,7 +154,7 @@ impl ServerlessTraceProcessor { /// traces are candidates; on a keep, stamps `_dd.errors_sr` on the root span. fn rescue_error_chunk(&self, chunk: &mut pb::TraceChunk, env: &str, now_secs: i64) -> bool { let Ok(root_idx) = trace_utils::get_root_span_index(&chunk.spans) else { - return false; // can't identify a root span; drop as before + return false; // no identifiable root span }; // Only errored traces are rescue candidates (matches the Go agent, which // only routes error traces through the ErrorTPS ScoreSampler). An error @@ -179,14 +163,14 @@ impl ServerlessTraceProcessor { return false; } - // Build the borrow-only views in a scope so they (and their immutable - // borrow of chunk.spans) drop before the `_dd.errors_sr` mutation below. + // Scoped so the views release their borrow of chunk.spans before the + // `_dd.errors_sr` mutation below. let decision = { let mut sampler = self.error_sampler.lock().expect("error sampler poisoned"); let root = &chunk.spans[root_idx]; - // AlwaysKeep ignores the spans apart from a bounds check on - // root_index, so only the root view is built there. RateLimited - // needs every span to compute the trace signature. + // AlwaysKeep only bounds-checks root_index, so building the root + // view alone is enough. RateLimited needs every span to compute + // the trace signature. let root_view; let all_views; let (spans, root_index) = if matches!(*sampler, ErrorsSampler::AlwaysKeep { .. }) { @@ -744,9 +728,7 @@ impl SendingTraceProcessor { } } -/// Enabled error sampler for constructing `ServerlessTraceProcessor` in tests -/// (sampler behavior itself is unit-tested in the `datadog-agent-trace-sampler` -/// crate; these call sites just need a value). +/// Enabled error sampler for constructing `ServerlessTraceProcessor` in tests. #[cfg(test)] pub(crate) fn default_error_sampler() -> Arc> { new_error_sampler(true) @@ -1612,9 +1594,8 @@ mod tests { ); } - /// With the error sampler disabled (the shipping default, - /// `apm_error_sampler_enabled: false`), errored P0 chunks are dropped as - /// before: no rescue, no `_dd.errors_sr`. + /// With the error sampler disabled (the shipping default), errored P0 + /// chunks are dropped: no rescue, no `_dd.errors_sr`. #[test] fn test_disabled_error_sampler_drops_errored_p0_chunks() { use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig;