chore(config): migrate the Datadog forwarder to typed config#1993
Conversation
dd-octo-sts
Bot
added
area/components
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Migrates the Datadog forwarder subsystem from GenericConfiguration map-based lookups to the typed SalukiConfiguration/SharedConfiguration model, including live refresh via Live<T> projections. This aligns forwarder construction and runtime refresh behavior with the centralized config-inventory work (issue #1788) and reduces reliance on raw serde parsing/default restatement.
Changes:
- Rewired Datadog + Cluster Agent forwarders and related runtime refresh paths to use typed config (
SharedConfiguration) and live views (Live<SalukiConfiguration>). - Refactored endpoint/API-key refresh logic to be model-driven (primary, MRF override, additional endpoints) and updated validation/retry gating accordingly.
- Promoted
run_pathand secrets-management keys into inventory, generated witness/typed schema fields, translator consumption, and docs updates.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/saluki-components/src/forwarders/datadog/mod.rs | Builds forwarder config from typed shared model; threads optional live view for endpoint API-key refresh. |
| lib/saluki-components/src/forwarders/cluster_agent/mod.rs | Switches forwarder config derivation to typed shared model for consistent defaults/proxy/retry behavior. |
| lib/saluki-components/src/common/datadog/validation.rs | Updates API-key validation task to revalidate based on typed live projections of relevant key sources. |
| lib/saluki-components/src/common/datadog/retry.rs | Rebuilds retry config from typed forwarder model and updates the 403 “secrets gate” to use live typed config. |
| lib/saluki-components/src/common/datadog/proxy.rs | Builds proxy configuration from typed proxy model and removes raw-config deserialization paths. |
| lib/saluki-components/src/common/datadog/io.rs | Threads typed live config into I/O forwarder and adjusts proxy setup to typed config shape. |
| lib/saluki-components/src/common/datadog/endpoints.rs | Refactors endpoint resolution and API-key refresh to use typed model fields and Live<T> sources. |
| lib/saluki-components/src/common/datadog/config.rs | Replaces serde-driven forwarder config with field-by-field construction from shared typed model. |
| lib/datadog-agent/config/src/generated/witness.rs | Adds witnessed keys for run_path and secrets-management fields. |
| lib/datadog-agent/config/src/generated/datadog_configuration.rs | Adds generated schema fields/defaults for run_path and secrets-management keys. |
| lib/datadog-agent/config/schema/schema_overlay.yaml | Promotes run_path and secrets-management keys from excluded to inventory with metadata. |
| lib/agent-data-plane-config/src/shared.rs | Extends shared typed config with secrets and run_path used by forwarder/retry logic. |
| lib/agent-data-plane-config-system/src/translators/datadog_translator.rs | Consumes newly witnessed keys into the typed shared configuration. |
| docs/agent-data-plane/configuration/dogstatsd.md | Documents new inventory keys in the configuration reference table. |
| bin/agent-data-plane/src/cli/run.rs | Updates topology wiring to pass typed shared config + live views into forwarders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let classifier = if let Some(live) = live { | ||
| let gate: HttpRetryPredicate<B> = | ||
| Arc::new(move |response| response.status() == StatusCode::FORBIDDEN && secrets_in_use(&config)); | ||
| Arc::new(move |response| response.status() == StatusCode::FORBIDDEN && secrets_in_use(&live)); | ||
| StandardHttpClassifier::new().with_predicate(gate) |
| fn consume_run_path(&mut self, value: String) { | ||
| self.config.shared.run_path = PathBuf::from(value); | ||
| } |
| ApiKeySource::Additional { keys, url, index } => keys | ||
| .as_ref()? | ||
| .current() | ||
| .get(url) | ||
| .and_then(|list| list.get(*index)) | ||
| .map(|key| key.trim()) | ||
| .filter(|key| !key.is_empty()) | ||
| .map(str::to_string), |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab8a10af9c
ℹ️ 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".
| api_key: endpoints.api_key.clone(), | ||
| api_key_refresh: ApiKeyRefresh::Primary, | ||
| site: endpoints.site.clone().unwrap_or_else(|| DEFAULT_SITE.to_string()), | ||
| dd_url: endpoints.dd_url.clone(), |
There was a problem hiding this comment.
When a user sets only site (for example datadoghq.eu) and leaves dd_url unset, the typed DatadogConfiguration still supplies the generated default dd_url of https://app.datadoghq.com. Copying that value here makes build_primary_endpoint treat dd_url as explicitly configured, so the site setting is ignored and traffic is routed to the default US endpoint. Before this migration, dd_url stayed None unless it was actually present in the raw config.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn consume_run_path(&mut self, value: String) { | ||
| self.config.shared.run_path = PathBuf::from(value); |
There was a problem hiding this comment.
When run_path is omitted, the generated Datadog model defaults it to the literal string ${run_path}; storing that verbatim means RetryConfiguration::from_model sees a non-empty run path and derives ${run_path}/transactions_to_retry whenever forwarder_storage_path is empty. In configurations that enable disk persistence via forwarder_storage_max_size_in_bytes without an explicit storage path, the retry queue will initialize under a literal placeholder path or fail instead of using a real runtime directory or leaving the path unset.
Useful? React with 👍 / 👎.
Binary Size Analysis (Agent Data Plane)Baseline: 7b98c6f · Comparison: fb62d7d · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
| ApiKeySource::Additional { keys, url, index } => keys | ||
| .as_ref()? | ||
| .current() | ||
| .get(url) | ||
| .and_then(|list| list.get(*index)) | ||
| .map(|key| key.trim()) | ||
| .filter(|key| !key.is_empty()) | ||
| .map(str::to_string), |
| fn secrets_in_use(live: &Live<SalukiConfiguration>) -> bool { | ||
| let config = live.current(); | ||
| let secrets = &config.shared.secrets; | ||
| secrets.refresh_on_api_key_failure_interval > 0 || !secrets.backend_command.trim().is_empty() | ||
| } |
| fn consume_run_path(&mut self, value: String) { | ||
| self.config.shared.run_path = PathBuf::from(value); | ||
| } |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (3)Experiments configured
Bounds Checks: ✅ Passed (3)
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 ( |
webern
left a comment
webern
left a comment
There was a problem hiding this comment.
Claude Code, reviewing on behalf of webern.
Overall this is a clean, faithful cutover: field-by-field construction, no restated defaults, tests rewritten to build from the typed model, and the runtime API-key refresh reworked into an explicit ApiKeySource that preserves the primary / MRF / additional-endpoint behaviors. I checked the promoted keys against the vendored schema (run_path, secret_backend_command, secret_refresh_on_api_key_failure_interval are all witnessed correctly), and the forwarder numeric defaults flowing through the generated DatadogConfiguration match the old serde defaults (concurrency 10, num_workers 1, timeout 20, buffer 100, backoff 2/2/64, recovery 2, flush ratio 0.5, disk ratio 0.8, outdated 10 days, capacity 900s).
There is one blocker and one narrower issue. Both were also flagged independently by Codex/Copilot and I agree with them.
Blocker: dd_url schema default overrides site (data routing regression)
consume_dd_url sets shared.endpoints.dd_url = non_empty(value), and the generated DatadogConfiguration.dd_url defaults to the schema value https://app.datadoghq.com. So when a user does not set dd_url, the typed model still carries dd_url = Some("https://app.datadoghq.com") rather than None.
EndpointConfiguration::from_model copies that through, and calculate_resolved_endpoint treats any Some(dd_url) as an explicit override that takes precedence over site:
let raw_endpoint = match override_url {
Some(url) => url.to_string(), // dd_url wins, site ignored
None => format!("https://app.{}", base_domain) // site path
};Net effect: a deployment that sets only site (e.g. datadoghq.eu, us5.datadoghq.com, ap1.datadoghq.com) and leaves dd_url unset — the normal way non-US customers configure the intake — now has its primary endpoint silently routed to app.datadoghq.com. Before this PR the forwarder read the raw dd_url key, which stayed None when unset, so site was honored.
This is the root cause already documented in the TODO(#1965) comment on Endpoints: consume_dd_url can't distinguish "user set it to the default URL" from "schema default filled it in," so it can't safely populate dd_url. The forwarder was the last reader relying on the raw-config escape hatch, and this PR removes it.
Worth noting the test suite doesn't catch this because the forwarder tests build from SharedConfiguration::default() (where dd_url is None), not from a translated config where the schema default is applied. A test that sets only site and asserts the resolved primary endpoint would have surfaced it, and would be good to add alongside the fix.
Narrower: run_path placeholder can seed a literal retry-queue path
run_path's schema default is the literal template ${run_path}, and consume_run_path stores it verbatim (saluki does no ${...} expansion). RetryConfiguration::from_model then sees a non-empty run path and, when forwarder_storage_path is empty, derives ${run_path}/transactions_to_retry. Previously an unset run_path read as None and left the storage path empty.
This is gated behind forwarder_storage_max_size_in_bytes > 0, so it's inert in the default (disk-persistence-off) configuration. But a deployment that enables on-disk retry without an explicit forwarder_storage_path and without run_path would initialize the queue under a literal ${run_path} directory instead of leaving it unset. Treating ${run_path} (and empty/whitespace) as unset in consume_run_path would restore the prior behavior.
Secondary (perf, non-blocking)
The retry 403 gate clones the entire SalukiConfiguration on every evaluated 403 (secrets_in_use → live.current()). On a sustained-403 path that's a large clone per response; projecting to Live<Secrets> once when building the predicate would keep it to the small struct. Copilot also flagged the additional-endpoints api_key() refresh cloning the full map per request — comparable to the old per-call deserialize, so less of a change, but a borrowed read path on Live would help both. These are follow-ups, not merge blockers.
CI is green (only the ADP benchmark job pending).
|
Follow-up to my review: after reading #1965, I no longer consider the |
| pub(crate) fn from_model(forwarder: &Forwarder, run_path: &Path) -> Self { | ||
| let mut storage_path = forwarder.storage_path.clone(); | ||
| if storage_path.parent().is_none() && !run_path.as_os_str().is_empty() { | ||
| storage_path = run_path.join(RETRY_TXN_DIR); | ||
| } |
| let trimmed = value.trim(); | ||
| if trimmed.is_empty() || trimmed == "${run_path}" { | ||
| return; | ||
| } | ||
| self.config.shared.run_path = PathBuf::from(value); | ||
| } |
webern
left a comment
webern
left a comment
There was a problem hiding this comment.
Claude Code, reviewing on behalf of webern — fresh clean-room pass on the current HEAD (07733e3).
First, the two items from the earlier round that the follow-up commits addressed, so they're off the table: consume_run_path now treats the ${run_path} placeholder (and empty/whitespace) as unset, and the 403 retry gate projects to Live<Secrets> instead of cloning the whole SalukiConfiguration per response. Both look good.
The rest of the cutover holds up well: field-by-field construction, no restated defaults, the forwarder numeric defaults flowing through the generated DatadogConfiguration still match the old serde defaults, and the ApiKeySource rework faithfully preserves the primary / MRF / additional-endpoint refresh behavior. The promoted keys (run_path, secret_backend_command, secret_refresh_on_api_key_failure_interval) are witnessed correctly against the vendored schema.
There is still one blocker, plus a second instance of the same root cause that hasn't been called out yet.
Blocker (still open): dd_url schema default shadows site
This is the same routing regression flagged last round, and it is unchanged at HEAD.
consume_dd_url sets shared.endpoints.dd_url = non_empty(value), and the generated DatadogConfiguration.dd_url defaults to the schema value https://app.datadoghq.com. drive() calls the consumer unconditionally, so when a user does not set dd_url the typed model still carries dd_url = Some("https://app.datadoghq.com") rather than None.
EndpointConfiguration::from_model copies that through, and calculate_resolved_endpoint treats any Some(dd_url) as an explicit override that beats site:
let raw_endpoint = match override_url {
Some(url) => url.to_string(), // dd_url wins, site ignored
None => format!("https://app.{}", base_domain), // site path
};Net effect: a deployment that sets only site (datadoghq.eu, us5.datadoghq.com, ap1.datadoghq.com, ...) and leaves dd_url unset — the normal way non-US1 customers point at their intake — now has its primary endpoint silently routed to app.datadoghq.com. Before this PR the forwarder read the raw dd_url key, which stayed None when unset, so site was honored. This is the last reader that relied on that raw escape hatch, so this cutover is what activates the regression.
TODO(#1965) on Endpoints already documents the underlying cause: the translator can't tell "user set dd_url to the default URL" from "schema default filled it in." One workable option here is that the dd_url default (https://app.datadoghq.com) and the default site resolution (datadoghq.com -> https://app.datadoghq.com) produce the same URL, so treating a dd_url equal to the schema default as unset in consume_dd_url would restore site precedence in the common case without hurting the coinciding-default case. Either way this needs to be resolved (or explicitly deferred with the routing consequence understood) before it merges.
The forwarder tests don't catch it because they build from SharedConfiguration::default() (where dd_url is None), not from a translated config where the schema default is applied. A test that sets only site and asserts the resolved primary endpoint would surface it.
Same root cause, not yet flagged: the deprecated forwarder_retry_queue_max_size fallback is now dead
queue_max_size_bytes() still prefers retry_queue_payloads_max_size and falls back to the deprecated retry_queue_max_size:
self.retry_queue_payloads_max_size
.or(self.retry_queue_max_size)
.unwrap_or(FORWARDER_RETRY_QUEUE_PAYLOADS_MAX_SIZE_BYTES)That .or() chain depends on retry_queue_payloads_max_size being None when the user didn't set it. But consume_forwarder_retry_queue_payloads_max_size sets Some(...) unconditionally, and drive() always feeds it the generated default (15728640). So post-translation the model always carries retry_queue_payloads_max_size = Some(15 MiB), the .or() never reaches the deprecated key, and a config that sets only forwarder_retry_queue_max_size now silently gets 15 MiB instead of the configured value. Previously that field deserialized as None when unset (no serde default), so the fallback worked.
Impact is much narrower than dd_url — it only bites a deprecated-key-only config — but it's the same defect: an Option that used to encode "was it set" is now always Some(default) after drive(). The rewritten queue_max_size_bytes_fallback_behavior test hides it by building Forwarder::default() (payloads = None), a shape that never occurs after translation. Whatever the fix for dd_url, this one wants the same treatment or an explicit note that the deprecated fallback is being dropped.
Test loss worth relocating
The four deser_additional_endpoints_* tests were deleted, but AdditionalEndpoints's serde parsing (JSON-string maps via PickFirst<DisplayFromStr>, and the single-string-or-seq value shape) is still live — it's used by the not-yet-migrated DatadogMetricsConfiguration, as the retained #[serde_as] and the TODO on the type note. Deleting those tests leaves that live parsing path with no coverage. Since the code isn't going away yet, relocating the deser tests (rather than dropping them) would keep the coverage until the metrics encoder migrates.
Minor / non-blocking
- Copilot's note that
RetryConfiguration::from_modelusesstorage_path.parent().is_none()(also true for/) — worth tidying, but it matches the pre-cutover behavior, so it's not a regression from this PR. - Copilot's note that
consume_run_pathstores the untrimmedvalueafter checkingtrimmed— cosmetic; a padded path was also possible before. Storing the trimmed value would be slightly cleaner. - Copilot's per-request clone of the whole
additional_endpointsmap inrefreshed_api_key— reasonable follow-up, comparable to the old per-call deserialize, not a merge blocker.
|
Claude Code, on behalf of webern — a follow-up now that the fixes have landed. Two of the three points from my earlier review are resolved:
On the third point — |
## Human Summary Caught by AI when reviewing #1993, it looks like this was a regression in the environment variable path only, which *should not* have affected customers anyway, but it looks like now we handle JSON in environment variable correctly if we happen to ever read that. ## AI Summary Restores the wider `additional_endpoints` input shapes that were lost when the metrics encoder and forwarder moved off the component serde onto the typed model. The old `AdditionalEndpoints` serde used `PickFirst<(DisplayFromStr, _)>` plus `OneOrMany`, so it accepted: - the whole map as a JSON string — the form an environment variable produces, e.g. `DD_ADDITIONAL_ENDPOINTS='{"https://app.datadoghq.com":["key"]}'`, and - a bare string in place of a one-element key list for a host. The generated `DatadogConfiguration` deserializes `additional_endpoints` as a plain `HashMap<String, Vec<String>>`, and the env overlay only materializes scalar/space-separated-list env values, so neither shape survives. Both now fail deserialization outright — a dual-shipping deployment configured through the environment fails to start. This adds an `additional_endpoints` case to `normalize_datadog_input_forms` (the same pass that already restores the `dogstatsd_eol_required` and `dogstatsd_mapper_profiles` env-var forms): parse the JSON-string form into an object, then wrap any bare-string host value into a one-element array. Invalid JSON is left in place so the downstream deserializer still surfaces the error. ## Change Type - [x] Bug fix ## How did you test this PR? - `cargo nextest run -p agent-data-plane-config-system` (added `additional_endpoints_accepts_json_string_scalar_or_map`, covering the JSON-string, JSON-string-with-scalar, native-scalar, and native-list shapes; confirmed it fails without the fix with `invalid type: string ..., expected a map`). - `make fmt` ## References - Merges into `m/pr5-cutover`
Cut the Datadog forwarder subsystem over from the raw GenericConfiguration map to the typed SalukiConfiguration model: - ForwarderConfiguration, EndpointConfiguration, ProxyConfiguration, and RetryConfiguration now build field-by-field from the shared endpoints model (no source serde, no restated defaults). - ResolvedEndpoint, TransactionForwarder, and ApiKeyValidator refresh API keys and secrets from a Live<SalukiConfiguration> view instead of a live GenericConfiguration; the reactive API-key sources are the primary key, the MRF override key, and the dual-shipping endpoint keys. - DatadogForwarderConfiguration builds from the model plus a live view; the Cluster Agent forwarder and run.rs call sites are rewired accordingly. - Home secret_backend_command, secret_refresh_on_api_key_failure_interval, and run_path as witnessed keys (promoted from excluded) under shared.secrets and shared.run_path. Progresses #1788
…recated fallback The forwarder retry-queue size keys forwarder_retry_queue_payloads_max_size and forwarder_retry_queue_max_size are now flagged saluki_overrides_default in the schema overlay, so the generator emits them as absence-aware Option<i64> instead of baking in the schema's 15 MiB default. The translator maps the Option through (preserving the max(0) clamp), leaving both model fields None when the operator does not set them. This restores the deprecated-key fallback in RetryConfiguration::queue_max_size_bytes(): a config setting only forwarder_retry_queue_max_size now resolves to that value rather than being clobbered by the schema default that drive() previously wrote unconditionally. Adds translator-level tests that exercise the deserialize -> translate() path (the only path that reproduced the regression).
The forwarder cutover moved additional_endpoints onto the typed model, and #1990 did the same for the metrics encoder, so AdditionalEndpoints's serde impl has no production caller. Its deserialization tests were the last consumer of serde_yaml in this crate; drop the now-unused dependency.
| fn consume_run_path(&mut self, value: String) { | ||
| // The vendored schema default is the literal template `${run_path}`, which Saluki does not | ||
| // expand. Treat that placeholder (as well as empty or whitespace-only values) as unset, | ||
| // leaving `run_path` as the empty default so the forwarder does not derive a bogus | ||
| // `${run_path}/transactions_to_retry` storage path. | ||
| let trimmed = value.trim(); | ||
| if trimmed.is_empty() || trimmed == "${run_path}" { | ||
| return; | ||
| } | ||
| self.config.shared.run_path = PathBuf::from(value); | ||
| } |
| let api_key_source = ApiKeySource::Additional { | ||
| keys: live | ||
| .as_ref() | ||
| .map(|live| live.project(|c| &c.shared.endpoints.additional_endpoints)), | ||
| url: raw_endpoint.to_string(), | ||
| index, | ||
| }; |
| @@ -86,7 +84,7 @@ impl ForwarderBuilder for DatadogForwarderConfiguration { | |||
| let forwarder = TransactionForwarder::from_config( | |||
| context, | |||
| self.forwarder_config.clone(), | |||
| self.configuration.clone(), | |||
| self.live.clone(), | |||
| get_dd_endpoint_name, | |||
| let saluki = config_system.config(); | ||
| let dd_forwarder_config = | ||
| DatadogForwarderConfiguration::from_model(&saluki.shared, Some(config_system.live(|c| c))) | ||
| .error_context("Failed to configure Datadog forwarder.")?; |
## Human Summary Caught by AI when reviewing #1993, it looks like this was a regression in the environment variable path only, which *should not* have affected customers anyway, but it looks like now we handle JSON in environment variable correctly if we happen to ever read that. ## AI Summary Restores the wider `additional_endpoints` input shapes that were lost when the metrics encoder and forwarder moved off the component serde onto the typed model. The old `AdditionalEndpoints` serde used `PickFirst<(DisplayFromStr, _)>` plus `OneOrMany`, so it accepted: - the whole map as a JSON string — the form an environment variable produces, e.g. `DD_ADDITIONAL_ENDPOINTS='{"https://app.datadoghq.com":["key"]}'`, and - a bare string in place of a one-element key list for a host. The generated `DatadogConfiguration` deserializes `additional_endpoints` as a plain `HashMap<String, Vec<String>>`, and the env overlay only materializes scalar/space-separated-list env values, so neither shape survives. Both now fail deserialization outright — a dual-shipping deployment configured through the environment fails to start. This adds an `additional_endpoints` case to `normalize_datadog_input_forms` (the same pass that already restores the `dogstatsd_eol_required` and `dogstatsd_mapper_profiles` env-var forms): parse the JSON-string form into an object, then wrap any bare-string host value into a one-element array. Invalid JSON is left in place so the downstream deserializer still surfaces the error. ## Change Type - [x] Bug fix ## How did you test this PR? - `cargo nextest run -p agent-data-plane-config-system` (added `additional_endpoints_accepts_json_string_scalar_or_map`, covering the JSON-string, JSON-string-with-scalar, native-scalar, and native-list shapes; confirmed it fails without the fix with `invalid type: string ..., expected a map`). - `make fmt` ## References - Merges into `m/pr5-cutover`
Agents are thrashing hard on #1965 and they have a hard time understanding that we can't fix it right now. I tried to get everything else fixed. Cuts the Datadog forwarder subsystem over from the raw `GenericConfiguration` map to the typed `SalukiConfiguration` model. This is a tightly-coupled cluster, so it moves together in one PR. Static construction (no more source serde, no restated defaults): - `ForwarderConfiguration`, `EndpointConfiguration`, `ProxyConfiguration`, and `RetryConfiguration` build field-by-field from `shared.endpoints` (and `shared.metrics_encoding` for the V3 settings). - `DatadogForwarderConfiguration` builds from the shared model; the Cluster Agent forwarder and the `run.rs` call sites are rewired to pass typed slices. Runtime refresh (now reads a `Live<SalukiConfiguration>` view instead of a live `GenericConfiguration`): - `ResolvedEndpoint` refreshes its API key from a typed source: the shared primary key, the Multi-Region Failover override key, or the dual-shipping endpoint keys by URL/index. - `TransactionForwarder` and `ApiKeyValidator` thread the live view; the retry policy's 403 secrets gate reads it too. `ApiKeyValidator` re-validates when any API-key source changes. Config model: - `secret_backend_command`, `secret_refresh_on_api_key_failure_interval`, and `run_path` are witnessed keys (in the vendored schema), promoted from `excluded:` to `inventory:` and modeled as `shared.secrets` and `shared.run_path`, with `consume_*` bodies in the Datadog translator. The `ForwarderConfiguration` and `ProxyConfiguration` config smoke tests are retired; equivalent coverage now lives in the construction and translator tests. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` (no changes) - `make check-all` - `make check-docs` - `cargo nextest run` for `saluki-components` (`common::datadog`, `forwarders`), `agent-data-plane-config`, `agent-data-plane-config-system`, and the `agent-data-plane` config/cli modules - Progresses #1788 - Merges into `m/pr5-cutover`
Human Summary
Agents are thrashing hard on #1965 and they have a hard time understanding that we can't fix it right now. I tried to get everything else fixed.
AI Summary
Cuts the Datadog forwarder subsystem over from the raw
GenericConfigurationmap to the typedSalukiConfigurationmodel. This is a tightly-coupled cluster, so it moves together in one PR.Static construction (no more source serde, no restated defaults):
ForwarderConfiguration,EndpointConfiguration,ProxyConfiguration, andRetryConfigurationbuild field-by-field from
shared.endpoints(andshared.metrics_encodingfor the V3 settings).DatadogForwarderConfigurationbuilds from the shared model; the Cluster Agent forwarder and therun.rscall sites are rewired to pass typed slices.Runtime refresh (now reads a
Live<SalukiConfiguration>view instead of a liveGenericConfiguration):ResolvedEndpointrefreshes its API key from a typed source: the shared primary key, theMulti-Region Failover override key, or the dual-shipping endpoint keys by URL/index.
TransactionForwarderandApiKeyValidatorthread the live view; the retry policy's 403 secretsgate reads it too.
ApiKeyValidatorre-validates when any API-key source changes.Config model:
secret_backend_command,secret_refresh_on_api_key_failure_interval, andrun_patharewitnessed keys (in the vendored schema), promoted from
excluded:toinventory:and modeled asshared.secretsandshared.run_path, withconsume_*bodies in the Datadog translator.The
ForwarderConfigurationandProxyConfigurationconfig smoke tests are retired; equivalentcoverage now lives in the construction and translator tests.
Change Type
How did you test this PR?
make build-schema-overlay && make fmt(no changes)make check-allmake check-docscargo nextest runforsaluki-components(common::datadog,forwarders),agent-data-plane-config,agent-data-plane-config-system, and theagent-data-planeconfig/cli modules
References
m/pr5-cutover