fix(config): migrate endpoints to typed config for provenance awareness - #2317
Conversation
|
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.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Binary Size Analysis (Agent Data Plane)Baseline: ca31d5f · Comparison: fd640ba · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA 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 ( |
There was a problem hiding this comment.
More details
The provenance-based endpoint and retry resolution paths preserve the intended distinctions between defaulted and explicitly configured values, including explicit schema-default URLs and zero queue sizes. Local execution was blocked by the unavailable pinned Rust toolchain and an unfetchable git dependency, so runtime confidence is limited to the surrounding code and existing test design.
🤖 Datadog Autotest · Commit f9aaad6 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| "datadog_only".to_string() | ||
| } | ||
|
|
||
| #[derive(Deserialize)] |
There was a problem hiding this comment.
Do we want to remove this? I think this is a behavior change since this causes us to not accept true as a boolean now?
Ran this regression test to confirm
#[test]
fn boolean_use_v3_api_series_enabled_is_normalized() {
let sources = SourceTree::all_explicit(json!({
"use_v3_api": {
"series": {
"enabled": true
}
}
}));
let config = translate_strict(&sources)
.expect("a boolean V3 series mode should translate");
assert_eq!(
config.shared.metrics_encoding.v3_series_mode.mode,
"true"
);
}
There was a problem hiding this comment.
This was a very important finding (perhaps, see below). I fixed it in a separate PR that is now a precursor to this one (this one has now been stacked on the fix).
To prove the where it was detected, I added your test in this PR at lib/agent-data-plane-config-system/src/system.rs around line 419.
The flaw may not have been as bad as it seemed at first. I believe the Agent actually sends us remote config post-type-coercion, and the Agent-ported coercion algorithms were already being used for ADP-local environment variables. So I think this would have only surfaced when ADP was parsing a local YAML file itself.
In any event, the fix is in #2325 and your helpful test now passes!
There was a problem hiding this comment.
More details
The typed-config cutover preserves the reviewed production paths: endpoint provenance now distinguishes explicit default URLs from Agent defaults, retry-size precedence retains explicit zero values, and MRF/Cluster Agent overrides remain isolated from global routing. No additional behavioral regression was supported by the diff and nearby tests; runtime execution was intentionally not performed.
🤖 Datadog Autotest · Commit 532719d · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
a8d8ab2 to
14477e5
Compare
13472b2 to
79c8319
Compare
Resolve Datadog primary endpoints and retry queue sizes in the typed configuration layer, preserving configuration provenance. The Agent streams dd_url at its schema default even when only site is set, so resolution needs provenance to tell an operator's choice of the default intake from a value nobody set. Migrate forwarders, metrics encoders, proxy and retry settings, MRF, and Cluster Agent construction to resolved typed configuration. Keep raw configuration access only for live API-key refresh, secrets retry behavior, and the unresolved run_path compatibility fallback. Empty API keys were rejected before typed configuration, so that validation moves to the config layer rather than being dropped. Update schema inventories, generated registries, smoke-test metadata, and component tests. The config-system skill now says where a migration has to end: values like dd_url resolve in the config layer, and no deserialization logic stays behind in the components. The endpoint integration case asserts the primary endpoint resolves from site alone, reading the site back from the translated configuration so a translation failure is distinguishable from a resolution failure, and matching configured_endpoint rather than the request URL, which carries a version prefix that would need updating on every bump.
…ss (#2317) ## Human Summary This migrates all components affected by #1965 (we were unable to detect explicitly set to default vs default config values) to typed config to pick up the fix provided in #2279. After implementation I ran a clean-room Opus audit with this prompt: > please do a clean-room audit of this commit `f4ae503e807b398ca55ee3f6275f554889cff645` > > Our goal is to correct a previous incorrect behavior in which we were not sensitive to whether a configuration value was set by the Agent as a default or whether the customer explicitly set the value (and happened to set it to the default value). > > Other than that specific intentional bug fix, no other behavioral changes are intended. > > Validate that only the desired behavioral change has occurred and that configuration defaults are in-tact. > > Read AGENTS.md and /config-system It only found two actionable items, which I fixed. One was that we were previously rejecting empty API keys, so I retained that behavior. The other was that an MRF endpoint consisting only of whitespace could be treated as a valid setting. Also fixed. All defaults stood up to scrutiny. Edit: @aqian01 found a flaw that went undetected which became #2325. ### Review Guide I tried breaking this up, which is why there are a bunch of subissues being closed all at once, but it did not divide very well. As such, the diff a bit large: Focus on these files: - `lib/saluki-components/*` - `bin/agent-data-plane/src/cli/run.rs` - `lib/agent-data-plane-config*` Deprioritize generated code and inventory churn in: - `lib/datadog-agent/*` ## AI Summary Migrate Datadog endpoint, retry, proxy, forwarder, metrics encoder, MRF, and Cluster Agent configuration consumers from raw `GenericConfiguration` deserialization to resolved typed configuration. Primary endpoint resolution now happens once in the typed configuration layer: - A default-sourced `dd_url` no longer shadows `site`. - An explicitly configured `dd_url`, including the schema-default URL, remains an override. - MRF and Cluster Agent destination overrides cannot be overwritten by global endpoint settings. - Retry queue size precedence uses configuration provenance, so explicit zero values are preserved. - Raw configuration access remains only for live API-key refresh, secrets retry behavior, and the `run_path` compatibility fallback whose schema default is still an unresolved placeholder. The schema inventories, generated registries, smoke-test metadata, and component tests are updated for the typed consumers. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - unit tests updated and created throughout - integration test modified to test this behavior. this was proven to have caught the original bug (`red -> green`) ## References - Closes: #1965 - Closes: #2310 - Closes: #2312 - Closes: #2313 - Closes: #2314 - Closes: #2315 - Closes: #2316 - Related to: #2279
## Human Summary In #2317 @aqian01 detected a flaw in the typed config deserialization mechanism. The ported, permissive, Viper type coercions that the Agent uses were only being applied to environment variables. This means that when we parsed a config file ourselves, we would not have applied those permissive coercions. This is now fixed so that we more completely mirror Agent config parsing behavior when migrating to typed config. ## AI Summary The Datadog Agent accepts configuration values through `cast`, so a leaf's declared type determines which alternate spellings it accepts. `dogstatsd_port: "8125"` and a schema-`string` leaf written as a YAML boolean (`use_v3_api.series.enabled: true`, read with `GetString`) are both valid Agent configuration. ADP's generated model instead deserialized each leaf strictly according to its schema type, causing startup translation to fail or rejecting the entire runtime configuration snapshot. This change: - Adds serde deserializers for boolean, integer, float, and string schema scalar types. - Adds a build-time `permissivize` pass that applies the appropriate deserializer to every generated scalar leaf based on its generated Rust type. - Makes `env_decode` use the same parsers, aligning file, environment, and configuration-stream inputs. - Fails code generation for unrecognized leaf types, so new schema types require an explicit coercion decision. - Retains hard errors for values that cannot be converted, rather than accepting `cast`'s zero values. - Removes the now-unnecessary `input_shape: string_or_integer` metadata and related types. Field types remain unchanged; only deserialization behavior is permissive. Numeric strings are accepted in decimal form only. The generated model diff adds deserializer attributes, and the change covers the full generated scalar model rather than individual keys. ## Change Type - [x] Bug fix ## How did you test this PR? - Unit tests cover accepted and rejected spellings for each scalar type. - A schema-driven test exercises every scalar leaf with a castable value and verifies the coerced value through a serialization round-trip. - Translation-gate tests cover a boolean on the V3 series mode string leaf and a quoted `dogstatsd_port`. - `make build-schema-overlay`, `cargo check --workspace --tests`, and `cargo clippy --workspace --tests` pass; config crate tests pass. ## References - Related: #2317 --- ## Note: #2317 merged into the wrong base I merged #2317 into `m/confra-cast` (this PR) instead of `main`. This PR now carries both changes. The #2317 description follows verbatim. --- # Merged from #2317: fix(config): migrate endpoints to typed config for provenance awareness ## Human Summary This migrates all components affected by #1965 (we were unable to detect explicitly set to default vs default config values) to typed config to pick up the fix provided in #2279. After implementation I ran a clean-room Opus audit with this prompt: > please do a clean-room audit of this commit `f4ae503e807b398ca55ee3f6275f554889cff645` > > Our goal is to correct a previous incorrect behavior in which we were not sensitive to whether a configuration value was set by the Agent as a default or whether the customer explicitly set the value (and happened to set it to the default value). > > Other than that specific intentional bug fix, no other behavioral changes are intended. > > Validate that only the desired behavioral change has occurred and that configuration defaults are in-tact. > > Read AGENTS.md and /config-system It only found two actionable items, which I fixed. One was that we were previously rejecting empty API keys, so I retained that behavior. The other was that an MRF endpoint consisting only of whitespace could be treated as a valid setting. Also fixed. All defaults stood up to scrutiny. Edit: @aqian01 found a flaw that went undetected which became #2325. ### Review Guide I tried breaking this up, which is why there are a bunch of subissues being closed all at once, but it did not divide very well. As such, the diff a bit large: Focus on these files: - `lib/saluki-components/*` - `bin/agent-data-plane/src/cli/run.rs` - `lib/agent-data-plane-config*` Deprioritize generated code and inventory churn in: - `lib/datadog-agent/*` ## AI Summary Migrate Datadog endpoint, retry, proxy, forwarder, metrics encoder, MRF, and Cluster Agent configuration consumers from raw `GenericConfiguration` deserialization to resolved typed configuration. Primary endpoint resolution now happens once in the typed configuration layer: - A default-sourced `dd_url` no longer shadows `site`. - An explicitly configured `dd_url`, including the schema-default URL, remains an override. - MRF and Cluster Agent destination overrides cannot be overwritten by global endpoint settings. - Retry queue size precedence uses configuration provenance, so explicit zero values are preserved. - Raw configuration access remains only for live API-key refresh, secrets retry behavior, and the `run_path` compatibility fallback whose schema default is still an unresolved placeholder. The schema inventories, generated registries, smoke-test metadata, and component tests are updated for the typed consumers. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - unit tests updated and created throughout - integration test modified to test this behavior. this was proven to have caught the original bug (`red -> green`) ## References - Closes: #1965 - Closes: #2310 - Closes: #2312 - Closes: #2313 - Closes: #2314 - Closes: #2315 - Closes: #2316 - Related to: #2279 Co-authored-by: matt.briggs <matt.briggs@datadoghq.com>
## Human Summary In #2317 @aqian01 detected a flaw in the typed config deserialization mechanism. The ported, permissive, Viper type coercions that the Agent uses were only being applied to environment variables. This means that when we parsed a config file ourselves, we would not have applied those permissive coercions. This is now fixed so that we more completely mirror Agent config parsing behavior when migrating to typed config. ## AI Summary The Datadog Agent accepts configuration values through `cast`, so a leaf's declared type determines which alternate spellings it accepts. `dogstatsd_port: "8125"` and a schema-`string` leaf written as a YAML boolean (`use_v3_api.series.enabled: true`, read with `GetString`) are both valid Agent configuration. ADP's generated model instead deserialized each leaf strictly according to its schema type, causing startup translation to fail or rejecting the entire runtime configuration snapshot. This change: - Adds serde deserializers for boolean, integer, float, and string schema scalar types. - Adds a build-time `permissivize` pass that applies the appropriate deserializer to every generated scalar leaf based on its generated Rust type. - Makes `env_decode` use the same parsers, aligning file, environment, and configuration-stream inputs. - Fails code generation for unrecognized leaf types, so new schema types require an explicit coercion decision. - Retains hard errors for values that cannot be converted, rather than accepting `cast`'s zero values. - Removes the now-unnecessary `input_shape: string_or_integer` metadata and related types. Field types remain unchanged; only deserialization behavior is permissive. Numeric strings are accepted in decimal form only. The generated model diff adds deserializer attributes, and the change covers the full generated scalar model rather than individual keys. ## Change Type - [x] Bug fix ## How did you test this PR? - Unit tests cover accepted and rejected spellings for each scalar type. - A schema-driven test exercises every scalar leaf with a castable value and verifies the coerced value through a serialization round-trip. - Translation-gate tests cover a boolean on the V3 series mode string leaf and a quoted `dogstatsd_port`. - `make build-schema-overlay`, `cargo check --workspace --tests`, and `cargo clippy --workspace --tests` pass; config crate tests pass. ## References - Related: #2317 --- ## Note: #2317 merged into the wrong base I merged #2317 into `m/confra-cast` (this PR) instead of `main`. This PR now carries both changes. The #2317 description follows verbatim. --- # Merged from #2317: fix(config): migrate endpoints to typed config for provenance awareness ## Human Summary This migrates all components affected by #1965 (we were unable to detect explicitly set to default vs default config values) to typed config to pick up the fix provided in #2279. After implementation I ran a clean-room Opus audit with this prompt: > please do a clean-room audit of this commit `f4ae503e807b398ca55ee3f6275f554889cff645` > > Our goal is to correct a previous incorrect behavior in which we were not sensitive to whether a configuration value was set by the Agent as a default or whether the customer explicitly set the value (and happened to set it to the default value). > > Other than that specific intentional bug fix, no other behavioral changes are intended. > > Validate that only the desired behavioral change has occurred and that configuration defaults are in-tact. > > Read AGENTS.md and /config-system It only found two actionable items, which I fixed. One was that we were previously rejecting empty API keys, so I retained that behavior. The other was that an MRF endpoint consisting only of whitespace could be treated as a valid setting. Also fixed. All defaults stood up to scrutiny. Edit: @aqian01 found a flaw that went undetected which became #2325. ### Review Guide I tried breaking this up, which is why there are a bunch of subissues being closed all at once, but it did not divide very well. As such, the diff a bit large: Focus on these files: - `lib/saluki-components/*` - `bin/agent-data-plane/src/cli/run.rs` - `lib/agent-data-plane-config*` Deprioritize generated code and inventory churn in: - `lib/datadog-agent/*` ## AI Summary Migrate Datadog endpoint, retry, proxy, forwarder, metrics encoder, MRF, and Cluster Agent configuration consumers from raw `GenericConfiguration` deserialization to resolved typed configuration. Primary endpoint resolution now happens once in the typed configuration layer: - A default-sourced `dd_url` no longer shadows `site`. - An explicitly configured `dd_url`, including the schema-default URL, remains an override. - MRF and Cluster Agent destination overrides cannot be overwritten by global endpoint settings. - Retry queue size precedence uses configuration provenance, so explicit zero values are preserved. - Raw configuration access remains only for live API-key refresh, secrets retry behavior, and the `run_path` compatibility fallback whose schema default is still an unresolved placeholder. The schema inventories, generated registries, smoke-test metadata, and component tests are updated for the typed consumers. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - unit tests updated and created throughout - integration test modified to test this behavior. this was proven to have caught the original bug (`red -> green`) ## References - Closes: #1965 - Closes: #2310 - Closes: #2312 - Closes: #2313 - Closes: #2314 - Closes: #2315 - Closes: #2316 - Related to: #2279 Co-authored-by: matt.briggs <matt.briggs@datadoghq.com> 79f5b0c

Human Summary
This migrates all components affected by #1965 (we were unable to detect explicitly set to default vs default config values) to typed config to pick up the fix provided in #2279.
After implementation I ran a clean-room Opus audit with this prompt:
It only found two actionable items, which I fixed. One was that we were previously rejecting empty API keys, so I retained that behavior. The other was that an MRF endpoint consisting only of whitespace could be treated as a valid setting. Also fixed. All defaults stood up to scrutiny.
Edit: @aqian01 found a flaw that went undetected which became #2325.
Review Guide
I tried breaking this up, which is why there are a bunch of subissues being closed all at once, but it did not divide very well. As such, the diff a bit large:
Focus on these files:
lib/saluki-components/*bin/agent-data-plane/src/cli/run.rslib/agent-data-plane-config*Deprioritize generated code and inventory churn in:
lib/datadog-agent/*AI Summary
Migrate Datadog endpoint, retry, proxy, forwarder, metrics encoder, MRF, and Cluster Agent configuration consumers from raw
GenericConfigurationdeserialization to resolved typed configuration.Primary endpoint resolution now happens once in the typed configuration layer:
dd_urlno longer shadowssite.dd_url, including the schema-default URL, remains an override.run_pathcompatibility fallback whose schema default is still an unresolved placeholder.The schema inventories, generated registries, smoke-test metadata, and component tests are updated for the typed consumers.
Change Type
How did you test this PR?
red -> green)References
dd_url's schema default makessiteunreachable in ADP. #1965