Skip to content

Commit fa2ec22

Browse files
iunanuaclaude
andcommitted
refactor(remote-config): polish DI API + typed RemoteConfigContent trait
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>
1 parent bb639a6 commit fa2ec22

10 files changed

Lines changed: 131 additions & 129 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datadog-ffe/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ publish = false
1111
bench = false
1212

1313
[dependencies]
14+
anyhow = { version = "1.0" }
1415
datadog-remote-config = { path = "../datadog-remote-config", default-features = false }
1516
faststr = { version = "0.2.23", default-features = false, features = ["serde"] }
1617
serde = { version = "1.0", default-features = false, features = ["derive", "rc"] }

datadog-ffe/src/remote_config.rs

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,12 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use crate::rules_based::UniversalFlagConfig;
5-
use datadog_remote_config::{ProductParser, RemoteConfigParsedData, RemoteConfigProduct};
6-
use std::any::Any;
5+
use datadog_remote_config::{RemoteConfigContent, RemoteConfigProduct};
76

8-
impl RemoteConfigParsedData for UniversalFlagConfig {
9-
fn as_any(&self) -> &dyn Any {
10-
self
11-
}
7+
impl RemoteConfigContent for UniversalFlagConfig {
8+
const PRODUCT: RemoteConfigProduct = RemoteConfigProduct::FfeFlags;
129

13-
fn product(&self) -> RemoteConfigProduct {
14-
RemoteConfigProduct::FfeFlags
10+
fn parse(data: &[u8]) -> anyhow::Result<Self> {
11+
Ok(UniversalFlagConfig::from_json(data.to_vec())?)
1512
}
1613
}
17-
18-
pub fn ffe_parser() -> ProductParser {
19-
Box::new(|data: &[u8]| {
20-
let parsed = UniversalFlagConfig::from_json(data.to_vec())?;
21-
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
22-
})
23-
}

datadog-live-debugger/src/remote_config.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,12 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use crate::probe_defs::LiveDebuggingData;
5-
use datadog_remote_config::{ProductParser, RemoteConfigParsedData, RemoteConfigProduct};
6-
use std::any::Any;
5+
use datadog_remote_config::{RemoteConfigContent, RemoteConfigProduct};
76

8-
impl RemoteConfigParsedData for LiveDebuggingData {
9-
fn as_any(&self) -> &dyn Any {
10-
self
11-
}
7+
impl RemoteConfigContent for LiveDebuggingData {
8+
const PRODUCT: RemoteConfigProduct = RemoteConfigProduct::LiveDebugger;
129

13-
fn product(&self) -> RemoteConfigProduct {
14-
RemoteConfigProduct::LiveDebugger
10+
fn parse(data: &[u8]) -> anyhow::Result<Self> {
11+
crate::parse_json::parse(&String::from_utf8_lossy(data))
1512
}
1613
}
17-
18-
pub fn live_debugger_parser() -> ProductParser {
19-
Box::new(|data: &[u8]| {
20-
let s = String::from_utf8_lossy(data);
21-
let parsed = crate::parse_json::parse(&s)?;
22-
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
23-
})
24-
}

datadog-remote-config/examples/remote_config_fetch.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use datadog_remote_config::config::dynamic::DynamicConfigFile;
54
use datadog_remote_config::fetch::{ConfigInvariants, ConfigOptions, SingleChangesFetcher};
65
use datadog_remote_config::file_change_tracker::{Change, FilePath};
76
use datadog_remote_config::file_storage::ParsedFileStorage;
@@ -87,13 +86,14 @@ async fn main() {
8786
}
8887
}
8988

90-
fn print_file_contents(contents: &anyhow::Result<Box<dyn RemoteConfigParsedData>>) {
89+
fn print_file_contents(contents: &anyhow::Result<Option<Box<dyn RemoteConfigParsedData>>>) {
90+
// Note: these contents may be large. Do not actually print it fully in a non-dev env.
9191
match contents {
92-
Ok(data) => {
93-
println!("Product: {:?}", data.product());
94-
if let Some(cfg) = data.as_any().downcast_ref::<DynamicConfigFile>() {
95-
println!("DynamicConfig action: {}", cfg.action);
96-
}
92+
Ok(Some(data)) => {
93+
println!("File contents: {data:?}");
94+
}
95+
Ok(None) => {
96+
println!("Unregistered product, no parsed data");
9797
}
9898
Err(e) => {
9999
println!("Failed parsing file: {e:?}");

datadog-remote-config/src/file_storage.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ pub type SimpleFileStorage = RawFileStorage<RawBytesParser>;
141141
pub struct RegistryParser(pub Arc<ParserRegistry>);
142142

143143
impl ParseFile for RegistryParser {
144-
type Parsed = anyhow::Result<Box<dyn RemoteConfigParsedData>>;
144+
type Parsed = anyhow::Result<Option<Box<dyn RemoteConfigParsedData>>>;
145145

146146
fn parse(&self, path: &RemoteConfigPath, contents: Vec<u8>) -> Self::Parsed {
147147
self.0.parse(path.product, &contents)

datadog-remote-config/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ mod targets;
2121

2222
pub use parse::*;
2323
pub use path::*;
24+
2425
use {
2526
libdd_common::tag::Tag,
2627
serde::{Deserialize, Serialize},

datadog-remote-config/src/parse.rs

Lines changed: 94 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,58 @@
33

44
use crate::{
55
config::{
6-
self, agent_config::AgentConfigFile, agent_task::AgentTaskFile, dynamic::DynamicConfigFile,
6+
agent_config::{self, AgentConfigFile},
7+
agent_task::{self, AgentTaskFile},
8+
dynamic::{self, DynamicConfigFile},
79
},
810
RemoteConfigPath, RemoteConfigProduct, RemoteConfigSource,
911
};
1012
use std::any::Any;
1113
use std::collections::HashMap;
14+
use std::fmt::{Debug, Display, Formatter, Result};
1215

13-
/// Opaque parsed payload for a remote config product. Product crates implement this on their own
14-
/// types and export a [`ProductParser`] factory; the RC crate stores and distributes the results
15-
/// without knowing their concrete type.
16-
pub trait RemoteConfigParsedData: Send + Sync + 'static {
16+
/// Opaque parsed payload of a remote config product. Implemented by every type that impls
17+
/// [`RemoteConfigContent`]; product crates do not implement this trait directly.
18+
///
19+
/// Use `parsed.as_any().downcast_ref::<T>()` to recover the concrete product type.
20+
pub trait RemoteConfigParsedData: Any + Debug + Send + Sync + 'static {
1721
fn as_any(&self) -> &dyn Any;
18-
fn product(&self) -> RemoteConfigProduct;
22+
}
23+
impl<T: RemoteConfigContent> RemoteConfigParsedData for T {
24+
fn as_any(&self) -> &dyn Any {
25+
self
26+
}
27+
}
28+
29+
/// Typed contract a product crate provides so the registry can build a parser for [`Self`].
30+
pub trait RemoteConfigContent: Any + Debug + Send + Sync + 'static {
31+
const PRODUCT: RemoteConfigProduct;
32+
fn parse(data: &[u8]) -> anyhow::Result<Self>
33+
where
34+
Self: Sized;
1935
}
2036

2137
/// A product-specific parser: converts raw bytes into a parsed payload.
2238
pub type ProductParser =
2339
Box<dyn Fn(&[u8]) -> anyhow::Result<Box<dyn RemoteConfigParsedData>> + Send + Sync>;
2440

41+
/// Returned by [`ParserRegistry::register`] when a parser is already registered for a product.
42+
#[derive(Debug)]
43+
pub struct AlreadyRegistered(pub RemoteConfigProduct);
44+
45+
impl Display for AlreadyRegistered {
46+
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
47+
write!(f, "parser already registered for product {}", self.0)
48+
}
49+
}
50+
51+
impl std::error::Error for AlreadyRegistered {}
52+
2553
/// Maps [`RemoteConfigProduct`] variants to their parser functions.
2654
///
27-
/// Consumers build a registry (optionally starting from [`default_registry`]) and inject it into
28-
/// the file storage or fetcher. Products with no registered parser return [`IgnoredProduct`]
29-
/// so callers can track the config without processing its contents.
55+
/// Build a registry (optionally starting from [`default_registry`]) and inject it into the file
56+
/// storage or fetcher. Products with no registered parser yield `Ok(None)` so callers can still
57+
/// track the config without processing its contents.
3058
pub struct ParserRegistry {
3159
parsers: HashMap<RemoteConfigProduct, ProductParser>,
3260
}
@@ -38,21 +66,45 @@ impl ParserRegistry {
3866
}
3967
}
4068

41-
/// Register a parser for a product. Replaces any existing parser for that product.
42-
pub fn register(&mut self, product: RemoteConfigProduct, parser: ProductParser) {
69+
/// Register `parser` for `product`. Errors if a parser is already registered for `product` —
70+
/// silent overwrites would mask configuration mistakes.
71+
pub fn register(
72+
&mut self,
73+
product: RemoteConfigProduct,
74+
parser: ProductParser,
75+
) -> anyhow::Result<(), AlreadyRegistered> {
76+
if self.parsers.contains_key(&product) {
77+
return Err(AlreadyRegistered(product));
78+
}
4379
self.parsers.insert(product, parser);
80+
Ok(())
81+
}
82+
83+
/// Builder-style registration of a typed [`RemoteConfigContent`] implementor.
84+
/// Panics if `T::PRODUCT` is already registered — intended for one-shot setup at the start of
85+
/// a process where a collision is a programmer error worth surfacing immediately.
86+
#[must_use]
87+
pub fn with<T: RemoteConfigContent>(mut self) -> Self {
88+
let parser: ProductParser = Box::new(|data: &[u8]| {
89+
let parsed = T::parse(data)?;
90+
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
91+
});
92+
#[allow(clippy::expect_used)]
93+
self.register(T::PRODUCT, parser)
94+
.expect("ParserRegistry::with: parser already registered");
95+
self
4496
}
4597

46-
/// Parse `data` for `product`. Returns [`IgnoredProduct`] (not an error) when no parser is
98+
/// Parse `data` for `product`. Returns `Ok(None)` (not an error) when no parser is
4799
/// registered, so callers can still track the config in their bookkeeping structures.
48100
pub fn parse(
49101
&self,
50102
product: RemoteConfigProduct,
51103
data: &[u8],
52-
) -> anyhow::Result<Box<dyn RemoteConfigParsedData>> {
104+
) -> anyhow::Result<Option<Box<dyn RemoteConfigParsedData>>> {
53105
match self.parsers.get(&product) {
54-
Some(parser) => parser(data),
55-
None => Ok(Box::new(IgnoredProduct(product))),
106+
Some(parser) => parser(data).map(Some),
107+
None => Ok(None),
56108
}
57109
}
58110
}
@@ -63,94 +115,58 @@ impl Default for ParserRegistry {
63115
}
64116
}
65117

66-
// ── Implementations for RC-internal product types ────────────────────────────
118+
// ── RemoteConfigContent impls for RC-internal product types ───────────────────
67119

68-
/// Sentinel returned by [`ParserRegistry::parse`] when no parser is registered for a product.
69-
/// Consumers that want to handle only specific products can downcast and ignore this type.
70-
pub struct IgnoredProduct(pub RemoteConfigProduct);
120+
impl RemoteConfigContent for AgentConfigFile {
121+
const PRODUCT: RemoteConfigProduct = RemoteConfigProduct::AgentConfig;
71122

72-
impl RemoteConfigParsedData for IgnoredProduct {
73-
fn as_any(&self) -> &dyn Any {
74-
self
75-
}
76-
fn product(&self) -> RemoteConfigProduct {
77-
self.0
123+
fn parse(data: &[u8]) -> anyhow::Result<Self> {
124+
Ok(agent_config::parse_json(data)?)
78125
}
79126
}
80127

81-
impl RemoteConfigParsedData for DynamicConfigFile {
82-
fn as_any(&self) -> &dyn Any {
83-
self
84-
}
85-
fn product(&self) -> RemoteConfigProduct {
86-
RemoteConfigProduct::ApmTracing
87-
}
88-
}
128+
impl RemoteConfigContent for AgentTaskFile {
129+
const PRODUCT: RemoteConfigProduct = RemoteConfigProduct::AgentTask;
89130

90-
impl RemoteConfigParsedData for AgentConfigFile {
91-
fn as_any(&self) -> &dyn Any {
92-
self
93-
}
94-
fn product(&self) -> RemoteConfigProduct {
95-
RemoteConfigProduct::AgentConfig
131+
fn parse(data: &[u8]) -> anyhow::Result<Self> {
132+
Ok(agent_task::parse_json(data)?)
96133
}
97134
}
98135

99-
impl RemoteConfigParsedData for AgentTaskFile {
100-
fn as_any(&self) -> &dyn Any {
101-
self
102-
}
103-
fn product(&self) -> RemoteConfigProduct {
104-
RemoteConfigProduct::AgentTask
136+
impl RemoteConfigContent for DynamicConfigFile {
137+
const PRODUCT: RemoteConfigProduct = RemoteConfigProduct::ApmTracing;
138+
139+
fn parse(data: &[u8]) -> anyhow::Result<Self> {
140+
Ok(dynamic::parse_json(data)?)
105141
}
106142
}
107143

108-
/// Returns a registry pre-loaded with parsers for the RC-internal products:
109-
/// [`RemoteConfigProduct::AgentConfig`], [`RemoteConfigProduct::AgentTask`], and
110-
/// [`RemoteConfigProduct::ApmTracing`].
144+
/// Returns a registry pre-loaded with parsers for the RC-internal products.
111145
///
112-
/// Consumers that need additional product parsers (live-debugger, FFE, …) should call
113-
/// [`ParserRegistry::register`] on the returned registry before use.
146+
/// Consumers that need additional product parsers (live-debugger, FFE, …) should chain
147+
/// [`ParserRegistry::with`] on the returned registry.
114148
pub fn default_registry() -> ParserRegistry {
115-
let mut registry = ParserRegistry::new();
116-
registry.register(
117-
RemoteConfigProduct::AgentConfig,
118-
Box::new(|data: &[u8]| {
119-
let parsed = config::agent_config::parse_json(data)?;
120-
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
121-
}),
122-
);
123-
registry.register(
124-
RemoteConfigProduct::AgentTask,
125-
Box::new(|data: &[u8]| {
126-
let parsed = config::agent_task::parse_json(data)?;
127-
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
128-
}),
129-
);
130-
registry.register(
131-
RemoteConfigProduct::ApmTracing,
132-
Box::new(|data: &[u8]| {
133-
let parsed = config::dynamic::parse_json(data)?;
134-
Ok(Box::new(parsed) as Box<dyn RemoteConfigParsedData>)
135-
}),
136-
);
137-
registry
149+
ParserRegistry::new()
150+
.with::<AgentConfigFile>()
151+
.with::<AgentTaskFile>()
152+
.with::<DynamicConfigFile>()
138153
}
139154

140155
// ── RemoteConfigValue ─────────────────────────────────────────────────────────
141156

142157
pub struct RemoteConfigValue {
143158
pub source: RemoteConfigSource,
144-
pub data: Box<dyn RemoteConfigParsedData>,
159+
pub product: RemoteConfigProduct,
160+
pub data: Option<Box<dyn RemoteConfigParsedData>>,
145161
pub config_id: String,
146162
pub name: String,
147163
}
148164

149-
impl std::fmt::Debug for RemoteConfigValue {
150-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
165+
impl Debug for RemoteConfigValue {
166+
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
151167
f.debug_struct("RemoteConfigValue")
152168
.field("source", &self.source)
153-
.field("product", &self.data.product())
169+
.field("product", &self.product)
154170
.field("config_id", &self.config_id)
155171
.field("name", &self.name)
156172
.finish()
@@ -163,6 +179,7 @@ impl RemoteConfigValue {
163179
let data = registry.parse(path.product, data)?;
164180
Ok(RemoteConfigValue {
165181
source: path.source,
182+
product: path.product,
166183
data,
167184
config_id: path.config_id.to_string(),
168185
name: path.name.to_string(),

0 commit comments

Comments
 (0)