chore(config): support environment variables in the config system - #2093
Conversation
This comment has been minimized.
This comment has been minimized.
Binary Size Analysis (Agent Data Plane)Baseline: f750b4c · Comparison: 0f451da · 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 ( |
3377f6b to
5958006
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90794cc38c
ℹ️ 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".
There was a problem hiding this comment.
The csv_comma_separated decoder silently truncates input when unexpected non-comma characters follow a closing quote in a quoted field (e.g., "a"rest,"b" returns ["a"]), diverging from Go's LazyQuotes behavior which would include the characters in the field. This bug is latent: no generated env key currently uses CsvCommaSeparated (0 entries in the generated table), so production is not affected today. It will become reachable when apm_config.ignore_resources (the sole YAML field with env_parser: csv_comma_separated) is added to the typed Rust model.
🤖 Datadog Autotest · Commit 90794cc · What is Autotest? · Any feedback? Reach out in #autotest
Build the typed configuration path without figment. The compatibility configuration loader remains unchanged, while the typed path composes the file and environment sources as JSON before one normal schema-driven deserialization. - Generate an environment reader from the vendored schema. It reads real environment names, decodes each value into the shape declared by the schema, and supports the Agent's named parsers plus type-based fallbacks. - Add `LoadedConfiguration` to build the typed base with explicit file/environment precedence, including the standard HTTP and HTTPS proxy variables. - Consume Agent `ConfigUpdate` values directly in `ConfigurationSystem`. Trial-fold each update, merge through known schema sections, replace schema leaves wholesale, and commit only after translation and deserialization succeed. Invalid updates retain the last known-good typed state. - Forward every update unchanged to the compatibility configuration path so existing consumers keep their current behavior while typed consumers migrate. The generated environment reader and schema-leaf merge derive their paths from the existing schema metadata, keeping nested environment handling and source precedence explicit.
) ## Human Summary This PR should unblock the typed config project. Issues arose from trying to deserialize typed config from `GenericConfiguration` because Figment was doing a sort of heuristic coercion of environment variables without knowledge of their destination types. I believe this is actually lossy, but either way, it was easier to remove Figment from the deserialization and translation pathway (since we have the Datadog schema) than it was to try to deserialize from Figment's model of the config map. In this PR we hand the initial file read over to the typed config system which deserializes it and applies environment variable precedence to it (configurable) based on the information we have in the schema and a port of the environment variable handling algorithms as found in the Agent. Ownership of the config stream is given to the new typed config system, and `GenericConfiguration` and the typed system operate independently with it. First the typed system merges what it receives into a copy its own `serde_json::Value` map, then it updates typed configuration and sends the `ConfigUpdate` on to the `GenericConfiguration` map which behaves as it always has, using Figment, etc. If an error occurs during typed config deserialization and translation, that error will be logged, but the `ConfigUpdate` will be sent on to `GenericConfiguration` anyway in order to preserve current system behavior. The thinking behind this is that during a transition to typed configuration, we want to leave things running the same way the have been, and we will add configuration tests to make sure we have no error logs. We want to gain confidence that we aren't seeing errors during deserialization or translation and quickly switch over to blocking the `ConfigUpdate` from reaching `GenericConfiguration` in the presence of an error so that the two views of config cannot drift from one another. The ultimate goal, really, is going to be to migrate everything over to typed config as quickly as possible so that two views of the config do not even exist in the system. This PR is separate from #2094 so that this PR can land without touching `agent-data-plane`. The integration with `agent-data-plane` takes place in the next PR, but looking at that PR will be helpful in terms of understanding how this integrates with `run.rs` and system startup. ## AI Summary Support environment variables whose flat, underscore-joined names correspond to nested typed configuration paths. - Add a generated overlay for Datadog configuration keys before typed deserialization. - Discover and overlay Saluki-only nested paths from the typed source model. - Support disabled, fallback, and override overlay modes while preserving the Agent configuration stream's precedence in fallback mode. - Preserve the Agent's whitespace-separated handling for string-list environment values. The overlays are applied per source before deserialization, so relocating keys for one source does not affect the other. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - [x] Conducted an extensive clean-room audit of with independent, no-context model reviews; found and fixed numeric byte-size compatibility and full-path duration-codegen collisions. - [x] All the usual local gates with AI-added unit tests for new functionality - [x] CI ## References Stacked on [#1986](#1986). Co-authored-by: matt.briggs <matt.briggs@datadoghq.com> be0a7a1
## Human Summary Integrates the typed config system into `agent-data-plane` even though no components are yet reading from it. We decided to take a migration approach to this, so components will be cut over to using typed config in isolated PRs where we can focus on correctness for each component. This PR introduces a privileged API endpoint `/config/internal` which is a serialization of `SalukiConfig`. This is going to be important for end-to-end integration testing, and the first such test is added in this PR. Also noteworthy is that all integration and correctness tests are passing here, which was not the case prior to #2093's improvement of how the type config system handles environment variables. ## AI Summary Integrate the typed configuration system into the Agent Data Plane runtime and expose its current value through the internal configuration endpoint. - Load the configuration system after the initial configuration snapshot is available. - Thread the configuration-system handle through runtime setup and the internal supervisor. - Serve the current configuration through `/config/internal`. - Use fallback environment-overlay behavior so Agent-provided values remain authoritative. - Add integration coverage for runtime configuration updates through the internal endpoint. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay` - `make fmt` - `make check-docs` - `make check-all` - `make test` All checks passed; 1,830 tests passed and 31 were skipped. ## References Stacked on [#2093](#2093). Co-authored-by: matt.briggs <matt.briggs@datadoghq.com>
## Human Summary Integrates the typed config system into `agent-data-plane` even though no components are yet reading from it. We decided to take a migration approach to this, so components will be cut over to using typed config in isolated PRs where we can focus on correctness for each component. This PR introduces a privileged API endpoint `/config/internal` which is a serialization of `SalukiConfig`. This is going to be important for end-to-end integration testing, and the first such test is added in this PR. Also noteworthy is that all integration and correctness tests are passing here, which was not the case prior to #2093's improvement of how the type config system handles environment variables. ## AI Summary Integrate the typed configuration system into the Agent Data Plane runtime and expose its current value through the internal configuration endpoint. - Load the configuration system after the initial configuration snapshot is available. - Thread the configuration-system handle through runtime setup and the internal supervisor. - Serve the current configuration through `/config/internal`. - Use fallback environment-overlay behavior so Agent-provided values remain authoritative. - Add integration coverage for runtime configuration updates through the internal endpoint. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay` - `make fmt` - `make check-docs` - `make check-all` - `make test` All checks passed; 1,830 tests passed and 31 were skipped. ## References Stacked on [#2093](#2093). Co-authored-by: matt.briggs <matt.briggs@datadoghq.com> e4cba81

Human Summary
This PR should unblock the typed config project. Issues arose from trying to deserialize typed config from
GenericConfigurationbecause Figment was doing a sort of heuristic coercion of environment variables without knowledge of their destination types. I believe this is actually lossy, but either way, it was easier to remove Figment from the deserialization and translation pathway (since we have the Datadog schema) than it was to try to deserialize from Figment's model of the config map.In this PR we hand the initial file read over to the typed config system which deserializes it and applies environment variable precedence to it (configurable) based on the information we have in the schema and a port of the environment variable handling algorithms as found in the Agent.
Ownership of the config stream is given to the new typed config system, and
GenericConfigurationand the typed system operate independently with it. First the typed system merges what it receives into a copy its ownserde_json::Valuemap, then it updates typed configuration and sends theConfigUpdateon to theGenericConfigurationmap which behaves as it always has, using Figment, etc.If an error occurs during typed config deserialization and translation, that error will be logged, but the
ConfigUpdatewill be sent on toGenericConfigurationanyway in order to preserve current system behavior. The thinking behind this is that during a transition to typed configuration, we want to leave things running the same way the have been, and we will add configuration tests to make sure we have no error logs.We want to gain confidence that we aren't seeing errors during deserialization or translation and quickly switch over to blocking the
ConfigUpdatefrom reachingGenericConfigurationin the presence of an error so that the two views of config cannot drift from one another. The ultimate goal, really, is going to be to migrate everything over to typed config as quickly as possible so that two views of the config do not even exist in the system.This PR is separate from #2094 so that this PR can land without touching
agent-data-plane. The integration withagent-data-planetakes place in the next PR, but looking at that PR will be helpful in terms of understanding how this integrates withrun.rsand system startup.AI Summary
Support environment variables whose flat, underscore-joined names correspond to nested typed configuration paths.
The overlays are applied per source before deserialization, so relocating keys for one source does not affect the other.
Change Type
How did you test this PR?
References
Stacked on #1986.