chore(config): fix the struct shape for saluki-only fields#1986
Merged
Conversation
|
1 task
Contributor
Author
This was referenced Jul 4, 2026
Binary Size Analysis (Agent Data Plane)Baseline: 33d6ac6 · Comparison: 75c5e64 · 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 ( |
1 task
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".
1 task
dd-octo-sts
Bot
added
the
area/test
Rework the shape of SalukiConfiguration and the SalukiOnly source so each maps cleanly to its real config-key path, before the environment-variable and runtime work builds on top. No component or runtime behavior changes here. - 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. - Give Aggregation an explicit Default whose values match what the aggregate transform expects; a zero window, in particular, is invalid downstream. - Drop the duplicate counter_expiry_seconds Saluki-only field: the key lives in the Datadog schema, so the witnessed path is authoritative and the source copy is a second source of truth.
webern
force-pushed
the
m/pr3b-env-keys
branch
2 times, most recently
from
095bd65 to
75c5e64
Compare
dd-octo-sts
Bot
removed
area/components
webern
changed the title
4 tasks
webern
changed the title
webern
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75c5e64744
ℹ️ 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".
webern
changed the title
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Jul 15, 2026
) ## 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>
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Jul 15, 2026
) ## 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
It is challenging to get the new typed config into exactly the right shape for component integration ahead of time since the context is so large. But one big mistake had to be fixed.
SalukiOnlyis a deserialization struct and needs to match the shape of the keys it will be deserializing. Fixed in this PR.AI Summary
Reshape the typed configuration model and Saluki-only source so their structures match the configuration-key hierarchy directly.
SalukiOnlyto deserialize the Saluki-only configuration paths without an additional structural translation layer.Aggregationdefaults that are valid for downstream aggregate transforms.counter_expiry_secondsfield so the Datadog-schema value remains authoritative.Change Type
How did you test this PR?
make build-schema-overlaymake fmtmake check-docsmake check-allmake testAll checks passed; 1,801 tests passed and 31 were skipped.
References
This is the first PR in the stacked configuration-system changes. The environment-variable and runtime-integration changes build on this branch.