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.17%
==========================================
Files 434 436 +2
Lines 70454 70847 +393
==========================================
+ Hits 50624 51027 +403
+ Misses 19830 19820 -10
🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 616217f | 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>
Bundles two adjustments to the parser-registry refactor.
Option A — polish:
- RemoteConfigParsedData becomes a marker trait with a blanket impl over
Any + Send + Sync + 'static; product crates no longer write as_any() or
manual impls.
- product() method dropped from the trait; RemoteConfigValue now stores
RemoteConfigProduct alongside the path components. Debug uses it.
- ParserRegistry::register returns Result<(), AlreadyRegistered> on
collision. No silent overwrites.
- IgnoredProduct sentinel removed. ParserRegistry::parse and
RegistryParser yield Ok(None) for unregistered products.
Option B — typed-config trait:
- New RemoteConfigContent { const PRODUCT; fn parse(&[u8]) }; product crates
implement this once instead of writing parser closures.
- ParserRegistry::with::<T>() builder method; default_registry() and the
sidecar use it (default_registry().with::<LiveDebuggingData>()).
- _parser() factory functions removed from datadog-live-debugger and
datadog-ffe.
- Free function downcast::<T>(parsed) replaces the as_any().downcast_ref
dance in tracer-flare and elsewhere.
- anyhow re-exported from datadog-remote-config so product crates can spell
the parse return type without their own anyhow dep (avoids new deps).
Co-Authored-By: Claude Opus 4.7 <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 typed
RemoteConfigContenttrait (with aPRODUCTconst and aparsemethod), an opaqueRemoteConfigParsedDatatrait-object boundary, and aParserRegistryto compose products. Product crates implementRemoteConfigContenton their own types; a blanketimpl<T: RemoteConfigContent> RemoteConfigParsedData for Tlets the registry hold them asBox<dyn RemoteConfigParsedData>without knowing the concrete type.Core changes
parse.rs:RemoteConfigDataenum;RemoteConfigContent(typed):const PRODUCT: RemoteConfigProduct+fn parse(&[u8]) -> anyhow::Result<Self>. This is the only trait product crates implement;RemoteConfigParsedData(opaque): blanket-implemented for everyRemoteConfigContent— product crates do not implement it directly;ParserRegistrywith builder-style.with::<T: RemoteConfigContent>()and a fail-fastregisterthat returnsAlreadyRegisteredon duplicate registration;parse()returnsanyhow::Result<Option<Box<dyn RemoteConfigParsedData>>>—Ok(None)when no parser is registered for the product (replaces the previousIgnoredProductsentinel);default_registry()chains.with::<AgentConfigFile>().with::<AgentTaskFile>().with::<DynamicConfigFile>().RemoteConfigValuecarriesproduct: RemoteConfigProductdirectly anddata: Option<Box<dyn RemoteConfigParsedData>>.file_storage.rs:ParseFileis instance-based with an associatedtype Parsed;RawBytesParserandRegistryParser(Arc<ParserRegistry>)are the concrete parsers.Cargo.toml: removedlive-debugger/ffefeatures and optional deps.datadog-live-debugger/datadog-ffe:impl RemoteConfigContent for LiveDebuggingData/impl RemoteConfigContent for UniversalFlagConfig. The previous parser-factory functions are gone.libdd-tracer-flare:impl TryFrom<&dyn RemoteConfigParsedData> for FlareActionwith downcast; call sites matchOk(Some(data))/Ok(None).datadog-sidecar:RemoteConfigManager::new()buildsdefault_registry().with::<LiveDebuggingData>()and readsvalue.productdirectly.Follow-up changes on this branch
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 registers theLiveDebuggingDataparser 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
In
datadog-remote-config(still required): add a variant toRemoteConfigProductand itsDisplay/try_parsearms inpath.rs. The product taxonomy is still a closed enum.In the product crate: implement
RemoteConfigContenton the product's config type:At construction time: chain
.with::<MyProductConfig>()onto aParserRegistry(typicallydefault_registry()).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.