Skip to content

Commit c6b2735

Browse files
iunanuaclaude
andcommitted
refactor(remote-config)!: replace ParserRegistry with consumer-owned product enum
Removes the dynamic-dispatch RemoteConfigParsedData trait + ParserRegistry + Box<dyn>+downcast pattern in favour of a consumer-owned, statically-typed product enum (option C). - datadog-remote-config: drop RemoteConfigParsedData, ProductParser, ParserRegistry, IgnoredProduct, default_registry, RegistryParser; replace with a generic RemoteConfigValue<T> and a BuiltinProducts enum (+ BuiltinProductsParser ZST) covering AgentConfig/AgentTask/ApmTracing. - datadog-live-debugger: replace live_debugger_parser() with an inherent LiveDebuggingData::parse(&[u8]) method; drop the trait impl. - datadog-ffe: drop the empty remote_config module entirely; consumers call UniversalFlagConfig::from_json directly. - datadog-sidecar: define SidecarProducts enum + SidecarParser ZST covering the four products the sidecar actually consumes plus Other; drop the Arc<ParserRegistry> from RemoteConfigManager and drive read_config via a typed match on RemoteConfigProduct. - libdd-tracer-flare: reuse BuiltinProducts; replace the TryFrom<&dyn RemoteConfigParsedData> impl with a TryFrom<&BuiltinProducts> match; drop downcast_ref calls in the listener loop. - examples/remote_config_fetch.rs: switch to BuiltinProducts. No new dependencies. Zero Box<dyn ...> remote-config types and zero downcast_ref calls in consumer code. cargo check --workspace is clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent bb639a6 commit c6b2735

8 files changed

Lines changed: 158 additions & 264 deletions

File tree

datadog-ffe/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
mod flag_type;
55

6-
pub mod remote_config;
76
pub mod rules_based;
87

98
pub use flag_type::{ExpectedFlagType, FlagType};

datadog-ffe/src/remote_config.rs

Lines changed: 0 additions & 23 deletions
This file was deleted.

datadog-live-debugger/src/remote_config.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,9 @@
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;
75

8-
impl RemoteConfigParsedData for LiveDebuggingData {
9-
fn as_any(&self) -> &dyn Any {
10-
self
6+
impl LiveDebuggingData {
7+
pub fn parse(data: &[u8]) -> anyhow::Result<Self> {
8+
crate::parse_json::parse(&String::from_utf8_lossy(data))
119
}
12-
13-
fn product(&self) -> RemoteConfigProduct {
14-
RemoteConfigProduct::LiveDebugger
15-
}
16-
}
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-
})
2410
}

datadog-remote-config/examples/remote_config_fetch.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
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
use datadog_remote_config::RemoteConfigProduct::ApmTracing;
9-
use datadog_remote_config::{RemoteConfigParsedData, Target};
8+
use datadog_remote_config::{BuiltinProducts, Target};
109
use libdd_common::tag::Tag;
1110
use libdd_common::Endpoint;
1211
use std::time::Duration;
@@ -87,13 +86,11 @@ 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<BuiltinProducts>) {
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(products) => {
93+
println!("Products contents: {products:?}");
9794
}
9895
Err(e) => {
9996
println!("Failed parsing file: {e:?}");

datadog-remote-config/src/file_storage.rs

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
use crate::fetch::FileStorage;
55
use crate::file_change_tracker::{FilePath, UpdatedFiles};
6-
use crate::parse::{default_registry, ParserRegistry, RemoteConfigParsedData};
76
use crate::RemoteConfigPath;
87
use libdd_common::MutexExt;
98
use std::ops::Deref;
@@ -31,7 +30,8 @@ impl<P: ParseFile> RawFileStorage<P> {
3130
}
3231
}
3332

34-
/// Instance-based file parser. Implementations may carry state (e.g. an [`Arc<ParserRegistry>`]).
33+
/// Instance-based file parser. Implementations may carry state (e.g. configuration to drive a
34+
/// product-specific parsing decision).
3535
pub trait ParseFile: Clone + Send + Sync {
3636
/// The type of the parsed/stored content.
3737
type Parsed: Send;
@@ -134,32 +134,5 @@ impl ParseFile for RawBytesParser {
134134
/// Stores the remote config file contents in raw (unparsed) form.
135135
pub type SimpleFileStorage = RawFileStorage<RawBytesParser>;
136136

137-
// ── RegistryParser ────────────────────────────────────────────────────────────
138-
139-
/// A [`ParseFile`] implementation that dispatches to a [`ParserRegistry`].
140-
#[derive(Clone)]
141-
pub struct RegistryParser(pub Arc<ParserRegistry>);
142-
143-
impl ParseFile for RegistryParser {
144-
type Parsed = anyhow::Result<Box<dyn RemoteConfigParsedData>>;
145-
146-
fn parse(&self, path: &RemoteConfigPath, contents: Vec<u8>) -> Self::Parsed {
147-
self.0.parse(path.product, &contents)
148-
}
149-
}
150-
151-
/// Stores the remote config file contents in parsed form using a [`ParserRegistry`].
152-
pub type ParsedFileStorage = RawFileStorage<RegistryParser>;
153-
154-
impl ParsedFileStorage {
155-
/// Create a storage backed by the given registry.
156-
pub fn with_registry(registry: Arc<ParserRegistry>) -> Self {
157-
RawFileStorage::new(RegistryParser(registry))
158-
}
159-
}
160-
161-
impl Default for ParsedFileStorage {
162-
fn default() -> Self {
163-
Self::with_registry(Arc::new(default_registry()))
164-
}
165-
}
137+
/// Stores remote config file contents parsed as [`crate::BuiltinProducts`].
138+
pub type ParsedFileStorage = RawFileStorage<crate::parse::BuiltinProductsParser>;

datadog-remote-config/src/parse.rs

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

4+
#[cfg(feature = "client")]
5+
use crate::file_storage::ParseFile;
46
use crate::{
57
config::{
68
self, agent_config::AgentConfigFile, agent_task::AgentTaskFile, dynamic::DynamicConfigFile,
79
},
810
RemoteConfigPath, RemoteConfigProduct, RemoteConfigSource,
911
};
10-
use std::any::Any;
11-
use std::collections::HashMap;
1212

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 {
17-
fn as_any(&self) -> &dyn Any;
18-
fn product(&self) -> RemoteConfigProduct;
19-
}
20-
21-
/// A product-specific parser: converts raw bytes into a parsed payload.
22-
pub type ProductParser =
23-
Box<dyn Fn(&[u8]) -> anyhow::Result<Box<dyn RemoteConfigParsedData>> + Send + Sync>;
24-
25-
/// Maps [`RemoteConfigProduct`] variants to their parser functions.
13+
/// Parsed payload for the products owned by `datadog-remote-config`.
2614
///
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.
30-
pub struct ParserRegistry {
31-
parsers: HashMap<RemoteConfigProduct, ProductParser>,
15+
/// Consumers that only care about these products can use this enum directly via
16+
/// [`BuiltinProductsParser`]. Consumers that need additional products should define their own
17+
/// enum and [`ParseFile`] implementation.
18+
#[derive(Debug)]
19+
#[allow(clippy::large_enum_variant)]
20+
pub enum BuiltinProducts {
21+
AgentConfig(AgentConfigFile),
22+
AgentTask(AgentTaskFile),
23+
ApmTracing(DynamicConfigFile),
24+
Other(RemoteConfigProduct),
3225
}
3326

34-
impl ParserRegistry {
35-
pub fn new() -> Self {
36-
ParserRegistry {
37-
parsers: HashMap::new(),
27+
impl BuiltinProducts {
28+
pub fn product(&self) -> RemoteConfigProduct {
29+
match self {
30+
BuiltinProducts::AgentConfig(_) => RemoteConfigProduct::AgentConfig,
31+
BuiltinProducts::AgentTask(_) => RemoteConfigProduct::AgentTask,
32+
BuiltinProducts::ApmTracing(_) => RemoteConfigProduct::ApmTracing,
33+
BuiltinProducts::Other(p) => *p,
3834
}
3935
}
4036

41-
/// Register a parser for a product. Replaces any existing parser for that product.
42-
pub fn register(&mut self, product: RemoteConfigProduct, parser: ProductParser) {
43-
self.parsers.insert(product, parser);
44-
}
45-
46-
/// Parse `data` for `product`. Returns [`IgnoredProduct`] (not an error) when no parser is
47-
/// registered, so callers can still track the config in their bookkeeping structures.
48-
pub fn parse(
49-
&self,
50-
product: RemoteConfigProduct,
51-
data: &[u8],
52-
) -> anyhow::Result<Box<dyn RemoteConfigParsedData>> {
53-
match self.parsers.get(&product) {
54-
Some(parser) => parser(data),
55-
None => Ok(Box::new(IgnoredProduct(product))),
56-
}
57-
}
58-
}
59-
60-
impl Default for ParserRegistry {
61-
fn default() -> Self {
62-
Self::new()
63-
}
64-
}
65-
66-
// ── Implementations for RC-internal product types ────────────────────────────
67-
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);
71-
72-
impl RemoteConfigParsedData for IgnoredProduct {
73-
fn as_any(&self) -> &dyn Any {
74-
self
75-
}
76-
fn product(&self) -> RemoteConfigProduct {
77-
self.0
37+
pub fn try_parse(product: RemoteConfigProduct, data: &[u8]) -> anyhow::Result<BuiltinProducts> {
38+
Ok(match product {
39+
RemoteConfigProduct::AgentConfig => {
40+
BuiltinProducts::AgentConfig(config::agent_config::parse_json(data)?)
41+
}
42+
RemoteConfigProduct::AgentTask => {
43+
BuiltinProducts::AgentTask(config::agent_task::parse_json(data)?)
44+
}
45+
RemoteConfigProduct::ApmTracing => {
46+
BuiltinProducts::ApmTracing(config::dynamic::parse_json(data)?)
47+
}
48+
other => BuiltinProducts::Other(other),
49+
})
7850
}
7951
}
8052

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-
}
53+
/// [`ParseFile`] implementation for [`BuiltinProducts`]. Use this with [`RawFileStorage`] when
54+
/// no extra products beyond the RC-internal set need parsing.
55+
///
56+
/// [`RawFileStorage`]: crate::file_storage::RawFileStorage
57+
#[cfg(feature = "client")]
58+
#[derive(Clone, Default)]
59+
pub struct BuiltinProductsParser;
8960

90-
impl RemoteConfigParsedData for AgentConfigFile {
91-
fn as_any(&self) -> &dyn Any {
92-
self
93-
}
94-
fn product(&self) -> RemoteConfigProduct {
95-
RemoteConfigProduct::AgentConfig
96-
}
97-
}
61+
#[cfg(feature = "client")]
62+
impl ParseFile for BuiltinProductsParser {
63+
type Parsed = anyhow::Result<BuiltinProducts>;
9864

99-
impl RemoteConfigParsedData for AgentTaskFile {
100-
fn as_any(&self) -> &dyn Any {
101-
self
102-
}
103-
fn product(&self) -> RemoteConfigProduct {
104-
RemoteConfigProduct::AgentTask
65+
fn parse(&self, path: &RemoteConfigPath, contents: Vec<u8>) -> Self::Parsed {
66+
BuiltinProducts::try_parse(path.product, &contents)
10567
}
10668
}
10769

108-
/// Returns a registry pre-loaded with parsers for the RC-internal products:
109-
/// [`RemoteConfigProduct::AgentConfig`], [`RemoteConfigProduct::AgentTask`], and
110-
/// [`RemoteConfigProduct::ApmTracing`].
70+
/// A parsed remote config file along with metadata extracted from its path.
11171
///
112-
/// Consumers that need additional product parsers (live-debugger, FFE, …) should call
113-
/// [`ParserRegistry::register`] on the returned registry before use.
114-
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
138-
}
139-
140-
// ── RemoteConfigValue ─────────────────────────────────────────────────────────
141-
142-
pub struct RemoteConfigValue {
72+
/// `T` is the consumer-defined parsed-payload enum; for the built-in set, use
73+
/// [`BuiltinProducts`].
74+
pub struct RemoteConfigValue<T> {
14375
pub source: RemoteConfigSource,
144-
pub data: Box<dyn RemoteConfigParsedData>,
76+
pub data: T,
14577
pub config_id: String,
14678
pub name: String,
14779
}
14880

149-
impl std::fmt::Debug for RemoteConfigValue {
81+
impl<T: std::fmt::Debug> std::fmt::Debug for RemoteConfigValue<T> {
15082
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
15183
f.debug_struct("RemoteConfigValue")
15284
.field("source", &self.source)
153-
.field("product", &self.data.product())
85+
.field("data", &self.data)
15486
.field("config_id", &self.config_id)
15587
.field("name", &self.name)
15688
.finish()
15789
}
15890
}
15991

160-
impl RemoteConfigValue {
161-
pub fn try_parse(path: &str, data: &[u8], registry: &ParserRegistry) -> anyhow::Result<Self> {
92+
impl<T> RemoteConfigValue<T> {
93+
pub fn try_parse(
94+
path: &str,
95+
data: &[u8],
96+
parse: impl FnOnce(RemoteConfigProduct, &[u8]) -> anyhow::Result<T>,
97+
) -> anyhow::Result<Self> {
16298
let path = RemoteConfigPath::try_parse(path)?;
163-
let data = registry.parse(path.product, data)?;
99+
let parsed = parse(path.product, data)?;
164100
Ok(RemoteConfigValue {
165101
source: path.source,
166-
data,
102+
data: parsed,
167103
config_id: path.config_id.to_string(),
168104
name: path.name.to_string(),
169105
})

0 commit comments

Comments
 (0)