fix(new_relic sink): Do not quote paths containing periods for the ev…

…ent API (#21323)

* fix(new_relic sink): Do not quote paths containing periods for the event API

This could not be accomplished the same way as #21305 since the event API cannot
handle nested JSON data. Instead, a new `LogEvent::convert_to_fields_unquoted`
method was added to produce the flattened data without quoting.

* Fix and add tests

* Drop the extra parameter from `all_fields`

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.

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<Item = (KeyString, &Value)> + 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<Item = (KeyString, &Value)> + 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()

lib/vector-core/src/event/util/log/all_fields.rs

@@ -11,13 +11,19 @@ static IS_VALID_PATH_SEGMENT: Lazy<Regex> = 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<PathComponent<'a>>,
     /// 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<PathPrefix>,
+        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);

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;
 

src/sinks/new_relic/model.rs

@@ -169,7 +169,7 @@ impl TryFrom<Vec<Event>> 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());
                 }
 

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"})));