diff --git a/changelog.d/new-relic-event-path-quoting.fix.md b/changelog.d/new-relic-event-path-quoting.fix.md new file mode 100644 index 0000000000000..4db6e8f97d920 --- /dev/null +++ b/changelog.d/new-relic-event-path-quoting.fix.md @@ -0,0 +1,3 @@ +The `new_relic` sink, when sending to the `event` API, would quote field names +containing periods or other meta-characters. This would produce broken field +names in the New Relic interface, and so that quoting has been removed. diff --git a/lib/vector-core/src/event/log_event.rs b/lib/vector-core/src/event/log_event.rs index 03b8908ff58b0..39a085c8b80a3 100644 --- a/lib/vector-core/src/event/log_event.rs +++ b/lib/vector-core/src/event/log_event.rs @@ -455,8 +455,9 @@ impl LogEvent { } } - /// Returns an iterator of all fields if the value is an Object. Otherwise, - /// a single field is returned with a "message" key + /// Returns an iterator of all fields if the value is an Object. Otherwise, a single field is + /// returned with a "message" key. Field names that are could be interpreted as alternate paths + /// (i.e. containing periods, square brackets, etc) are quoted. pub fn convert_to_fields(&self) -> impl Iterator + Serialize { if let Some(map) = self.as_map() { util::log::all_fields(map) @@ -465,6 +466,18 @@ impl LogEvent { } } + /// Returns an iterator of all fields if the value is an Object. Otherwise, a single field is + /// returned with a "message" key. Field names are not quoted. + pub fn convert_to_fields_unquoted( + &self, + ) -> impl Iterator + Serialize { + if let Some(map) = self.as_map() { + util::log::all_fields_unquoted(map) + } else { + util::log::all_fields_non_object_root(self.value()) + } + } + pub fn is_empty_object(&self) -> bool { if let Some(map) = self.as_map() { map.is_empty() diff --git a/lib/vector-core/src/event/util/log/all_fields.rs b/lib/vector-core/src/event/util/log/all_fields.rs index a8fa6f8e7c4cd..c237f65e30a82 100644 --- a/lib/vector-core/src/event/util/log/all_fields.rs +++ b/lib/vector-core/src/event/util/log/all_fields.rs @@ -11,13 +11,19 @@ static IS_VALID_PATH_SEGMENT: Lazy = Lazy::new(|| Regex::new(r"^[a-zA-Z0- /// Iterates over all paths in form `a.b[0].c[1]` in alphabetical order /// and their corresponding values. pub fn all_fields(fields: &ObjectMap) -> FieldsIter { - FieldsIter::new(fields) + FieldsIter::new(None, fields, true) +} + +/// Iterates over all paths in form `a.b[0].c[1]` in alphabetical order and their corresponding +/// values. Field names containing meta-characters are not quoted. +pub fn all_fields_unquoted(fields: &ObjectMap) -> FieldsIter { + FieldsIter::new(None, fields, false) } /// Same functionality as `all_fields` but it prepends a character that denotes the /// path type. pub fn all_metadata_fields(fields: &ObjectMap) -> FieldsIter { - FieldsIter::new_with_prefix(PathPrefix::Metadata, fields) + FieldsIter::new(Some(PathPrefix::Metadata), fields, true) } /// An iterator with a single "message" element @@ -57,25 +63,22 @@ pub struct FieldsIter<'a> { path: Vec>, /// Treat array as a single value and don't traverse each element. skip_array_elements: bool, + /// Add quoting to field names containing periods. + quote_meta: bool, } impl<'a> FieldsIter<'a> { - // TODO deprecate this in favor of `new_with_prefix`. - fn new(fields: &'a ObjectMap) -> FieldsIter<'a> { - FieldsIter { - path_prefix: None, - stack: vec![LeafIter::Map(fields.iter())], - path: vec![], - skip_array_elements: false, - } - } - - fn new_with_prefix(path_prefix: PathPrefix, fields: &'a ObjectMap) -> FieldsIter<'a> { + fn new( + path_prefix: Option, + fields: &'a ObjectMap, + quote_meta: bool, + ) -> FieldsIter<'a> { FieldsIter { - path_prefix: Some(path_prefix), + path_prefix, stack: vec![LeafIter::Map(fields.iter())], path: vec![], skip_array_elements: false, + quote_meta, } } @@ -87,6 +90,7 @@ impl<'a> FieldsIter<'a> { stack: vec![LeafIter::Root((value, false))], path: vec![], skip_array_elements: false, + quote_meta: false, } } @@ -96,6 +100,7 @@ impl<'a> FieldsIter<'a> { stack: vec![LeafIter::Map(fields.iter())], path: vec![], skip_array_elements: true, + quote_meta: false, } } @@ -137,10 +142,10 @@ impl<'a> FieldsIter<'a> { match path_iter.next() { None => break res.into(), Some(PathComponent::Key(key)) => { - if IS_VALID_PATH_SEGMENT.is_match(key) { - res.push_str(key); - } else { + if self.quote_meta && !IS_VALID_PATH_SEGMENT.is_match(key) { res.push_str(&format!("\"{key}\"")); + } else { + res.push_str(key); } } Some(PathComponent::Index(index)) => { @@ -226,15 +231,19 @@ mod test { assert_eq!(collected, expected); } + fn special_fields() -> ObjectMap { + fields_from_json(json!({ + "a-b": 1, + "a*b": 2, + "a b": 3, + ".a .b*": 4, + "\"a\"": 5, + })) + } + #[test] - fn keys_special() { - let fields = fields_from_json(json!({ - "a-b": 1, - "a*b": 2, - "a b": 3, - ".a .b*": 4, - "\"a\"": 5, - })); + fn keys_special_quoted() { + let fields = special_fields(); let mut collected: Vec<_> = all_fields(&fields).collect(); collected.sort_by(|(a, _), (b, _)| a.cmp(b)); @@ -254,6 +263,27 @@ mod test { assert_eq!(collected, expected); } + #[test] + fn keys_special_unquoted() { + let fields = special_fields(); + let mut collected: Vec<_> = all_fields_unquoted(&fields).collect(); + collected.sort_by(|(a, _), (b, _)| a.cmp(b)); + + let mut expected: Vec<(KeyString, &Value)> = vec![ + ("a-b", &Value::Integer(1)), + ("a*b", &Value::Integer(2)), + ("a b", &Value::Integer(3)), + (".a .b*", &Value::Integer(4)), + ("\"a\"", &Value::Integer(5)), + ] + .into_iter() + .map(|(k, v)| (k.into(), v)) + .collect(); + expected.sort_by(|(a, _), (b, _)| a.cmp(b)); + + assert_eq!(collected, expected); + } + #[test] fn metadata_keys_simple() { let fields = fields_from_json(json!({ @@ -274,22 +304,26 @@ mod test { assert_eq!(collected, expected); } + fn nested_fields() -> ObjectMap { + fields_from_json(json!({ + "a": { + "b": { + "c": 5 + }, + "a": 4, + "array": [null, 3, { + "x": 1 + }, [2]] + }, + "a.b.c": 6, + "d": {}, + "e": [], + })) + } + #[test] - fn keys_nested() { - let fields = fields_from_json(json!({ - "a": { - "b": { - "c": 5 - }, - "a": 4, - "array": [null, 3, { - "x": 1 - }, [2]] - }, - "a.b.c": 6, - "d": {}, - "e": [], - })); + fn keys_nested_quoted() { + let fields = nested_fields(); let expected: Vec<_> = vec![ ("a.a", Value::Integer(4)), ("a.array[0]", Value::Null), @@ -309,6 +343,30 @@ mod test { assert_eq!(collected, expected); } + #[test] + fn keys_nested_unquoted() { + let fields = nested_fields(); + let expected: Vec<_> = vec![ + ("a.a", Value::Integer(4)), + ("a.array[0]", Value::Null), + ("a.array[1]", Value::Integer(3)), + ("a.array[2].x", Value::Integer(1)), + ("a.array[3][0]", Value::Integer(2)), + ("a.b.c", Value::Integer(5)), + ("a.b.c", Value::Integer(6)), + ("d", Value::Object(ObjectMap::new())), + ("e", Value::Array(Vec::new())), + ] + .into_iter() + .map(|(k, v)| (k.into(), v)) + .collect(); + + let collected: Vec<_> = all_fields_unquoted(&fields) + .map(|(k, v)| (k, v.clone())) + .collect(); + assert_eq!(collected, expected); + } + #[test] fn test_non_object_root() { let value = Value::Integer(3); diff --git a/lib/vector-core/src/event/util/log/mod.rs b/lib/vector-core/src/event/util/log/mod.rs index cbb68c2da5218..b3454a7f1edac 100644 --- a/lib/vector-core/src/event/util/log/mod.rs +++ b/lib/vector-core/src/event/util/log/mod.rs @@ -2,7 +2,8 @@ mod all_fields; mod keys; pub use all_fields::{ - all_fields, all_fields_non_object_root, all_fields_skip_array_elements, all_metadata_fields, + all_fields, all_fields_non_object_root, all_fields_skip_array_elements, all_fields_unquoted, + all_metadata_fields, }; pub use keys::keys; diff --git a/src/sinks/new_relic/model.rs b/src/sinks/new_relic/model.rs index 0efc352449f2d..7a9649b2cedc1 100644 --- a/src/sinks/new_relic/model.rs +++ b/src/sinks/new_relic/model.rs @@ -169,7 +169,7 @@ impl TryFrom> for EventsApiModel { }; let mut event_model = ObjectMap::new(); - for (k, v) in log.convert_to_fields() { + for (k, v) in log.convert_to_fields_unquoted() { event_model.insert(k, v.clone()); } diff --git a/src/sinks/new_relic/tests.rs b/src/sinks/new_relic/tests.rs index 5fbd4f35fb54d..f68accfd7a251 100644 --- a/src/sinks/new_relic/tests.rs +++ b/src/sinks/new_relic/tests.rs @@ -134,6 +134,27 @@ fn generates_event_api_model_with_json_inside_message_field() { ); } +#[test] +fn generates_event_api_model_with_dotted_fields() { + let sub = value!({"two":"three"}); + let event = Event::Log(LogEvent::from(value!({ + "one.two": "Joe", + "eventType": "TestEvent", + "four": sub, + }))); + let model = + EventsApiModel::try_from(vec![event]).expect("Failed mapping events into API model"); + + assert_eq!( + to_value(&model).unwrap(), + json!([{ + "eventType": "TestEvent", + "one.two": "Joe", + "four.two": "three", + }]) + ); +} + #[test] fn generates_log_api_model_without_message_field() { let event = Event::Log(LogEvent::from(value!({"tag_key": "tag_value"})));