Skip to content

Commit

Permalink
[library-config] Skip error on unknown config key (#868)
Browse files Browse the repository at this point in the history
# What does this PR do?

* Add custom deserializer for the configuration map to not error on unknown keys
  • Loading branch information
paullegranddc committed Feb 11, 2025
1 parent 52bd068 commit ff3c95e
Showing 1 changed file with 70 additions and 27 deletions.
97 changes: 70 additions & 27 deletions library-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,7 @@ impl<'a> Matcher<'a> {
}

/// Returns the first set of configurations that match the current process
fn find_stable_config<'b>(
&'a self,
cfg: &'b StableConfig,
) -> Option<&'b HashMap<LibraryConfigName, String>> {
fn find_stable_config<'b>(&'a self, cfg: &'b StableConfig) -> Option<&'b ConfigMap> {
for rule in &cfg.rules {
if rule.selectors.iter().all(|s| self.selector_match(s)) {
return Some(&rule.configuration);
Expand Down Expand Up @@ -248,6 +245,52 @@ impl ProcessInfo {
}
}

/// A (key, value) struct
///
/// This type has a custom serde Deserialize implementation from maps:
/// * It skips invalid/unknown keys in the map
/// * Since the storage is a Boxed slice and not a Hashmap, it doesn't over-allocate
#[derive(Debug, Default, PartialEq, Eq)]
struct ConfigMap(Box<[(LibraryConfigName, String)]>);

impl<'de> serde::Deserialize<'de> for ConfigMap {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
struct ConfigMapVisitor;
impl<'de> serde::de::Visitor<'de> for ConfigMapVisitor {
type Value = ConfigMap;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("struct ConfigMap(HashMap<LibraryConfig, String>)")
}

fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
where
A: serde::de::MapAccess<'de>,
{
let mut configs = Vec::new();
configs.reserve_exact(map.size_hint().unwrap_or(0));
loop {
let k = match map.next_key::<LibraryConfigName>() {
Ok(Some(k)) => k,
Ok(None) => break,
Err(_) => {
map.next_value::<serde::de::IgnoredAny>()?;
continue;
}
};
let v = map.next_value::<String>()?;
configs.push((k, v));
}
Ok(ConfigMap(configs.into_boxed_slice()))
}
}
deserializer.deserialize_map(ConfigMapVisitor)
}
}

#[repr(C)]
#[derive(Clone, Copy, serde::Deserialize, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
Expand Down Expand Up @@ -349,7 +392,7 @@ struct Selector {
#[derive(serde::Deserialize, Debug, PartialEq, Eq)]
struct Rule {
selectors: Vec<Selector>,
configuration: HashMap<LibraryConfigName, String>,
configuration: ConfigMap,
}

#[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)]
Expand All @@ -358,7 +401,7 @@ struct StableConfig {
#[serde(default)]
config_id: Option<String>,
#[serde(default)]
apm_configuration_default: HashMap<LibraryConfigName, String>,
apm_configuration_default: ConfigMap,

// Phase 2
#[serde(default)]
Expand Down Expand Up @@ -589,6 +632,9 @@ impl Configurator {
// Phase 1: take host default config
cfg.extend(
mem::take(&mut stable_config.apm_configuration_default)
.0
// TODO(paullgdc): use Box<[I]>::into_iter when we can use rust 1.80
.into_vec()
.into_iter()
.map(|(k, v)| {
(
Expand Down Expand Up @@ -623,7 +669,7 @@ impl Configurator {
return Ok(());
};

for (name, config_val) in configs {
for (name, config_val) in configs.0.iter() {
let value = matcher.template_config(config_val)?;
library_config.insert(
*name,
Expand Down Expand Up @@ -682,23 +728,10 @@ mod tests {

use super::{Configurator, ProcessInfo};
use crate::{
LibraryConfig, LibraryConfigName, LibraryConfigSource, Matcher, Operator, Origin, Rule,
Selector, StableConfig,
ConfigMap, LibraryConfig, LibraryConfigName, LibraryConfigSource, Matcher, Operator,
Origin, Rule, Selector, StableConfig,
};

macro_rules! map {
($(($key:expr , $value:expr)),* $(,)?) => {
{
#[allow(unused_mut)]
let mut map = std::collections::HashMap::new();
$(
map.insert($key, $value);
)*
map
}
};
}

fn test_config(local_cfg: &[u8], fleet_cfg: &[u8], expected: Vec<LibraryConfig>) {
let process_info: ProcessInfo = ProcessInfo {
args: vec![
Expand Down Expand Up @@ -842,8 +875,12 @@ apm_configuration_default:
DD_APPSEC_ENABLED: true
DD_IAST_ENABLED: true
DD_DYNAMIC_INSTRUMENTATION_ENABLED: true
# extra keys should be skipped without errors
FOO_BAR: quoicoubeh
DD_DATA_JOBS_ENABLED: true
DD_APPSEC_SCA_ENABLED: true
wtf:
- 1
",
vec![
LibraryConfig {
Expand Down Expand Up @@ -991,6 +1028,7 @@ rules:

#[test]
fn test_parse_static_config() {
use LibraryConfigName::*;
let mut tmp = tempfile::NamedTempFile::new().unwrap();
tmp.reopen()
.unwrap()
Expand All @@ -1004,6 +1042,8 @@ rules:
configuration:
DD_PROFILING_ENABLED: true
DD_SERVICE: my-service
# extra keys should be skipped without errors
FOOBAR: maybe??
",
)
.unwrap();
Expand All @@ -1015,7 +1055,7 @@ rules:
cfg,
StableConfig {
config_id: None,
apm_configuration_default: HashMap::default(),
apm_configuration_default: ConfigMap::default(),
tags: HashMap::default(),
rules: vec![Rule {
selectors: vec![Selector {
Expand All @@ -1025,10 +1065,13 @@ rules:
},
key: None,
}],
configuration: map![
(LibraryConfigName::DdProfilingEnabled, "true".to_owned()),
(LibraryConfigName::DdService, "my-service".to_owned())
],
configuration: ConfigMap(
vec![
(DdProfilingEnabled, "true".to_owned()),
(DdService, "my-service".to_owned())
]
.into_boxed_slice()
),
}]
}
)
Expand Down

0 comments on commit ff3c95e

Please sign in to comment.