Skip to content

Commit 166c295

Browse files
authored
chore(config): migrate TraceSamplerConfiguration to typed config (#1984)
## AI Summary Build `TraceSamplerConfiguration` from the resolved traces configuration instead of the raw configuration map. Move trace sampler defaults into the typed configuration layers and remove the sampler-specific source parsing from the shared APM helper. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` ## References - Progresses #1788
1 parent 9041f1e commit 166c295

5 files changed

Lines changed: 221 additions & 256 deletions

File tree

bin/agent-data-plane/src/cli/run.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,9 +650,9 @@ async fn add_baseline_traces_pipeline_to_blueprint(
650650
.with_environment_provider(env_provider.clone())
651651
.await?;
652652
let trace_obfuscation_config = TraceObfuscationConfiguration::from_apm_configuration(config)?;
653-
let trace_sampler_config = TraceSamplerConfiguration::from_configuration(config)
654-
.error_context("Failed to configure Trace Sampler transform.")?;
655653
let saluki = config_system.config();
654+
let trace_sampler_config = TraceSamplerConfiguration::from_configuration(&saluki.domains.traces)
655+
.error_context("Failed to configure Trace Sampler transform.")?;
656656
let ottl_filter_config = OttlFilterConfiguration::from_configuration(&saluki.domains.traces.ottl_filter)
657657
.error_context("Failed to configure OTTL filter processor.")?;
658658
let ottl_transform_config = OttlTransformConfiguration::from_configuration(config)

lib/agent-data-plane-config-system/src/saluki_only.rs

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@
5252
//! type coerces the source form (durations are [`DurationString`], not `Duration`), and that
5353
//! `seed` copies it to the model destination the component reads.
5454
//! 2. Value arrives wrong only when the key is absent: the default bug is not here. The default
55-
//! lives in the model `Default` in `agent-data-plane-config`, because these fields are
56-
//! `Option<T>` and `seed` writes only when set. Fix the model `Default`, not this struct.
55+
//! lives in the shared model default in `agent-data-plane-config`, and required values are
56+
//! copied by `seed` even when they were absent from the source. Fix the shared default, not this
57+
//! struct.
5758
//! 3. Then add or extend the round-trip test below (set the real key, assert the model field). A
5859
//! missing test is why a silent transport failure was not caught.
5960
//!
@@ -75,7 +76,10 @@
7576
use std::time::Duration;
7677

7778
use agent_data_plane_config::control::ListenAddress;
78-
use agent_data_plane_config::domains::traces::{OttlErrorMode, OttlFilter, OttlTransform};
79+
use agent_data_plane_config::domains::traces::{
80+
default_error_sampling_enabled, default_rare_sampler_cardinality, default_rare_sampler_cooldown,
81+
default_rare_sampler_tps, default_trace_environment, OttlErrorMode, OttlFilter, OttlTransform,
82+
};
7983
use agent_data_plane_config::SalukiConfiguration;
8084
use bytesize::ByteSize;
8185
use saluki_config::DurationString;
@@ -85,8 +89,9 @@ use serde::Deserialize;
8589
///
8690
/// Flat keys are top-level fields; nested keys live on matching nested sub-structs. Every field is
8791
/// `#[serde(default)]` and `Option`-typed (or an empty collection), so a deployment may set no
88-
/// Saluki-only keys at all and each falls back to its model default. Deserialized from the same
89-
/// merged map as the Datadog source model, so unknown (Datadog) keys are ignored.
92+
/// Saluki-only keys at all and each falls back to its model default. Required values with
93+
/// component defaults use the shared default from `agent-data-plane-config`. Deserialized from the
94+
/// same merged map as the Datadog source model, so unknown (Datadog) keys are ignored.
9095
#[derive(Clone, Debug, Default, Deserialize)]
9196
#[serde(default)]
9297
pub struct SalukiOnly {
@@ -213,29 +218,55 @@ pub struct DataPlaneMetricsV3Series {
213218

214219
/// `apm_config.*` Saluki-only knobs. (The Datadog Agent publishes many other `apm_config.*` keys;
215220
/// those are witnessed and ignored here.)
216-
#[derive(Clone, Debug, Default, Deserialize)]
221+
#[derive(Clone, Debug, Deserialize)]
217222
#[serde(default)]
218223
pub struct ApmConfig {
219224
/// Default trace environment (`apm_config.default_env`).
220-
pub default_env: Option<String>,
225+
#[serde(default = "default_trace_environment")]
226+
pub default_env: String,
221227
/// Whether error sampling is enabled (`apm_config.error_sampling_enabled`).
222-
pub error_sampling_enabled: Option<bool>,
228+
#[serde(default = "default_error_sampling_enabled")]
229+
pub error_sampling_enabled: bool,
223230
/// Rare sampler tuning (`apm_config.rare_sampler.*`).
224231
pub rare_sampler: ApmRareSampler,
225232
/// SQL obfuscation knobs (`apm_config.obfuscation.*`).
226233
pub obfuscation: ApmObfuscation,
227234
}
228235

236+
impl Default for ApmConfig {
237+
fn default() -> Self {
238+
Self {
239+
default_env: default_trace_environment(),
240+
error_sampling_enabled: default_error_sampling_enabled(),
241+
rare_sampler: ApmRareSampler::default(),
242+
obfuscation: ApmObfuscation::default(),
243+
}
244+
}
245+
}
246+
229247
/// `apm_config.rare_sampler.*`.
230-
#[derive(Clone, Debug, Default, Deserialize)]
248+
#[derive(Clone, Debug, Deserialize)]
231249
#[serde(default)]
232250
pub struct ApmRareSampler {
233251
/// Tracked-signature cardinality (`apm_config.rare_sampler.cardinality`).
234-
pub cardinality: Option<usize>,
252+
#[serde(default = "default_rare_sampler_cardinality")]
253+
pub cardinality: usize,
235254
/// Cooldown between rare-sample emissions (`apm_config.rare_sampler.cooldown`).
236-
pub cooldown: Option<f64>,
255+
#[serde(default = "default_rare_sampler_cooldown")]
256+
pub cooldown: f64,
237257
/// Rare-sample traces-per-second budget (`apm_config.rare_sampler.tps`).
238-
pub tps: Option<f64>,
258+
#[serde(default = "default_rare_sampler_tps")]
259+
pub tps: f64,
260+
}
261+
262+
impl Default for ApmRareSampler {
263+
fn default() -> Self {
264+
Self {
265+
cardinality: default_rare_sampler_cardinality(),
266+
cooldown: default_rare_sampler_cooldown(),
267+
tps: default_rare_sampler_tps(),
268+
}
269+
}
239270
}
240271

241272
/// `apm_config.obfuscation.*`.
@@ -458,21 +489,11 @@ impl SalukiOnly {
458489

459490
// domains.traces
460491
let traces = &mut config.domains.traces;
461-
if let Some(v) = self.apm_config.default_env.clone() {
462-
traces.default_env = v;
463-
}
464-
if let Some(v) = self.apm_config.error_sampling_enabled {
465-
traces.error_sampling_enabled = v;
466-
}
467-
if let Some(v) = self.apm_config.rare_sampler.cardinality {
468-
traces.rare_sampler.cardinality = v;
469-
}
470-
if let Some(v) = self.apm_config.rare_sampler.cooldown {
471-
traces.rare_sampler.cooldown = v;
472-
}
473-
if let Some(v) = self.apm_config.rare_sampler.tps {
474-
traces.rare_sampler.tps = v;
475-
}
492+
traces.default_env = self.apm_config.default_env.clone();
493+
traces.error_sampling_enabled = self.apm_config.error_sampling_enabled;
494+
traces.rare_sampler.cardinality = self.apm_config.rare_sampler.cardinality;
495+
traces.rare_sampler.cooldown = self.apm_config.rare_sampler.cooldown;
496+
traces.rare_sampler.tps = self.apm_config.rare_sampler.tps;
476497
if let Some(v) = self.apm_config.obfuscation.sql.dbms.clone() {
477498
traces.obfuscation.sql.dbms = v;
478499
}
@@ -700,9 +721,9 @@ mod tests {
700721
}
701722
}
702723

703-
/// An absent key leaves the model default in place (the common case). `seed` writes only present
704-
/// options, so this exercises the `Option`-in-source / default-in-model split. A wrong value
705-
/// here means the model `Default` is wrong, not this struct.
724+
/// An absent key leaves the model default in place (the common case). Required Saluki-only
725+
/// values are deserialized from their shared defaults and seeded into the model. A wrong value
726+
/// here means the shared default is wrong, not this struct.
706727
#[test]
707728
fn absent_keys_leave_model_defaults() {
708729
let saluki_only: SalukiOnly = serde_json::from_value(json!({})).expect("empty source deserializes");
@@ -714,5 +735,12 @@ mod tests {
714735
assert_eq!(agg.context_limit, 1_000_000);
715736
assert_eq!(agg.flush_interval, Duration::from_secs(15));
716737
assert_eq!(agg.passthrough_idle_flush_timeout, Duration::from_secs(1));
738+
739+
let traces = &config.domains.traces;
740+
assert_eq!(traces.default_env, default_trace_environment());
741+
assert_eq!(traces.error_sampling_enabled, default_error_sampling_enabled());
742+
assert_eq!(traces.rare_sampler.cardinality, default_rare_sampler_cardinality());
743+
assert_eq!(traces.rare_sampler.cooldown, default_rare_sampler_cooldown());
744+
assert_eq!(traces.rare_sampler.tps, default_rare_sampler_tps());
717745
}
718746
}

lib/agent-data-plane-config/src/domains/traces.rs

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,33 @@
22
33
use serde::{Deserialize, Serialize};
44

5+
/// The default environment applied when a trace has no explicit environment.
6+
pub fn default_trace_environment() -> String {
7+
"none".to_owned()
8+
}
9+
10+
/// Whether error spans are sampled independently of the base sampler by default.
11+
pub const fn default_error_sampling_enabled() -> bool {
12+
true
13+
}
14+
15+
/// The default target for rare-span traces per second.
16+
pub const fn default_rare_sampler_tps() -> f64 {
17+
5.0
18+
}
19+
20+
/// The default rare-sampler cooldown, in seconds.
21+
pub const fn default_rare_sampler_cooldown() -> f64 {
22+
300.0
23+
}
24+
25+
/// The default rare-sampler signature cardinality.
26+
pub const fn default_rare_sampler_cardinality() -> usize {
27+
200
28+
}
29+
530
/// Resolved traces configuration.
6-
#[derive(Clone, Debug, Default, PartialEq, Serialize)]
31+
#[derive(Clone, Debug, PartialEq, Serialize)]
732
pub struct Domain {
833
/// Environment tag applied to traces.
934
pub env: String,
@@ -56,8 +81,31 @@ pub struct Domain {
5681
pub ottl_transform: OttlTransform,
5782
}
5883

84+
impl Default for Domain {
85+
fn default() -> Self {
86+
Self {
87+
env: String::default(),
88+
default_env: default_trace_environment(),
89+
compute_stats_by_span_kind: false,
90+
peer_tags: Vec::default(),
91+
peer_tags_aggregation: false,
92+
error_sampling_enabled: default_error_sampling_enabled(),
93+
error_tracking_standalone_enabled: false,
94+
errors_per_second: 0.0,
95+
target_traces_per_second: 0.0,
96+
enable_rare_sampler: false,
97+
rare_sampler: RareSampler::default(),
98+
probabilistic_sampler: ProbabilisticSampler::default(),
99+
obfuscation: Obfuscation::default(),
100+
otlp: OtlpTraces::default(),
101+
ottl_filter: OttlFilter::default(),
102+
ottl_transform: OttlTransform::default(),
103+
}
104+
}
105+
}
106+
59107
/// Rare-span sampler.
60-
#[derive(Clone, Debug, Default, PartialEq, Serialize)]
108+
#[derive(Clone, Debug, PartialEq, Serialize)]
61109
pub struct RareSampler {
62110
/// Maximum number of distinct span signatures tracked. (not in Datadog Agent config schema)
63111
pub cardinality: usize,
@@ -70,6 +118,16 @@ pub struct RareSampler {
70118
pub tps: f64,
71119
}
72120

121+
impl Default for RareSampler {
122+
fn default() -> Self {
123+
Self {
124+
cardinality: default_rare_sampler_cardinality(),
125+
cooldown: default_rare_sampler_cooldown(),
126+
tps: default_rare_sampler_tps(),
127+
}
128+
}
129+
}
130+
73131
/// APM probabilistic sampler.
74132
#[derive(Clone, Debug, Default, PartialEq, Serialize)]
75133
pub struct ProbabilisticSampler {

0 commit comments

Comments
 (0)