feat(config): migrate DogStatsD source to translated config#1898
Open
webern wants to merge 7 commits into
Open
feat(config): migrate DogStatsD source to translated config#1898webern wants to merge 7 commits into
webern wants to merge 7 commits into
Conversation
dd-octo-sts
Bot
added
area/components
webern
changed the title
This was referenced Jun 22, 2026
Contributor
Author
|
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. |
1 task
This comment has been minimized.
This comment has been minimized.
Binary Size Analysis (Agent Data Plane)Baseline: 9c1abde · Comparison: 8bbad95 · 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 (35)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 ( |
webern
force-pushed
the
m/dsd-cutover
branch
2 times, most recently
from
55e3b2b to
f221f9b
Compare
1 task
webern
force-pushed
the
m/dsd-cutover
branch
3 times, most recently
from
0e03948 to
f741905
Compare
webern
force-pushed
the
m/scaffold
branch
2 times, most recently
from
5181d4e to
541cf25
Compare
1 task
Add the first end-to-end slice of the typed config system. DynamicConfig<T> and DogStatsDConfig land in saluki-component-config; a minimal SalukiConfiguration embeds DogStatsDConfig in agent-data-plane-config; and a Translator implements the generated witness in agent-data-plane-config-system, seeding the Saluki-only base then driving the Datadog source over it. Only DogStatsD-source keys have a native destination so far; the remaining witness methods are temporary no-ops until their destinations land. Trim the new crates' dependencies to what this slice actually uses.
Add ConfigurationSystem and ConfigurationSystemLoader with the consume-by-value load -> start transition wrapping the existing translator. start() parses the retained map as a DatadogConfiguration, translates once, and freezes the master SalukiConfiguration. raw_map() exposes the retained map for un-flipped components; saluki() returns an owned clone of the master. A temporary bridge carries DogStatsD Saluki-only knobs from their DD_* keys into the seed, preserving behavior until a dedicated source for those keys is wired. No production caller yet.
Build the configuration system from the final configuration and make it the single source. The top-level config binding becomes the system's retained map; create_topology and the DogStatsD topology helper now take the system and read that map. The DogStatsD source is not yet moved onto typed config, so behavior is unchanged; this only introduces the runtime plumbing.
Add DogStatsDConfiguration::new, which builds the source from its component-native config and re-applies the capture-queue depth floor. The topology helper constructs the source from the translated typed config instead of parsing the raw map. The raw-map constructor is retained, unused, until the remaining callers move over.
Add a privileged API worker that serializes the translated native configuration to JSON on /config/internal. It holds the shared configuration system and reads the translated master per request, so it reflects later re-translation without change. The body is served unscrubbed, like the existing /config route.
Add a Panoramic integration case that sets DogStatsD source config via DD_* env vars, starts ADP, and asserts the values appear on /config/internal. It checks a translated field and a seeded field, proving the source-to-native translation runs end-to-end.
webern
force-pushed
the
m/dsd-cutover
branch
2 times, most recently
from
c65c6cf to
c7a7928
Compare
Add a generated config smoke test in the agent-data-plane-config crate that mostly replicates the functionality of config_registry smoke tests (which have to be deleted one-by-one). Each value that is declared in the overlay as MIGRATED_TO_CONFIG_SYSTEM will run through a config system smoke test that asserts setting the key to a non-default value results in a non-default SalukiConfiguration.
webern
changed the title
There was a problem hiding this comment.
Pull request overview
This PR wires the agent-data-plane to a new typed configuration translation system, and migrates the DogStatsD source component to be driven from the translated native SalukiConfiguration rather than direct deserialization from the raw config map. It also adds a privileged /config/internal endpoint to expose the translated native configuration for validation and debugging.
Changes:
- Introduces
ConfigurationSystem(load -> start) to translateGenericConfiguration -> DatadogConfiguration -> SalukiConfigurationonce at startup, while retainingraw_map()as an escape hatch for unmigrated components. - Refactors DogStatsD source config plumbing so
saluki-componentsconsumessaluki-component-config::dogstatsd::SourceConfig(plain data) and no longer deserializesDogStatsDConfiguration. - Adds generated translation smoke tests and a new integration test case validating
/config/internalshows end-to-end translated DogStatsD source values.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/cases/adp-config-internal-dogstatsd/config.yaml | New integration case asserting translated DogStatsD source config is served via /config/internal. |
| Makefile | Extends build-schema-overlay to also build agent-data-plane-config-system (needed for codegen tests). |
| lib/saluki-components/src/sources/dogstatsd/resolver.rs | Updates resolver logic to read limits from config.source.*. |
| lib/saluki-components/src/sources/dogstatsd/origin.rs | Moves OriginEnrichmentConfiguration to saluki-component-config. |
| lib/saluki-components/src/sources/dogstatsd/mod.rs | Refactors DogStatsDConfiguration into a builder embedding SourceConfig; removes serde-based deserialization and old config smoke test. |
| lib/saluki-components/Cargo.toml | Adds dependency on saluki-component-config. |
| lib/saluki-component-config/src/dogstatsd.rs | Improves docs and clarifies the role of SourceConfig as the source slice. |
| lib/datadog-agent/config/src/classifier/mod.rs | Adds MIGRATED_TO_CONFIG_SYSTEM sentinel struct name for overlay/test classification. |
| lib/datadog-agent/config/schema/schema_overlay.yaml | Reattributes migrated DogStatsD source keys’ used_by to MigratedToConfigSystem. |
| lib/datadog-agent/config-testing/src/config_registry/dogstatsd.rs | Updates registry annotations to reference the migration sentinel instead of DogStatsDConfiguration. |
| lib/datadog-agent/config-testing/src/config_registry/aggregate.rs | Removes DogStatsD usage for a key that remains owned by AggregateConfiguration. |
| lib/datadog-agent/config-overlay-model/src/smoke_test_support.rs | Documents and adds the MigratedToConfigSystem enum variant and const mapping. |
| lib/datadog-agent/config-overlay-model/src/saluki_keys.rs | Updates Saluki-only keys’ used_by to point at the migration sentinel constant. |
| lib/agent-data-plane-config/src/dogstatsd.rs | Expands docs clarifying dogstatsd::Config is the domain umbrella and SourceConfig is the component slice. |
| lib/agent-data-plane-config-system/src/system.rs | New config-system lifecycle + DD->Saluki-only bridging for select DogStatsD knobs. |
| lib/agent-data-plane-config-system/src/smoke_test/mod.rs | New translation smoke-test harness for generated per-key tests. |
| lib/agent-data-plane-config-system/src/lib.rs | Exposes ConfigurationSystem publicly; wires smoke-test module under cfg(test). |
| lib/agent-data-plane-config-system/Cargo.toml | Adds runtime/build/dev deps for translation tests and codegen. |
| lib/agent-data-plane-config-system/build.rs | Generates one translation smoke test per overlay key marked MigratedToConfigSystem. |
| Cargo.lock | Adds new crate dependencies for config-system and component-config. |
| bin/agent-data-plane/src/internal/mod.rs | Threads ConfigurationSystem through internal supervisor creation. |
| bin/agent-data-plane/src/internal/control_plane.rs | Registers new InternalConfigWorker on the privileged API. |
| bin/agent-data-plane/src/internal/config_internal.rs | New privileged /config/internal route serving translated SalukiConfiguration. |
| bin/agent-data-plane/src/cli/run.rs | Makes config-system the single config loader; migrates DogStatsD source builder to typed SourceConfig. |
| bin/agent-data-plane/Cargo.toml | Adds dependency on agent-data-plane-config-system. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3
to
+7
| //! Each test checks a single key by setting a non default value in a `GenericConfiguration` map | ||
| //! then running that map through `DatadogConfiguration` deserialization and witness trait | ||
| //! translation. The resulting `SalukiConfiguration` is checked against default construction to | ||
| //! ensure that it's value has been perturbed. This ensures that none of the witness trait function | ||
| //! implementations are inert. |
Comment on lines
469
to
473
| let traffic_capture = TrafficCapture::with_workload_provider( | ||
| self.capture_path.clone(), | ||
| self.capture_depth.max(MIN_CAPTURE_DEPTH), | ||
| self.source.capture_path.clone(), | ||
| self.source.capture_depth.max(MIN_CAPTURE_DEPTH), | ||
| self.workload_provider.clone(), | ||
| ); |
Comment on lines
+96
to
+100
| let datadog: DatadogConfiguration = self | ||
| .config | ||
| .as_typed() | ||
| .error_context("Failed to parse Datadog configuration.")?; | ||
|
|
![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
Keep component-config docs source-neutral
The root AGENTS.md says generic lib/saluki-* crates should not reference the Datadog Agent by name; this new public Rustdoc in lib/saluki-component-config adds a Core Agent-specific migration detail to a crate whose own module docs describe source-agnostic component config. Move that source-language note to ADP/config-system docs or phrase it generically so the generic API surface stays source-neutral.
ℹ️ 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 dsd_config = DogStatsDConfiguration::from_configuration(config) | ||
| .error_context("Failed to configure DogStatsD source.")? | ||
| let dsd_config = DogStatsDConfiguration::new(system.saluki().components.dogstatsd.source) |
There was a problem hiding this comment.
In deployments that rely on run_path for the default DogStatsD capture directory, this new construction path skips the old from_configuration fixup that derived run_path/dsd_capture when dogstatsd_capture_path was empty. The translator now leaves the default as an empty PathBuf, and TrafficCapture rejects implicit captures with no configured path, so agent-data-plane dogstatsd capture without an explicit path regresses. Preserve the run_path fallback in the translated/new path.
Useful? React with 👍 / 👎.
Comment on lines
687
to
689
| test_support: | ||
| used_by: [DogStatsDConfiguration] | ||
| used_by: [MigratedToConfigSystem] | ||
| test_json: '["udp"]' |
There was a problem hiding this comment.
When users set dogstatsd_eol_required as a space-separated string, marking this key as migrated sends it through the generated Datadog model, whose field is a plain Vec<String>. The old source config explicitly used deserialize_space_separated_or_seq and had coverage for the string form, but the new smoke test here only exercises an array, so the legacy string form now fails deserialization before the source is built. Keep the string-or-list normalization in the config-system path.
Useful? React with 👍 / 👎.
Comment on lines
+124
to
+126
| if let Ok(Some(v)) = dd.try_get_typed::<bool>("dogstatsd_allow_context_heap_allocs") { | ||
| dsd.allow_context_heap_allocs = Some(v); | ||
| } |
There was a problem hiding this comment.
For these Saluki-only DD fallbacks, each branch matches only Ok(Some(_)); if a configured value has the wrong type or is out of range, try_get_typed returns Err and this code silently leaves the native field at its default. Before this migration, the source deserializer failed startup for invalid typed values such as an invalid dogstatsd_tcp_port, so this can make bad configuration look accepted while changing runtime behavior. Return or record the bridge error instead of dropping it.
Useful? React with 👍 / 👎.
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
This PR introduces the native saluki configuration type for the DogStatsD source component. Note the naming is quite confusing here:
DogStatsDConfiguration- is for the source component only. The rest of the pipeline has other config names.dogstatsd::SourceConfigin the new system, and that is aggregated bydogstatsd::Configwhich can be throught of as the whole DogStatsD pipeline.Just the DogStatsD source is modeled, at first, and wired through the new system in order to present a less overwhelming diff and establish the migration pattern.
The corresponding smoke test needs to be deleted because it is not compatible with the new system. However, a comparable test has been added to the new system under
smoke_testwhich provides similar guarantees. If we mark the field as supported andMIGRATED_TO_CONFIG_SYSTEMthen we will assert that setting that field produces a non-defaultSalukiConfigurationafter translation.AI Summary
Makes the configuration system the single config loader for the data plane and drives the DogStatsD
source from typed, translated configuration.
ConfigurationSystemwith aload->startlifecycle: it takes ownership of the finalconfiguration, translates the Datadog source into the native model once, and exposes both a typed
view (
saluki()) and the raw map (raw_map()) for components not yet on typed config.run.rsas the sole config loader. Every component other than the DogStatsD sourcereads the raw map.
SourceConfig, its slice of the typed native config.DogStatsDConfigurationis built byDogStatsDConfiguration::newfrom aSourceConfigplusruntime-injected handles (workload provider, capture and replay control). It implements no
Deserializeand is never built from the raw map./config/internalroute that serves the native configuration as JSON.target struct, which
DogStatsDConfigurationno longer does. The source's schema-overlay keys arere-attributed to a
MigratedToConfigSystemsentinel inused_by, which holds them out of everystruct's smoke test while satisfying the overlay's non-empty
used_byrule.agent-data-plane-config-systemgains abuild.rsthat generates onetranslation smoke test per
MigratedToConfigSystemkey. Each test injects a non-default valueinto
GenericConfiguration, runs the fullConfigurationSystempipeline (deserialize ->witness drive -> translation), and asserts the resulting
SalukiConfigurationdiffers from thedefault-translated baseline. Tests auto-grow as more keys migrate.
adp-config-internal-dogstatsd) that sets DogStatsD sourceconfig via
DD_*, starts the data plane, and asserts the translated values on/config/internal.Change Type
How did you test this PR?
build.rsinagent-data-plane-config-systemgenerates one#[tokio::test]perMigratedToConfigSystemoverlay key. Each test injects a non-default valueinto
GenericConfigurationand asserts translation produces aSalukiConfigurationthat differsfrom the default. Currently 34 generated tests covering all migrated DogStatsD source keys.
load->start->saluki()/raw_map()), translation correctness(field propagation, error surfacing, seed/drive disjointness, DD bridge for Saluki-only knobs).
adp-config-internal-dogstatsdsets DogStatsD source config viaDD_*env vars, starts ADP in Docker, and asserts the translated values appear on/config/internal. Runs in CI.make check-allandmake build-schema-overlayare clean locally.References