Skip to content

Commit

Permalink
fix(new_relic sink): Do not quote paths containing periods for the ev…
Browse files Browse the repository at this point in the history
…ent 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.
  • Loading branch information
bruceg committed Sep 19, 2024
1 parent 8238e5a commit 9b67aa9
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 24 deletions.
3 changes: 3 additions & 0 deletions changelog.d/new-relic-event-path-quoting.fix.md
Original file line number Diff line number Diff line change
@@ -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.
21 changes: 17 additions & 4 deletions lib/vector-core/src/event/log_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl LogEvent {
pub fn all_event_fields(
&self,
) -> Option<impl Iterator<Item = (KeyString, &Value)> + 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.
Expand All @@ -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<Item = (KeyString, &Value)> + 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<Item = (KeyString, &Value)> + Serialize {
if let Some(map) = self.as_map() {
util::log::all_fields(map, false)
} else {
util::log::all_fields_non_object_root(self.value())
}
Expand Down
35 changes: 17 additions & 18 deletions lib/vector-core/src/event/util/log/all_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ 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)
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
Expand Down Expand Up @@ -57,25 +57,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_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<PathPrefix>,
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,
}
}

Expand All @@ -87,6 +84,7 @@ impl<'a> FieldsIter<'a> {
stack: vec![LeafIter::Root((value, false))],
path: vec![],
skip_array_elements: false,
quote_periods: false,
}
}

Expand All @@ -96,6 +94,7 @@ impl<'a> FieldsIter<'a> {
stack: vec![LeafIter::Map(fields.iter())],
path: vec![],
skip_array_elements: true,
quote_periods: false,
}
}

Expand Down Expand Up @@ -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)) => {
Expand Down
2 changes: 1 addition & 1 deletion lib/vector-core/src/event/util/log/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = KeyString> + '_ {
all_fields(fields).map(|(k, _)| k)
all_fields(fields, true).map(|(k, _)| k)
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion src/sinks/new_relic/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
21 changes: 21 additions & 0 deletions src/sinks/new_relic/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"})));
Expand Down

0 comments on commit 9b67aa9

Please sign in to comment.