Skip to content

chore(config): migrate trace sampler to typed configuration - #2191

Draft
webern wants to merge 1 commit into
m/confra-dtcfgfrom
m/confra-apmcf
Draft

chore(config): migrate trace sampler to typed configuration#2191
webern wants to merge 1 commit into
m/confra-dtcfgfrom
m/confra-apmcf

Conversation

@webern

@webern webern commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Human Summary

This is a relatively straightforward conversion of TraceSamplerConfiguration to typed config. No issues were encountered. The Codex flag is because we are changing our default to match the Agent (which is what we want).

AI Summary

Migrate the trace sampler transform to the typed traces configuration domain.

  • Construct TraceSamplerConfiguration from domains.traces instead of GenericConfiguration and ApmConfig.
  • Preserve the Saluki-only defaults for the trace sampler in the typed model.
  • Remove sampler-only fields, accessors, and legacy deserialization tests from ApmConfig.
  • Update the traces pipeline construction to use the typed configuration.

Change Type

  • Non-functional (chore, refactoring, docs)

How did you test this PR?

  • make fmt
  • make build-schema-overlay (no generated drift)
  • cargo check --workspace --tests
  • cargo clippy -p saluki-components -p agent-data-plane-config-system -p agent-data-plane -p agent-data-plane-config --all-targets
  • cargo nextest run -p saluki-components -p agent-data-plane-config-system -p agent-data-plane-config (843 passed, 1 skipped)

References

Build the trace sampler from the resolved traces domain slice instead of
reading GenericConfiguration through the shared ApmConfig helper.

- Take the sampler's probabilistic, error, priority, and rare-sampler
  inputs from domains.traces, and drop its raw deserialization.
- Establish the Saluki-only sampler defaults in the traces model, matching
  ADP's historical values: default_env "none", error_sampling_enabled on,
  and rare_sampler tps 5, cooldown 300s, cardinality 200.
- Trim ApmConfig to the span-kind, peer-tags, environment, hostname, and
  obfuscation surface still read by the APM stats and trace obfuscation
  transforms, removing the now-unused sampler fields and their legacy
  deserialization tests.

The witnessed apm_config.probabilistic_sampling.sampling_percentage takes
the Datadog schema default of 0 rather than ADP's former 100. This only
affects the effective rate when the APM probabilistic sampler is enabled
without an explicit percentage; the sampler is off by default.
@webern
webern requested a review from a team as a code owner July 23, 2026 06:00
@dd-octo-sts dd-octo-sts Bot added area/components Sources, transforms, and destinations. transform/trace-sampler Trace Sampler synchronous transform. labels Jul 23, 2026

webern commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@datadog-prod-us1-3 datadog-prod-us1-3 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Datadog Autotest: PASS

More details

The migration preserves trace sampler behavior through the resolved configuration path: generated schema defaults still provide the historical 10 TPS values, while Saluki-only defaults and explicit nested settings are seeded into the traces domain. No diff-only behavioral regression was identified. No additional tests recommended: the existing configuration transport coverage already exercises the migrated Saluki-only fields, and execution was blocked by unavailable Rust dependencies.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Datadog Autotest · Commit fa6af8d · What is Autotest? · Any feedback? Reach out in #autotest

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa6af8d373

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let otlp_sampling_rate = normalize_sampling_rate(traces.otlp.probabilistic_sampler_sampling_percentage / 100.0);
Self {
probabilistic_sampler_enabled: traces.probabilistic_sampler.enabled,
sampling_percentage: traces.probabilistic_sampler.sampling_percentage,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore the omitted probabilistic sampler percentage fallback

When apm_config.probabilistic_sampler.enabled is set but sampling_percentage is omitted, this now copies the typed Datadog default of 0_f64 from lib/datadog-agent/config/src/generated/datadog_configuration.rs:1769; the removed raw ApmConfig default was 100%, so line 111 builds a sampling rate of 0.0 and the probabilistic path drops all non-error/non-rare traces. Preserve the previous 100% fallback for the omitted-percentage case so this typed-config cutover remains behaviorally transparent.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is correct and a bugfix that goes toward #1802. ADP should honor the core Agent defaults. Fortunately this feature isn't released to customers yet so only our tests are affected.

@pr-commenter

pr-commenter Bot commented Jul 23, 2026

Copy link
Copy Markdown

Binary Size Analysis (Agent Data Plane)

Baseline: 0f99ce3 · Comparison: fa6af8d · diff
Analysis Configuration: stripped binaries · Pass/Fail Threshold: +5%
Sizes: 40.79 MiB (baseline) vs 41.03 MiB (comparison)
Size Change: +246.12 KiB (+0.59%)

✅ Binary size difference within threshold

Changes by Module
Module File Size Symbols
figment +194.17 KiB 277
saluki_components::common::datadog -79.81 KiB 109
tonic_prost +67.70 KiB 23
agent_data_plane_config_system::saluki_env_overlay::PathRecorder -67.26 KiB 24
otlp_protos::otlp_include::opentelemetry -49.26 KiB 85
hyper +43.88 KiB 84
agent_data_plane_config_system::saluki_only::_ +40.65 KiB 9
core +38.01 KiB 2727
tokio +37.97 KiB 929
prost -31.72 KiB 123
h2 -24.09 KiB 138
serde +24.08 KiB 47
serde_with +20.49 KiB 15
saluki_components::transforms::dogstatsd_mapper -17.96 KiB 8
anon.2106486f8d317d0f26c228a10a0c25ea.531.llvm.15798623470946910919 +17.01 KiB 1
anon.4f47395eaac7e6a86c9111625e37e6d6.395.llvm.1637700629175368008 -16.91 KiB 1
hyper_util -16.42 KiB 10
hashbrown +14.73 KiB 97
piecemeal -14.69 KiB 20
saluki_components::encoders::datadog -14.44 KiB 100
Detailed Symbol Changes
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +2.3%  +266Ki  +2.2%  +200Ki    [13972 Others]
  [NEW] +44.2Ki  [NEW] +43.9Ki    _<agent_data_plane_config_system::saluki_only::_::<impl serde_core::de::Deserialize for agent_data_plane_config_system::saluki_only::SalukiOnly>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::h029b6a2668b5ce6e
  [NEW] +18.9Ki  [NEW] +18.7Ki    _<tonic_prost::codec::ProstEncoder<T> as tonic::codec::Encoder>::encode::hf2f024643e98714e
  [NEW] +17.0Ki  [NEW]     +76    anon.2106486f8d317d0f26c228a10a0c25ea.531.llvm.15798623470946910919
  [NEW] +12.7Ki  [NEW] +12.6Ki    _<figment::value::de::ConfiguredValueDe<I> as serde_core::de::Deserializer>::deserialize_any::h5b0bc0cf25ffe876
  [NEW] +12.4Ki  [NEW]     +81    anon.2106486f8d317d0f26c228a10a0c25ea.567.llvm.15798623470946910919
  [NEW] +11.6Ki  [NEW] +11.5Ki    _<agent_data_plane_config_system::saluki_env_overlay::StructWalker as serde_core::de::MapAccess>::next_value_seed::hc68cc5819be30644
 +17e2% +11.3Ki +20e2% +11.3Ki    prost::message::Message::encode_to_vec::hec9d535755282a1d
 +48e2% +11.0Ki +81e2% +11.0Ki    std::sys::backtrace::__rust_begin_short_backtrace::h636265569ecef8fc
  [NEW] +10.8Ki  [NEW] +10.7Ki    figment::figment::Figment::extract::haee1684d6196b8e4
  [DEL] -10.9Ki  [DEL] -10.7Ki    saluki_components::sources::otlp::logs::transform::transform_log_record::h3302f377f7bd27f4
  [DEL] -11.5Ki  [DEL] -11.4Ki    h2::server::Connection<T,B>::poll_closed::h92b70f56726577b1
  [DEL] -11.6Ki  [DEL] -11.6Ki    h2::server::Connection<T,B>::poll_closed::hc9c179a40ea7c55d
  [DEL] -12.1Ki  [DEL] -12.0Ki    saluki_components::sources::dogstatsd::replay::writer::run_capture_loop::he083f64fbd1fc69a
  [DEL] -12.2Ki  [DEL]     -81    anon.4f47395eaac7e6a86c9111625e37e6d6.672.llvm.1637700629175368008
  [DEL] -12.4Ki  [DEL] -12.3Ki    h2::server::Connection<T,B>::poll_closed::h29dd8a9319b0a688
  [DEL] -14.2Ki  [DEL] -14.0Ki    saluki_components::common::datadog::apm::_::_<impl serde_core::de::Deserialize for saluki_components::common::datadog::apm::ApmConfiguration>::deserialize::h8279d5536d30f1e2
  [DEL] -15.6Ki  [DEL] -15.3Ki    saluki_components::common::datadog::config::_::_<impl serde_core::de::Deserialize for saluki_components::common::datadog::config::ForwarderConfiguration>::deserialize::h8491640142ecc530
  [DEL] -16.8Ki  [DEL] -16.6Ki    _<agent_data_plane_config_system::saluki_env_overlay::PathRecorder as serde_core::de::Deserializer>::deserialize_struct::h3faf0c90a7e1b643
  [DEL] -16.9Ki  [DEL]     -76    anon.4f47395eaac7e6a86c9111625e37e6d6.395.llvm.1637700629175368008
  [DEL] -36.2Ki  [DEL] -35.9Ki    _<saluki_components::common::datadog::obfuscation::_::<impl serde_core::de::Deserialize for saluki_components::common::datadog::obfuscation::ObfuscationConfig>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::h04547875805b5e90
  +0.6%  +246Ki  +0.5%  +179Ki    TOTAL

@pr-commenter

pr-commenter Bot commented Jul 23, 2026

Copy link
Copy Markdown

Regression Detector (Agent Data Plane)

Run ID: 0622e0ce-2477-4124-abd8-caa09b1c3ca3
Baseline: 0f99ce37 · Comparison: fa6af8d3 · diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment (3)

Experiments configured erratic: true are tagged (ignored) and skipped when determining which experiments regressed or improved. Experiments which are detected as erratic at runtime are tagged (erratic) to flag that the run's sample dispersion was high, but their regression / improvement signal still counts.

experiment goal Δ mean % links
quality_gates_rss_idle memory ⚪ +0.30 metrics profiles logs
quality_gates_rss_dsd_medium memory ⚪ +0.24 metrics profiles logs
quality_gates_rss_dsd_low memory ⚪ -0.24 metrics profiles logs
Bounds Checks: ✅ Passed (3)
experiment check replicates observed links
quality_gates_rss_dsd_low memory_usage 10/10 ✅ 45.2 MiB ≤ 50 MiB metrics profiles logs
quality_gates_rss_dsd_medium memory_usage 10/10 ✅ 67 MiB ≤ 75 MiB metrics profiles logs
quality_gates_rss_idle memory_usage 10/10 ✅ 31.2 MiB ≤ 40 MiB metrics profiles logs
Explanation

A change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression (is_regression: true). Improvements use the matching criteria for the improving direction. Experiments configured erratic: true (tagged (ignored)) are skipped outright; experiments detected as erratic at runtime (tagged (erratic)) still count, since that flag describes sample dispersion rather than directional certainty. The Δ mean % cell is colored accordingly: 🟢 = improvement, 🔴 = regression, ⚪ = neutral. Reduction in CPU or memory is an improvement; reduction in ingress throughput is a regression.

@webern
webern marked this pull request as draft July 24, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/components Sources, transforms, and destinations. transform/trace-sampler Trace Sampler synchronous transform.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant