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..a36addc0d1de3 100644 --- a/lib/vector-core/src/event/log_event.rs +++ b/lib/vector-core/src/event/log_event.rs @@ -434,7 +434,7 @@ impl LogEvent { pub fn all_event_fields( &self, ) -> Option + Serialize> { - self.as_map().map(all_fields) + self.as_map().map(|map| all_fields(map, true)) } /// Similar to [`LogEvent::all_event_fields`], but doesn't traverse individual array elements. @@ -455,11 +455,24 @@ 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) + util::log::all_fields(map, true) + } else { + util::log::all_fields_non_object_root(self.value()) + } + } + + /// 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(map, false) } else { util::log::all_fields_non_object_root(self.value()) } 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..163cf565b3bc1 100644 --- a/lib/vector-core/src/event/util/log/all_fields.rs +++ b/lib/vector-core/src/event/util/log/all_fields.rs @@ -10,14 +10,14 @@ 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) +pub fn all_fields(fields: &ObjectMap, quote_periods: bool) -> FieldsIter { + FieldsIter::new(None, fields, quote_periods) } /// 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 +57,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_periods: bool, } impl<'a> FieldsIter<'a> { - // TODO deprecate this in favor of `new_with_prefix`. - fn new(fields: &'a ObjectMap) -> FieldsIter<'a> { + fn new( + path_prefix: Option, + fields: &'a ObjectMap, + quote_periods: bool, + ) -> 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> { - FieldsIter { - path_prefix: Some(path_prefix), + path_prefix, stack: vec![LeafIter::Map(fields.iter())], path: vec![], skip_array_elements: false, + quote_periods, } } @@ -87,6 +84,7 @@ impl<'a> FieldsIter<'a> { stack: vec![LeafIter::Root((value, false))], path: vec![], skip_array_elements: false, + quote_periods: false, } } @@ -96,6 +94,7 @@ impl<'a> FieldsIter<'a> { stack: vec![LeafIter::Map(fields.iter())], path: vec![], skip_array_elements: true, + quote_periods: false, } } @@ -137,10 +136,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_periods && !IS_VALID_PATH_SEGMENT.is_match(key) { res.push_str(&format!("\"{key}\"")); + } else { + res.push_str(key); } } Some(PathComponent::Index(index)) => { diff --git a/lib/vector-core/src/event/util/log/keys.rs b/lib/vector-core/src/event/util/log/keys.rs index 9a5a41c36eaac..7ad56385bedda 100644 --- a/lib/vector-core/src/event/util/log/keys.rs +++ b/lib/vector-core/src/event/util/log/keys.rs @@ -5,7 +5,7 @@ use crate::event::{KeyString, ObjectMap}; /// It is implemented as a wrapper around `all_fields` to reduce code /// duplication. pub fn keys(fields: &ObjectMap) -> impl Iterator + '_ { - all_fields(fields).map(|(k, _)| k) + all_fields(fields, true).map(|(k, _)| k) } #[cfg(test)] 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"})));