refactor(remote-config)!: DI via RemoteConfigParsedData trait + ParserRegistry#1945
Draft
refactor(remote-config)!: DI via RemoteConfigParsedData trait + ParserRegistry#1945
Conversation
…Data trait + ParserRegistry Replace the closed `RemoteConfigData` enum with an extensible trait-object model so product crates register their own parsers and `datadog-remote-config` has zero compile-time dependencies on product crates. Changes: - `datadog-remote-config/src/parse.rs`: removed `RemoteConfigData` enum; added `RemoteConfigParsedData` trait, `ProductParser` type alias, `ParserRegistry` struct, and `default_registry()` (pre-loads AgentConfig, AgentTask, ApmTracing parsers). Internal config types implement the trait. - `datadog-remote-config/src/file_storage.rs`: `ParseFile` is now instance- based with `type Parsed`; `RawBytesParser` and `RegistryParser` concrete types; `ParsedFileStorage` uses `RegistryParser(Arc<ParserRegistry>)`. - `datadog-remote-config/Cargo.toml`: removed `live-debugger` and `ffe` Cargo features and their optional deps. - `datadog-remote-config/src/path.rs`: `LiveDebugger` variant made unconditional (feature-gate removed together with the feature). - `datadog-remote-config/src/config/dynamic.rs`: added `Clone` to `DynamicConfigFile`, `DynamicConfigTarget`, `DynamicConfig`. - `datadog-live-debugger`: new `src/remote_config.rs` implements `RemoteConfigParsedData` for `LiveDebuggingData` and exports `live_debugger_parser()`; added `datadog-remote-config` dep. - `datadog-ffe`: new `src/remote_config.rs` implements `RemoteConfigParsedData` for `UniversalFlagConfig` and exports `ffe_parser()`; added `datadog-remote-config` dep. - `libdd-tracer-flare`: migrated to downcast pattern; `impl TryFrom<&dyn RemoteConfigParsedData> for FlareAction`; `handle_remote_config_data` takes `&dyn RemoteConfigParsedData`. - `datadog-sidecar`: `RemoteConfigManager` holds `Arc<ParserRegistry>`; `read_config()` takes registry; removed `features = ["live-debugger"]` from remote-config dep; downcast replaces `RemoteConfigData` match. - `examples/remote_config_fetch.rs`: updated to registry-based API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ParserRegistry::parse was bailing on unregistered products, which differs from the old RemoteConfigData::Ignored(product) behavior. Consumers like RemoteConfigManager rely on getting a successful (Ignored) value to insert the config into active_configs, preventing repeated parse warnings on every fetch_update call. Restore the semantics by returning Ok(Box::new(IgnoredProduct(product))) instead of an error. IgnoredProduct implements RemoteConfigParsedData so callers can downcast to detect it if needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…igParsedData impls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📚 Documentation Check Results📦
|
Contributor
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
Contributor
🔒 Cargo Deny Results📦
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1945 +/- ##
==========================================
+ Coverage 71.85% 72.02% +0.16%
==========================================
Files 434 436 +2
Lines 70454 70839 +385
==========================================
+ Hits 50624 51021 +397
+ Misses 19830 19818 -12
🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: bb639a6 | Docs | Datadog PR Page | Give us feedback! |
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
…ChangesFetcher Move runtime-discovered extra services off `Target` (where they would force existing struct-literal callers to update and conflict with `Target`'s `Hash` / `Eq` / map-key role) onto the per-client mutable state owned by the fetcher. New API: - `ConfigClientState::set_extra_services(Vec<String>)` - `SingleFetcher::set_extra_services(Vec<String>)` - `SingleChangesFetcher::set_extra_services(Vec<String>)` `fetch_once` reads `extra_services` from `opaque_state` and forwards them on the wire via `ClientTracer.extra_services` exactly as before. Default is an empty vec, so the change is non-breaking for all existing callers. Replace-semantics: each set fully overrides the previous list. Test: `test_extra_services_forwarded_in_client_tracer` covers default empty state, forwarding after set, and replace semantics across three sequential polls. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
iunanua
commented
May 6, 2026
| } | ||
| } | ||
|
|
||
| pub fn ffe_parser() -> ProductParser { |
Contributor
Author
There was a problem hiding this comment.
no one uses this parser 🤷♂️
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…parsing Pre-refactor, datadog-sidecar enabled the `live-debugger` Cargo feature on `datadog-remote-config`, which made the closed-enum parser handle LiveDebugger payloads. After the dependency-inversion refactor that feature is gone — `RemoteConfigManager::new()` now defaulted to `default_registry()` (only AgentConfig / AgentTask / ApmTracing), so LiveDebugger configs flowing through the sidecar's manager were silently parsed as `IgnoredProduct`. Restore the old behavior by registering `live_debugger_parser()` explicitly in `RemoteConfigManager::new()`. Consumers building a custom registry can still call `new_with_registry` to opt out. Add `test_live_debugger_config_parsed`: a regression test that drives a `SERVICE_CONFIGURATION` LiveDebugger payload through the SHM round trip and asserts the downcast to `LiveDebuggingData` succeeds (rules out silent fallback to `IgnoredProduct`). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
What does this PR do?
The
datadog-remote-configcrate previously owned the full deserialization pipeline for every product via a closedRemoteConfigDataenum. Adding a new product required editing the RC crate itself, importing product types, and adding enum variants — creating compile-time coupling todatadog-live-debuggeranddatadog-ffevia Cargo optional features.This PR flips the dependency: RC defines a
RemoteConfigParsedDatatrait and aParserRegistry; product crates implement the trait on their own types and export parser factories. RC stores parsed payloads as opaqueBox<dyn RemoteConfigParsedData>trait objects.Core changes
parse.rs: removedRemoteConfigDataenum; addedRemoteConfigParsedDatatrait,ProductParser,ParserRegistry,default_registry().file_storage.rs:ParseFileis instance-based with associatedtype Parsed;RawBytesParserandRegistryParser(Arc<ParserRegistry>)concrete parsers.Cargo.toml: removedlive-debugger/ffefeatures and optional deps.datadog-live-debugger: newsrc/remote_config.rswithimpl RemoteConfigParsedData for LiveDebuggingData+live_debugger_parser().datadog-ffe: newsrc/remote_config.rswithimpl RemoteConfigParsedData for UniversalFlagConfig+ffe_parser().libdd-tracer-flare:impl TryFrom<&dyn RemoteConfigParsedData> for FlareAction; downcast replaces enum match.datadog-sidecar:RemoteConfigManagerholdsArc<ParserRegistry>; removedfeatures = ["live-debugger"]from the remote-config dep.Follow-up changes on this branch
IgnoredProductsentinel.ParserRegistry::parsepreviously bailed when no parser was registered, which differed from the oldRemoteConfigData::Ignored(product)behavior. Sidecar'sRemoteConfigManagerrelied on theOk(Ignored)form to track unknown configs inactive_configsand avoid repeated parse warnings on every poll. Restored by returningOk(Box::new(IgnoredProduct(product)))instead of an error;IgnoredProductimplementsRemoteConfigParsedDataso callers can downcast to detect it.datadog-sidecarenabled thelive-debuggerCargo feature ondatadog-remote-config, which made the closed-enum parser handle LiveDebugger payloads. After the DI refactor that feature is gone, soRemoteConfigManager::new()now explicitly registerslive_debugger_parser()on top ofdefault_registry()to preserve pre-refactor sidecar behavior. Added a regression test (test_live_debugger_config_parsed) that drives aSERVICE_CONFIGURATIONpayload through the SHM round trip and asserts the downcast toLiveDebuggingDatasucceeds.set_extra_serviceson the fetchers. Runtime-discovered extra services live onConfigClientState(the per-client mutable state), not onTarget. New API:ConfigClientState::set_extra_services(Vec<String>),SingleFetcher::set_extra_services,SingleChangesFetcher::set_extra_services.fetch_oncereads fromopaque_stateand forwards viaClientTracer.extra_servicesexactly as before. Replace-semantics: each set fully overrides the previous list. This was kept offTargetbecauseTargetisHash + Eq + Clone(used as a map key inMultiTargetFetcher) — putting growing dynamic state on it would force costly map rekeying or break theHash/Eqcontract.Adding a new RC product going forward
datadog-remote-config(still required): add a variant toRemoteConfigProductand itsDisplay/try_parsearms inpath.rs. The product taxonomy is still a closed enum.RemoteConfigParsedDataon the product's config type, export afn product_parser() -> ProductParserfactory.ParserRegistry::register(RemoteConfigProduct::YourProduct, product_parser()).Only the parsing is fully extensible after this PR; the product enumeration is still closed. Making
RemoteConfigProductopen (e.g.Cow<'static, str>or a sealed trait) would close that gap and is a reasonable follow-up.Motivation
Drives
datadog-remote-config's coupling to product crates from compile-time (Cargo features + closed enum) to runtime (registry injection), so consumers compose the products they care about without forcing the RC crate to know about every product.