chore(config): fix env-overlay behavior and saluki only shape#1986
chore(config): fix env-overlay behavior and saluki only shape#1986webern wants to merge 6 commits into
Conversation
|
Binary Size Analysis (Agent Data Plane)Baseline: 2715682 · Comparison: f4bc7fe · 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 (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
changed the title
![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: 3a5cfe3652
ℹ️ 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 flats: Vec<String> = candidate_flats | ||
| .into_iter() | ||
| .filter(|flat| !(segments.len() == 1 && segments[0] == flat)) | ||
| .collect(); |
There was a problem hiding this comment.
For top-level StringList settings such as dogstatsd_tags, this filter removes the only flat form because it is already at the destination path, so no EnvOverlayKey is emitted and apply_env_overlay never calls to_string_list. With an env var like DD_DOGSTATSD_TAGS="env:prod team:core", the merged value remains a JSON string while DatadogConfiguration::dogstatsd_tags expects Vec<String>, causing the configuration-system load to fail instead of splitting the list. Keep these entries, or split in place, whenever value_type is StringList.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was a good finding. I am pushing a fix that I still need to take a closer look at before merging.
webern
force-pushed
the
m/pr3-dynamic
branch
2 times, most recently
from
4e175c2 to
fea786d
Compare
Environment variables reach the config map as flat, underscore-joined top-level keys (`DD_AUTOSCALING_FAILOVER_ENABLED` -> `autoscaling_failover_enabled`), while `DatadogConfiguration` deserializes the nested shape. The whole-struct deserialize never reads those flat keys, so an env var set for a multi-segment key is silently dropped. The legacy per-key lookup bridged this at query time by retrying the dotted key with `.` replaced by `_`; the typed path issues no per-key queries, so that bridge no longer fires. Reinstate it as a one-time pass over the merged value, driven by a generated table of the supported multi-segment keys. For each known dotted path, relocate any matching flat key into the nested slot the deserializer reads. This runs the safe direction (known path -> its single flat form), so it never has to guess where an arbitrary flat key's nesting boundaries are. String lists are split on whitespace to match the Agent's env convention; scalars relocate verbatim. The mode is chosen at deserialization: Fallback fills only slots the Agent left empty (matching the legacy precedence, restoring prior behavior), Override lets an env var replace an Agent-supplied value, and Disabled relocates nothing. The overlay is applied only to the Datadog input; the Saluki-only source deserializes from the untouched value and is out of scope here. (cherry picked from commit 7e6e4095dbfc12224773216c4c6ecf6611103442)
Preparatory fixes the component cutovers depend on; none of these change a component yet. - Load ConfigurationSystem before topology assembly and thread the system handle down to the DogStatsD pipeline builder (unused for now, hence the leading underscore). - Reshape the Saluki-only source struct to mirror the config key hierarchy exactly, so each field maps by plain serde to its real config-key path. - Model the memory limit as a byte size (u64 in the model, ByteSize in the source) so a bare-integer value no longer fails the whole config load.
Environment variables reach the merged map as flat, underscore-joined keys, but SalukiOnly deserializes a nested shape, so a DD_* var for a dotted Saluki-only path (e.g. DD_DATA_PLANE_STANDALONE_MODE) was silently dropped. The Datadog overlay already fixes this for schema keys via a generated table; SalukiOnly has no table by design. Discover SalukiOnly's canonical leaf paths algorithmically from its derived Deserialize using a hand-rolled recording deserializer (no new dependency), treating scalar-like custom types (DurationString, ByteSize, Duration) as leaves. Cache the paths. Before deserializing SalukiOnly, relocate each multi-segment path's flat form into its nested slot, honoring EnvOverlayMode (Disabled/Fallback/Override). Single-segment keys are already flat and untouched.
Human Summary
Part of the typed config initiative, this PR retains ADP's ability to read
DD_*environmentvariables for dotted config paths. The alias paths are created from the schema and added to the
deserialization logic of
DatadogConfiguration.It is debatable whether this should be done when we are receiving our authoritative config from the
Agent, but it is definitely needed in standalone mode, so the behavior is retained during the
typed-config migration.
AI Summary
Preserves environment-variable reachability when ADP deserializes its complete typed source models
instead of querying individual keys from
GenericConfiguration.For
DatadogConfiguration, environment variables enter the merged map as flat keys while thegenerated model expects nested paths. A generated table maps each supported Datadog key's actual
Agent environment-variable names to its nested path. It records alias priority, handles declared
environment names that differ from a mechanical
.-to-_conversion, and preserves the Agent'shandling of string-list values.
For
SalukiOnly, there is deliberately no key table or alias list. A recording serde deserializerdiscovers the canonical leaf paths directly from the hand-written source model, and the overlay
derives each flat environment spelling algorithmically by replacing path separators with
underscores. Discovery descends through optional nested structs, treats scalar-like source types as
leaves, and is cached after its first run. Single-segment keys are already flat and are left alone.
Both overlays honor the selected precedence mode:
Fallbackfills only a missing nested value, leaving Agent-supplied configuration authoritative.Overrideallows a flat environment value to replace an existing nested value.Disabledleaves the merged map unchanged.This PR also prepares the configuration system for typed component construction without cutting a
component over:
ConfigurationSystembefore topology assembly and threads it to the DogStatsD builder.SalukiOnlyto mirror canonical source key paths, so plain serde transports top-level andnested keys correctly.
memory_limitas a byte size at the source boundary, accepting integer byte counts andbyte-size strings.
coverage.
Change Type
How did you test this PR?
cargo test -p agent-data-plane-config-system saluki_env_overlaycargo check -p agent-data-plane-config-systemcargo check -p agent-data-planeDatadog env-name overrides, alias priority, string-list conversion, optional-struct discovery,
scalar-like leaves, and Saluki-only source transport.
References