Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5ba576d
Rescue errored P0 trace chunks from backend drop
lucaspimentel Sep 18, 2026
5d5b911
Exercise error rescue in stats regression tests
lucaspimentel Sep 18, 2026
ab8ab90
Read sampler clock while holding the sampler lock
lucaspimentel Sep 18, 2026
156a99f
Clamp sampler timestamps against backward clock steps
lucaspimentel Sep 24, 2026
42cfbf1
Match test oracle env with processor fallback env
lucaspimentel Sep 24, 2026
f99a229
Match clone-budget oracle env with processor fallback env
lucaspimentel Sep 24, 2026
abb5314
Document root-resolution fallback in sampler doc comment
lucaspimentel Sep 24, 2026
b895a83
Share the payload-env fallback rule between stats and error rescue
lucaspimentel Sep 24, 2026
6bf49b7
Fix error-rescue clock read to happen under the sampler lock
lucaspimentel Sep 24, 2026
a6e2afe
Simplify error rescue tests
lucaspimentel Sep 30, 2026
0e91eaa
Add opt-in rescue for tracer-dropped errors
lucaspimentel Oct 19, 2026
23fc576
Simplify error rescue test assertions
lucaspimentel Oct 19, 2026
e95e578
Remove SCL error sampler mode setting
lucaspimentel Oct 27, 2026
efad1e1
Share error sampler floor under one mutex
lucaspimentel Oct 27, 2026
5006f9a
Route error rescue through shared clock helper
lucaspimentel Oct 27, 2026
c88d4a9
Rescue errored no-priority chunks
lucaspimentel Oct 27, 2026
f4fe8da
Decouple error rescue from stats computation
lucaspimentel Oct 28, 2026
dea8b63
Consolidate error rescue eligibility tests
lucaspimentel Oct 28, 2026
8d998cf
Merge empty-chunk rescue tests
lucaspimentel Oct 28, 2026
b874bf4
Consolidate error sampler adapter tests
lucaspimentel Oct 28, 2026
3fa988d
Simplify shared error sampler budget test
lucaspimentel Oct 28, 2026
391cbfe
Simplify error sampler config tests
lucaspimentel Oct 28, 2026
4f436a7
Simplify error rescue processing and tests
lucaspimentel Oct 28, 2026
0c9cb42
Skip error sampler lock without rescue candidates
lucaspimentel Oct 28, 2026
c8d9f76
Focus error rescue tests on distinct guarantees
lucaspimentel Oct 28, 2026
02671a8
Simplify error rescue with lazy locking
lucaspimentel Oct 28, 2026
c6eb0f9
Consolidate error rescue clock regression tests
lucaspimentel Oct 28, 2026
e4b566f
Consolidate disabled error rescue tests
lucaspimentel Oct 28, 2026
2e184e3
Remove tests for unused error sampler setting
lucaspimentel Oct 28, 2026
acf430c
Focus error rescue config tests on wiring
lucaspimentel Oct 28, 2026
a27b25c
Focus decision maker tests on precedence
lucaspimentel Oct 28, 2026
cef1032
Simplify disabled error rescue warning test
lucaspimentel Oct 28, 2026
4d46981
Remove redundant rescue root bounds checks
lucaspimentel Oct 28, 2026
6c2277c
Clarify shared error rescue state construction
lucaspimentel Oct 28, 2026
74b4dff
Consolidate error rescue eligibility docs
lucaspimentel Oct 28, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions crates/datadog-agent-trace-sampler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ rescue strategies are available, selected by `ErrorSamplerMode`:
`ScoreSampler` (`ScoreSampler` targeting `ErrorTPS`, from `pkg/trace/sampler/`
in `DataDog/datadog-agent`): keep up to `target_tps` error traces per second,
distributed fairly across distinct trace signatures. Suits continuous
processes that can hit error storms (the Serverless Compatibility Layer's
default).
processes that can hit error storms (the Serverless Compatibility Layer uses
this strategy).

Per-platform defaults are chosen by each consumer's config layer (e.g. via a
`DD_APM_ERROR_SAMPLER_MODE` env var), not this crate — it has no notion of
which platform it runs on.
Each consumer chooses its strategy through its configuration or integration
layer, not this crate, which has no notion of which platform it runs on.

## Why dependency-free

Expand Down
1 change: 1 addition & 0 deletions crates/datadog-serverless-compat/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ pub async fn main() {

let trace_processor = Arc::new(trace_processor::ServerlessTraceProcessor::new(
stats_concentrator.as_ref().map(|c| c.handle.clone()),
trace_processor::ServerlessTraceProcessor::new_error_rescue_state(&config),
));

let stats_flusher = Arc::new(stats_flusher::ServerlessStatsFlusher {
Expand Down
2 changes: 2 additions & 0 deletions crates/datadog-trace-agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ libdd-trace-stats = { version = "7.0.0", features = ["stats-obfuscation"] }
libdd-common = { workspace = true, features = ["https"] }
libdd-trace-obfuscation = { workspace = true, features = ["https"] }
libdd-trace-utils = { workspace = true, features = ["https", "mini_agent"] }
datadog-agent-trace-sampler = { path = "../datadog-agent-trace-sampler" }
datadog-fips = { path = "../datadog-fips" }
reqwest = { version = "0.12.23", features = [
"json",
Expand All @@ -43,6 +44,7 @@ prost = "0.14.1"

[dev-dependencies]
datadog-mock-intake = { path = "../datadog-mock-intake" }
tracing-test = { version = "0.2.5", default-features = false }
rmp-serde = "1.1.1"
serial_test = "2.0.0"
duplicate = "2.0.1"
Expand Down
155 changes: 155 additions & 0 deletions crates/datadog-trace-agent/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,57 @@ use std::env;
use std::str::FromStr;
use std::sync::OnceLock;

use datadog_agent_trace_sampler::{ErrorSamplerConfig, ErrorSamplerMode};
use libdd_trace_obfuscation::obfuscation_config;
use libdd_trace_utils::config_utils::{
read_cloud_env, trace_intake_url, trace_intake_url_prefixed, trace_stats_url,
trace_stats_url_prefixed,
};
use libdd_trace_utils::trace_utils;
use tracing::warn;

const DEFAULT_APM_RECEIVER_PORT: u16 = 8126;
/// Default error rescue budget: error traces per second, matching the Go agent's `ErrorTPS`.
const DEFAULT_ERROR_SAMPLER_TPS: f64 = 10.0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

Rescue is on by default for all users

When a user upgrades, rescue starts at 10 error traces per second. The user does not need to change a setting. The Lambda extension makes rescue opt-in. Its config comment gives the reason: the extension uses AlwaysKeep, so it has no limit on volume. This PR has a limit, so on by default can be the right choice. But it changes how many traces each user ingests.

The budget is per process. So the total increases with the number of function instances.

Can the PR description state this change in behavior, and say who approved the default? It can also tell users how to turn rescue off (DD_APM_ERROR_TPS=0).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping rescue enabled by default with a per-process target of 10 TPS, matching the Go agent's ErrorTPS default. DD_APM_ERROR_TPS=0 disables it. The PR description already documents both settings; 10 TPS is an adaptive target, not a hard cap.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

Thank you. Your point about the adaptive target is correct, and I agree with the Go agent default. My only request: add one sentence to the PR description that says this turns on error rescue for current users when they upgrade. The release notes can then use it.


/// Parses the rate-limited error rescue budget from an optional raw TPS value.
///
/// `DD_APM_ERROR_TPS` sets the target error traces per second, default 10. Parsed
/// as `f64`; non-finite or unparsable values warn and fall back to the default.
/// A finite value <= 0 disables rescue.
///
/// The strategy is fixed to `RateLimited`, and `extra_sample_rate` is fixed at
/// 1.0. Startup stays operational when the optional TPS setting is malformed.
fn parse_error_sampler_config(raw: Option<&str>) -> ErrorSamplerConfig {
let mut config = ErrorSamplerConfig {
mode: ErrorSamplerMode::RateLimited,
Comment thread
lucaspimentel marked this conversation as resolved.
target_tps: DEFAULT_ERROR_SAMPLER_TPS,
extra_sample_rate: 1.0,
};

if let Some(raw) = raw {
match raw.trim().parse::<f64>() {
Ok(tps) if tps.is_finite() => config.target_tps = tps,
_ => {
warn!(
"Invalid DD_APM_ERROR_TPS {raw:?}; using default {DEFAULT_ERROR_SAMPLER_TPS}"
);
}
}
}

config
}

// The trace agent already constructs its own configuration. Adopting
// datadog-agent-config solely for this flag would introduce broader loader coupling.
fn error_rare_sample_tracer_drop_enabled(raw: &str) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

Minor: this parser copies the shared config crate

The shared config crate in this repo has the same split in split_comma_then_space, with its own test. This PR copies the 6-line function and adds a 28-line grammar test. I understand that you do not want this crate to use the shared config loader. A small public function in the shared crate can avoid that. It still adds a dependency, so this is your decision.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the small local parser for now. It predates #186. Consolidating the two config types would let this code use the shared apm_features field directly, without introducing a public helper and temporary dependency wiring.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

OK, merging the two config types later is a better solution than a public helper.

let delimiter = if raw.contains(',') { ',' } else { ' ' };
raw.split(delimiter)
.map(str::trim)
.any(|entry| entry == "error_rare_sample_tracer_drop")
}

const DEFAULT_DOGSTATSD_PORT: u16 = 8125;
const DSM_PIPELINE_STATS_ROUTE: &str = "/api/v0.1/pipeline_stats";

Expand Down Expand Up @@ -128,6 +171,12 @@ pub struct Config {
pub additional_metric_tags_cardinality_limit: Option<usize>,
/// Whether the agent should compute trace stats
pub agent_stats_computation_enabled: bool,
/// Rate-limited error rescue settings, with the budget parsed from
/// `DD_APM_ERROR_TPS`. `extra_sample_rate` is fixed at 1.0 and not
/// configurable. See `parse_error_sampler_config` for the defaults.
pub error_sampler: ErrorSamplerConfig,
/// Opt-in eligibility for tracer-dropped errors, independent of sampler enablement.
pub error_rare_sample_tracer_drop: bool,
}

impl Config {
Expand Down Expand Up @@ -304,6 +353,9 @@ impl Config {
agent_stats_computation_enabled: env::var("DD_AGENT_STATS_COMPUTATION_ENABLED")
.map(|val| val.to_lowercase() == "true")
.unwrap_or(true),
error_sampler: parse_error_sampler_config(env::var("DD_APM_ERROR_TPS").ok().as_deref()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

The settings still differ from the Lambda extension

Thank you for removing the mode setting. Two other differences from the Lambda extension remain:

Lambda extension This PR
Turn rescue on or off DD_SERVERLESS_ERROR_SAMPLER_ENABLED, off by default DD_APM_ERROR_TPS <= 0 turns it off. It is on by default.
DD_APM_ERROR_TPS Not read. The extension logs a warning when a user sets it. The budget, default 10

Do you plan to align the two products, in this PR or in a follow-up? If the differences are intentional, can you put the reason in the PR description?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the current SCL settings in this PR. SCL uses the Go-agent-style RateLimited sampler, with DD_APM_ERROR_TPS controlling its target and zero disabling rescue. No configuration alignment with the Lambda extension is included here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Written by Claude, an AI assistant, for Yiming]

OK, the Go agent setting is a good reason. We can align the Lambda extension separately.

error_rare_sample_tracer_drop: env::var("DD_APM_FEATURES")
.is_ok_and(|raw| error_rare_sample_tracer_drop_enabled(&raw)),
})
}
}
Expand All @@ -315,6 +367,7 @@ mod tests {
use std::collections::HashMap;

use crate::config;
use datadog_agent_trace_sampler::ErrorSamplerMode;

#[test]
#[serial]
Expand Down Expand Up @@ -888,6 +941,106 @@ mod tests {
},
);
}

#[test]
fn test_apm_features_grammar() {
let feature = "error_rare_sample_tracer_drop";
for (raw, expected) in [
("".to_string(), false),
(" ".to_string(), false),
("other".to_string(), false),
(feature.to_string(), true),
(format!(" {feature} "), true),
(format!("other {feature}"), true),
(format!("other,{feature}"), true),
(format!(" other ,, {feature} "), true),
(format!("other,{feature} extra"), false),
(format!("other {feature},extra"), false),
(format!("other\t{feature}"), false),
(format!("\t{feature}\t"), true),
(feature.to_uppercase(), false),
(format!("prefix_{feature}"), false),
(format!("{feature}_suffix"), false),
] {
assert_eq!(
super::error_rare_sample_tracer_drop_enabled(&raw),
expected,
"{raw:?}"
);
}
}

#[cfg(unix)]
#[test]
#[serial]
fn test_unreadable_apm_features_is_disabled() {
use std::os::unix::ffi::OsStringExt;
temp_env::with_var(
"DD_APM_FEATURES",
Some(std::ffi::OsString::from_vec(vec![0xff])),
|| {
temp_env::with_vars(
[
("DD_API_KEY", Some("test")),
("FUNCTIONS_EXTENSION_VERSION", Some("~4")),
("FUNCTIONS_WORKER_RUNTIME", Some("dotnet")),
("WEBSITE_SITE_NAME", Some("test")),
("DD_APM_ERROR_TPS", None),
],
|| {
assert!(!config::Config::new().unwrap().error_rare_sample_tracer_drop);
},
);
},
);
}

#[test]
#[serial]
fn test_error_sampler_env_is_wired() {
temp_env::with_vars(
[
("DD_API_KEY", Some("_not_a_real_key_")),
("FUNCTIONS_EXTENSION_VERSION", Some("~4")),
("FUNCTIONS_WORKER_RUNTIME", Some("dotnet")),
("WEBSITE_SITE_NAME", Some("my-azure-function")),
("DD_APM_FEATURES", Some("error_rare_sample_tracer_drop")),
("DD_APM_ERROR_TPS", Some("5.5")),
],
|| {
let config = config::Config::new().unwrap();
assert!(config.error_rare_sample_tracer_drop);
assert_eq!(config.error_sampler.target_tps, 5.5);
assert_eq!(config.error_sampler.mode, ErrorSamplerMode::RateLimited);
assert_eq!(config.error_sampler.extra_sample_rate, 1.0);
},
);
}

#[test]
fn test_error_sampler_tps_parsing() {
for (raw, expected_tps) in [
(None, 10.0),
(Some("5.5"), 5.5),
(Some("0"), 0.0),
(Some("-1"), -1.0),
(Some(" 10 "), 10.0),
(Some("not_a_number"), 10.0),
(Some("inf"), 10.0),
(Some("-inf"), 10.0),
(Some("NaN"), 10.0),
(Some("1e400"), 10.0),
(Some(""), 10.0),
] {
let error_sampler = super::parse_error_sampler_config(raw);
assert_eq!(
error_sampler.target_tps, expected_tps,
"DD_APM_ERROR_TPS {raw:?} should parse to {expected_tps}"
);
assert_eq!(error_sampler.mode, ErrorSamplerMode::RateLimited);
assert_eq!(error_sampler.extra_sample_rate, 1.0);
}
}
}

/// Test helpers for creating Config instances in tests
Expand Down Expand Up @@ -930,6 +1083,8 @@ pub mod test_helpers {
additional_metric_tags: vec![],
additional_metric_tags_cardinality_limit: None,
agent_stats_computation_enabled: true,
error_sampler: ErrorSamplerConfig::default(),
error_rare_sample_tracer_drop: false,
}
}
}
11 changes: 6 additions & 5 deletions crates/datadog-trace-agent/src/stats_concentrator_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::sync::Arc;
use tokio::sync::{mpsc, oneshot};

use crate::config::Config;
use crate::trace_processor::resolve_payload_env;
use libdd_library_config::tracer_metadata::TracerMetadata;
use libdd_trace_protobuf::pb::{ClientStatsPayload, TraceChunk};
use libdd_trace_stats::span_concentrator::{CardinalityLimitConfig, SpanConcentrator};
Expand Down Expand Up @@ -191,11 +192,11 @@ impl StatsConcentratorService {
// Do not set hostname so the trace stats backend can aggregate stats properly
hostname: String::new(),
// Prefer env from the tracer payload, fall back to agent config
env: metadata
.service_env
.clone()
.filter(|s| !s.is_empty())
.unwrap_or_else(|| self.config.env.clone()),
env: resolve_payload_env(
metadata.service_env.as_deref().unwrap_or(""),
&self.config.env,
)
.to_string(),
version: metadata.service_version.clone().unwrap_or_default(),
lang: metadata.tracer_language.clone(),
tracer_version: metadata.tracer_version.clone(),
Expand Down
2 changes: 2 additions & 0 deletions crates/datadog-trace-agent/src/stats_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ mod tests {
additional_metric_tags: vec![],
additional_metric_tags_cardinality_limit: None,
agent_stats_computation_enabled,
error_sampler: datadog_agent_trace_sampler::ErrorSamplerConfig::default(),
error_rare_sample_tracer_drop: false,
}
}

Expand Down
Loading
Loading