Skip to content

Commit 9041f1e

Browse files
webernwebern
authored
chore(config): migrate MrfConfiguration to typed config (#1985)
## AI Summary Build multi-region failover configuration from the typed `SalukiConfiguration` domain model and preserve endpoint normalization during translation. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted MRF and configuration-system unit tests ## References - Progresses #1788
1 parent ff1c107 commit 9041f1e

4 files changed

Lines changed: 114 additions & 109 deletions

File tree

‎bin/agent-data-plane/src/cli/run.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,17 +490,18 @@ async fn add_baseline_metrics_pipeline_to_blueprint(
490490
// Metrics, then forwarding.
491491
.connect_components_in_order(["metrics_enrich", "dd_metrics_encode", "dd_out"])?;
492492

493-
add_mrf_metrics_pipeline_to_blueprint(blueprint, config)?;
493+
add_mrf_metrics_pipeline_to_blueprint(blueprint, config, config_system)?;
494494
let saluki = config_system.config();
495495
add_autoscaling_failover_metrics_pipeline_to_blueprint(blueprint, config, &saluki.shared.autoscaling_failover)?;
496496

497497
Ok(())
498498
}
499499

500500
fn add_mrf_metrics_pipeline_to_blueprint(
501-
blueprint: &mut TopologyBlueprint, config: &GenericConfiguration,
501+
blueprint: &mut TopologyBlueprint, config: &GenericConfiguration, config_system: &ConfigurationSystem,
502502
) -> Result<(), GenericError> {
503-
let mrf_config = MrfConfiguration::from_configuration(config)
503+
let saluki = config_system.config();
504+
let mrf_config = MrfConfiguration::from_configuration(&saluki.domains.multi_region_failover)
504505
.error_context("Failed to configure Multi-Region Failover metrics pipeline.")?;
505506

506507
let Some((mrf_dd_url, mrf_api_key)) = mrf_config.metrics_endpoint_override() else {

‎lib/agent-data-plane-config-system/src/translators/datadog_translator.rs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -792,11 +792,11 @@ impl DatadogConfigWitness for DatadogTranslator<'_> {
792792
}
793793

794794
fn consume_multi_region_failover_api_key(&mut self, value: String) {
795-
self.config.domains.multi_region_failover.api_key = non_empty(value);
795+
self.config.domains.multi_region_failover.api_key = non_empty(value.trim().to_string());
796796
}
797797

798798
fn consume_multi_region_failover_dd_url(&mut self, value: String) {
799-
self.config.domains.multi_region_failover.dd_url = non_empty(value);
799+
self.config.domains.multi_region_failover.dd_url = non_empty(value.trim().to_string());
800800
}
801801

802802
fn consume_multi_region_failover_enabled(&mut self, value: bool) {
@@ -812,7 +812,7 @@ impl DatadogConfigWitness for DatadogTranslator<'_> {
812812
}
813813

814814
fn consume_multi_region_failover_site(&mut self, value: String) {
815-
self.config.domains.multi_region_failover.site = non_empty(value);
815+
self.config.domains.multi_region_failover.site = non_empty(value.trim().to_string());
816816
}
817817

818818
fn consume_no_proxy_nonexact_match(&mut self, value: bool) {
@@ -1085,6 +1085,11 @@ mod tests {
10851085
"dogstatsd_port": 9125,
10861086
"dogstatsd_tag_cardinality": "high",
10871087
"expected_tags_duration": "15s",
1088+
"multi_region_failover": {
1089+
"api_key": " mrf-key ",
1090+
"dd_url": " https://mrf.example.com ",
1091+
"site": " datadoghq.eu "
1092+
},
10881093
"telemetry": { "dogstatsd_origin": true },
10891094
}))
10901095
.expect("datadog source deserializes");
@@ -1118,6 +1123,15 @@ mod tests {
11181123
config.shared.endpoints.dd_url.as_deref(),
11191124
Some("https://custom.example.com")
11201125
);
1126+
assert_eq!(config.domains.multi_region_failover.api_key.as_deref(), Some("mrf-key"));
1127+
assert_eq!(
1128+
config.domains.multi_region_failover.dd_url.as_deref(),
1129+
Some("https://mrf.example.com")
1130+
);
1131+
assert_eq!(
1132+
config.domains.multi_region_failover.site.as_deref(),
1133+
Some("datadoghq.eu")
1134+
);
11211135
// Seeded Saluki-only field.
11221136
assert_eq!(config.domains.dogstatsd.listeners.tcp_port, 8126);
11231137
}

‎lib/saluki-components/src/config/mrf.rs‎

Lines changed: 57 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Multi-region failover configuration.
22
3-
use saluki_config::GenericConfiguration;
3+
use agent_data_plane_config::domains::multi_region_failover::Domain;
44
use saluki_error::GenericError;
55

66
const MRF_METRICS_ENDPOINT_PREFIX: &str = "https://app.mrf.";
@@ -17,19 +17,15 @@ pub struct MrfConfiguration {
1717
}
1818

1919
impl MrfConfiguration {
20-
/// Creates a new `MrfConfiguration` from the given configuration.
21-
pub fn from_configuration(config: &GenericConfiguration) -> Result<Self, GenericError> {
20+
/// Creates a new `MrfConfiguration` from the resolved multi-region failover configuration.
21+
pub fn from_configuration(config: &Domain) -> Result<Self, GenericError> {
2222
Ok(Self {
23-
enabled: config.try_get_typed("multi_region_failover.enabled")?.unwrap_or(false),
24-
failover_metrics: config
25-
.try_get_typed("multi_region_failover.failover_metrics")?
26-
.unwrap_or(false),
27-
metric_allowlist: config
28-
.try_get_typed("multi_region_failover.metric_allowlist")?
29-
.unwrap_or_default(),
30-
api_key: get_non_empty_string(config, "multi_region_failover.api_key")?,
31-
site: get_non_empty_string(config, "multi_region_failover.site")?,
32-
dd_url: get_non_empty_string(config, "multi_region_failover.dd_url")?,
23+
enabled: config.enabled,
24+
failover_metrics: config.failover_metrics,
25+
metric_allowlist: config.metric_allowlist.clone(),
26+
api_key: config.api_key.clone(),
27+
site: config.site.clone(),
28+
dd_url: config.dd_url.clone(),
3329
})
3430
}
3531

@@ -86,37 +82,24 @@ impl MrfConfiguration {
8682
}
8783
}
8884

89-
fn get_non_empty_string(config: &GenericConfiguration, key: &str) -> Result<Option<String>, GenericError> {
90-
Ok(config
91-
.try_get_typed::<String>(key)?
92-
.map(|value| value.trim().to_string())
93-
.filter(|value| !value.is_empty()))
94-
}
95-
9685
#[cfg(test)]
9786
mod tests {
98-
use saluki_config::ConfigurationLoader;
99-
use serde_json::json;
100-
10187
use super::*;
10288

103-
async fn mrf_config_from(value: serde_json::Value) -> MrfConfiguration {
104-
let (config, _) = ConfigurationLoader::for_tests(Some(value), None, false).await;
105-
MrfConfiguration::from_configuration(&config).expect("MRF configuration should deserialize")
89+
fn mrf_config_from(config: Domain) -> MrfConfiguration {
90+
MrfConfiguration::from_configuration(&config).expect("MRF configuration should build")
10691
}
10792

108-
#[tokio::test]
109-
async fn parses_mrf_configuration_keys() {
110-
let config = mrf_config_from(json!({
111-
"multi_region_failover": {
112-
"enabled": true,
113-
"failover_metrics": true,
114-
"metric_allowlist": ["first.metric", "second.metric"],
115-
"api_key": "mrf-api-key",
116-
"site": "datadoghq.eu"
117-
}
118-
}))
119-
.await;
93+
#[test]
94+
fn builds_from_model_configuration() {
95+
let config = mrf_config_from(Domain {
96+
enabled: true,
97+
failover_metrics: true,
98+
metric_allowlist: vec!["first.metric".to_string(), "second.metric".to_string()],
99+
api_key: Some("mrf-api-key".to_string()),
100+
site: Some("datadoghq.eu".to_string()),
101+
..Default::default()
102+
});
120103

121104
assert!(config.is_metrics_forwarding_requested());
122105
assert_eq!(config.metric_allowlist(), ["first.metric", "second.metric"]);
@@ -127,54 +110,45 @@ mod tests {
127110
);
128111
}
129112

130-
#[tokio::test]
131-
async fn metrics_endpoint_override_requires_api_key_and_endpoint() {
132-
let missing_api_key = mrf_config_from(json!({
133-
"multi_region_failover": {
134-
"enabled": true,
135-
"failover_metrics": true,
136-
"site": "datadoghq.eu"
137-
}
138-
}))
139-
.await;
113+
#[test]
114+
fn metrics_endpoint_override_requires_api_key_and_endpoint() {
115+
let missing_api_key = mrf_config_from(Domain {
116+
enabled: true,
117+
failover_metrics: true,
118+
site: Some("datadoghq.eu".to_string()),
119+
..Default::default()
120+
});
140121
assert_eq!(missing_api_key.metrics_endpoint_override(), None);
141122

142-
let missing_endpoint = mrf_config_from(json!({
143-
"multi_region_failover": {
144-
"enabled": true,
145-
"failover_metrics": true,
146-
"api_key": "mrf-api-key"
147-
}
148-
}))
149-
.await;
123+
let missing_endpoint = mrf_config_from(Domain {
124+
enabled: true,
125+
failover_metrics: true,
126+
api_key: Some("mrf-api-key".to_string()),
127+
..Default::default()
128+
});
150129
assert_eq!(missing_endpoint.metrics_endpoint_override(), None);
151130

152-
let ready = mrf_config_from(json!({
153-
"multi_region_failover": {
154-
"enabled": true,
155-
"failover_metrics": true,
156-
"api_key": "mrf-api-key",
157-
"dd_url": "https://mrf.example.com"
158-
}
159-
}))
160-
.await;
131+
let ready = mrf_config_from(Domain {
132+
enabled: true,
133+
failover_metrics: true,
134+
api_key: Some("mrf-api-key".to_string()),
135+
dd_url: Some("https://mrf.example.com".to_string()),
136+
..Default::default()
137+
});
161138
assert_eq!(
162139
ready.metrics_endpoint_override(),
163140
Some(("https://mrf.example.com".to_string(), "mrf-api-key".to_string()))
164141
);
165142
}
166143

167-
#[tokio::test]
168-
async fn metrics_endpoint_override_does_not_require_failover_metrics() {
169-
let config = mrf_config_from(json!({
170-
"multi_region_failover": {
171-
"enabled": true,
172-
"failover_metrics": false,
173-
"api_key": "mrf-api-key",
174-
"dd_url": "https://mrf.example.com"
175-
}
176-
}))
177-
.await;
144+
#[test]
145+
fn metrics_endpoint_override_does_not_require_failover_metrics() {
146+
let config = mrf_config_from(Domain {
147+
enabled: true,
148+
api_key: Some("mrf-api-key".to_string()),
149+
dd_url: Some("https://mrf.example.com".to_string()),
150+
..Default::default()
151+
});
178152

179153
assert!(!config.is_metrics_forwarding_requested());
180154
assert_eq!(
@@ -183,15 +157,13 @@ mod tests {
183157
);
184158
}
185159

186-
#[tokio::test]
187-
async fn dd_url_takes_precedence_over_site() {
188-
let config = mrf_config_from(json!({
189-
"multi_region_failover": {
190-
"site": "datadoghq.eu",
191-
"dd_url": "https://custom-mrf.example.com"
192-
}
193-
}))
194-
.await;
160+
#[test]
161+
fn dd_url_takes_precedence_over_site() {
162+
let config = mrf_config_from(Domain {
163+
site: Some("datadoghq.eu".to_string()),
164+
dd_url: Some("https://custom-mrf.example.com".to_string()),
165+
..Default::default()
166+
});
195167

196168
assert_eq!(
197169
config.metrics_endpoint_url().as_deref(),

‎lib/saluki-components/src/transforms/mrf_gateway/mod.rs‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,15 @@ impl Transform for MrfMetricsGateway {
217217

218218
#[cfg(test)]
219219
mod tests {
220+
use agent_data_plane_config::domains::multi_region_failover::Domain;
220221
use saluki_config::{dynamic::ConfigUpdate, ConfigurationLoader};
221222
use saluki_core::data_model::event::{metric::Metric, Event};
222223
use serde_json::json;
223224

224225
use super::*;
225226

226227
async fn dynamic_gateway_from_config(
227-
value: serde_json::Value,
228+
value: serde_json::Value, mrf_domain: Domain,
228229
) -> (MrfMetricsGateway, tokio::sync::mpsc::Sender<ConfigUpdate>) {
229230
let (config, sender) = ConfigurationLoader::for_tests(Some(value), None, true).await;
230231
let sender = sender.expect("dynamic sender should exist");
@@ -234,20 +235,28 @@ mod tests {
234235
.expect("initial dynamic snapshot should be sent");
235236
config.ready().await;
236237

237-
let mrf_config = MrfConfiguration::from_configuration(&config).expect("MRF configuration should deserialize");
238+
let mrf_config = MrfConfiguration::from_configuration(&mrf_domain).expect("MRF configuration should build");
238239
(MrfMetricsGateway::new(mrf_config, config), sender)
239240
}
240241

241242
#[tokio::test]
242243
async fn failover_metrics_dynamic_update_toggles_forwarding() {
243-
let (mut gw, sender) = dynamic_gateway_from_config(json!({
244-
"multi_region_failover": {
245-
"enabled": true,
246-
"failover_metrics": false,
247-
"api_key": "mrf-api-key",
248-
"dd_url": "https://mrf.example.com"
249-
}
250-
}))
244+
let (mut gw, sender) = dynamic_gateway_from_config(
245+
json!({
246+
"multi_region_failover": {
247+
"enabled": true,
248+
"failover_metrics": false,
249+
"api_key": "mrf-api-key",
250+
"dd_url": "https://mrf.example.com"
251+
}
252+
}),
253+
Domain {
254+
enabled: true,
255+
api_key: Some("mrf-api-key".to_string()),
256+
dd_url: Some("https://mrf.example.com".to_string()),
257+
..Default::default()
258+
},
259+
)
251260
.await;
252261
let mut watcher = gw
253262
.configuration
@@ -286,14 +295,23 @@ mod tests {
286295

287296
#[tokio::test]
288297
async fn metric_allowlist_dynamic_update_changes_filtering() {
289-
let (mut gw, sender) = dynamic_gateway_from_config(json!({
290-
"multi_region_failover": {
291-
"enabled": true,
292-
"failover_metrics": true,
293-
"api_key": "mrf-api-key",
294-
"dd_url": "https://mrf.example.com"
295-
}
296-
}))
298+
let (mut gw, sender) = dynamic_gateway_from_config(
299+
json!({
300+
"multi_region_failover": {
301+
"enabled": true,
302+
"failover_metrics": true,
303+
"api_key": "mrf-api-key",
304+
"dd_url": "https://mrf.example.com"
305+
}
306+
}),
307+
Domain {
308+
enabled: true,
309+
failover_metrics: true,
310+
api_key: Some("mrf-api-key".to_string()),
311+
dd_url: Some("https://mrf.example.com".to_string()),
312+
..Default::default()
313+
},
314+
)
297315
.await;
298316
let mut watcher = gw
299317
.configuration

0 commit comments

Comments
 (0)