diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index 659afb3788266..4fd1cd54e7120 100644 --- a/docs/reference/remap/del.cue +++ b/docs/reference/remap/del.cue @@ -3,17 +3,18 @@ package metadata remap: functions: del: { arguments: [ { - name: "paths" - description: "The paths of the fields to delete." + name: "path" + description: "The path of the field to delete." required: true - multiple: true type: ["string"] }, ] - return: ["null"] + return: ["any"] category: "Event" description: #""" - Removed the fields specified by the given paths from the root `event` object. Multiple fields can be specified. + Removes the field specified by the given path from the event object. If the field exists, + the field's value is returned by the delete operation, while `null` is returned if the field + doesn't exist. """# examples: [ { @@ -21,13 +22,38 @@ remap: functions: del: { input: { "field1": 1 "field2": 2 - "field3": 3 + } + source: "del(.field1)" + output: { + "field2": 2 + } + }, + { + title: "Delete existing only" + input: { + "user_id": 1 } source: #""" - del(.field1, .field3) + .user.id = if exists(.user_id) { + del(.user_id) + } else { + "unknown" + } """# output: { - "field2": 2 + user: { + "id": 1 + } + } + }, + { + title: "Rename field" + input: { + old_field: "please rename me" + } + source: ".new_field = del(.old_field)" + output: { + new_field: "please rename me" } }, ] diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index d6688565391dd..7985da7c042b3 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -9,47 +9,58 @@ impl Function for Del { } fn parameters(&self) -> &'static [Parameter] { - generate_param_list! { - accepts = |_| true, - required = false, - keywords = [ - "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", - ], - } + &[Parameter { + keyword: "path", + accepts: |_| true, + required: true, + }] } fn compile(&self, mut arguments: ArgumentList) -> Result> { - let mut paths = vec![]; - paths.push(arguments.required_path("1")?); - - for i in 2..=16 { - if let Some(path) = arguments.optional_path(&format!("{}", i))? { - paths.push(path) - } - } + let path = arguments.required_path("path")?; - Ok(Box::new(DelFn { paths })) + Ok(Box::new(DelFn { path })) } } #[derive(Debug, Clone)] pub struct DelFn { - paths: Vec, + path: Path, +} + +impl DelFn { + #[cfg(test)] + fn new(path: Path) -> Self { + Self { path } + } } impl Expression for DelFn { fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result { - self.paths - .iter() - .try_for_each(|path| object.remove(path.as_ref(), false))?; - - Ok(Value::Null) + // TODO: we're silencing the result of the `remove` call here, to make + // this function infallible. + // + // This isn't correct though, since, while deleting Vector log fields is + // infallible, deleting metric fields is not. + // + // For example, if you try to delete `.name` in a metric event, the call + // returns an error, since this is an immutable field. + // + // After some debating, we've decided to _silently ignore_ deletions of + // immutable fields for now, but we'll circle back to this in the near + // future to potentially improve this situation. + // + // see tracking issue: https://github.com/timberio/vector/issues/5887 + Ok(object + .remove(self.path.as_ref(), false) + .ok() + .flatten() + .unwrap_or(Value::Null)) } fn type_def(&self, _: &state::Compiler) -> TypeDef { TypeDef { - fallible: true, - kind: value::Kind::Null, + kind: value::Kind::all(), ..Default::default() } } @@ -58,15 +69,65 @@ impl Expression for DelFn { #[cfg(test)] mod tests { use super::*; + use crate::map; + use std::str::FromStr; - test_type_def![static_type_def { - expr: |_| DelFn { - paths: vec![Path::from("foo")] - }, - def: TypeDef { - fallible: true, - kind: value::Kind::Null, - ..Default::default() - }, - }]; + #[test] + fn del() { + let cases = vec![ + ( + // String field exists + map!["exists": "value"], + Ok(value!("value")), + DelFn::new(Path::from("exists")), + ), + ( + // String field doesn't exist + map!["exists": "value"], + Ok(value!(null)), + DelFn::new(Path::from("does_not_exist")), + ), + ( + // Array field exists + map!["exists": value!([1, 2, 3])], + Ok(value!([1, 2, 3])), + DelFn::new(Path::from("exists")), + ), + ( + // Null field exists + map!["exists": value!(null)], + Ok(value!(null)), + DelFn::new(Path::from("exists")), + ), + ( + // Map field exists + map!["exists": map!["foo": "bar"]], + Ok(value!(map!["foo": "bar"])), + DelFn::new(Path::from("exists")), + ), + ( + // Integer field exists + map!["exists": 127], + Ok(value!(127)), + DelFn::new(Path::from("exists")), + ), + ( + // Array field exists + map!["exists": value!([1, 2, 3])], + Ok(value!(2)), + DelFn::new(remap::Path::from_str(".exists[1]").unwrap().into()), + ), + ]; + + let mut state = state::Program::default(); + + for (object, exp, func) in cases { + let mut object: Value = object.into(); + let got = func + .execute(&mut state, &mut object) + .map_err(|e| format!("{:#}", anyhow::anyhow!(e))); + + assert_eq!(got, exp); + } + } } diff --git a/lib/remap-functions/src/lib.rs b/lib/remap-functions/src/lib.rs index 1be21e128cdd6..5dda58c65f2a0 100644 --- a/lib/remap-functions/src/lib.rs +++ b/lib/remap-functions/src/lib.rs @@ -246,10 +246,6 @@ pub use uuid_v4::UuidV4; pub fn all() -> Vec> { vec![ - #[cfg(feature = "md5")] - Box::new(Md5), - #[cfg(feature = "sha1")] - Box::new(Sha1), #[cfg(feature = "assert")] Box::new(Assert), #[cfg(feature = "ceil")] @@ -294,6 +290,8 @@ pub fn all() -> Vec> { Box::new(IsNullish), #[cfg(feature = "log")] Box::new(Log), + #[cfg(feature = "md5")] + Box::new(Md5), #[cfg(feature = "merge")] Box::new(Merge), #[cfg(feature = "now")] @@ -330,6 +328,8 @@ pub fn all() -> Vec> { Box::new(Replace), #[cfg(feature = "round")] Box::new(Round), + #[cfg(feature = "sha1")] + Box::new(Sha1), #[cfg(feature = "sha2")] Box::new(Sha2), #[cfg(feature = "sha3")] diff --git a/lib/remap-functions/src/only_fields.rs b/lib/remap-functions/src/only_fields.rs index 8ddd59530e9d2..a29f6da74fff4 100644 --- a/lib/remap-functions/src/only_fields.rs +++ b/lib/remap-functions/src/only_fields.rs @@ -46,7 +46,7 @@ impl Expression for OnlyFieldsFn { .iter() .map(|path| (path, path.to_string())) .filter(|(_, key)| paths.iter().find(|p| key.starts_with(p.as_str())).is_none()) - .try_for_each(|(path, _)| object.remove(&path, true))?; + .try_for_each(|(path, _)| object.remove(&path, true).map(|_| ()))?; Ok(Value::Null) } diff --git a/lib/remap-lang/src/object.rs b/lib/remap-lang/src/object.rs index 069987ff2169c..193bd1a5c8350 100644 --- a/lib/remap-lang/src/object.rs +++ b/lib/remap-lang/src/object.rs @@ -55,5 +55,5 @@ pub trait Object: std::fmt::Debug { /// /// If `compact` is true, after deletion, if an empty object or array is /// left behind, it should be removed as well. - fn remove(&mut self, path: &Path, compact: bool) -> Result<(), String>; + fn remove(&mut self, path: &Path, compact: bool) -> Result, String>; } diff --git a/lib/remap-lang/src/value/object.rs b/lib/remap-lang/src/value/object.rs index f2985f15c1f0d..dea8e91ff44f1 100644 --- a/lib/remap-lang/src/value/object.rs +++ b/lib/remap-lang/src/value/object.rs @@ -14,9 +14,11 @@ impl Object for Value { self.paths().map_err(|err| err.to_string()) } - fn remove(&mut self, path: &Path, compact: bool) -> Result<(), String> { + fn remove(&mut self, path: &Path, compact: bool) -> Result, String> { + let value = self.get(path)?; self.remove_by_path(path, compact); - Ok(()) + + Ok(value) } } @@ -193,10 +195,18 @@ mod tests { #[test] fn object_remove() { let cases = vec![ + ( + value!({foo: "bar"}), + vec![Field(Regular("baz".to_owned()))], + false, + None, + Some(value!({foo: "bar"})), + ), ( value!({foo: "bar"}), vec![Field(Regular("foo".to_owned()))], false, + Some(value!("bar")), Some(value!({})), ), ( @@ -206,30 +216,35 @@ mod tests { Regular("foo".to_owned()), ])], false, + Some(value!("bar")), Some(value!({})), ), ( value!({foo: "bar", baz: "qux"}), vec![], false, + Some(value!({foo: "bar", baz: "qux"})), Some(value!({})), ), ( value!({foo: "bar", baz: "qux"}), vec![], true, + Some(value!({foo: "bar", baz: "qux"})), Some(value!({})), ), ( value!({foo: [0]}), vec![Field(Regular("foo".to_owned())), Index(0)], false, + Some(value!(0)), Some(value!({foo: []})), ), ( value!({foo: [0]}), vec![Field(Regular("foo".to_owned())), Index(0)], true, + Some(value!(0)), Some(value!({})), ), ( @@ -240,6 +255,7 @@ mod tests { Index(0), ], false, + Some(value!(0)), Some(value!({foo: {"bar baz": []}, bar: "baz"})), ), ( @@ -250,15 +266,16 @@ mod tests { Index(0), ], true, + Some(value!(0)), Some(value!({bar: "baz"})), ), ]; - for (mut object, segments, compact, expect) in cases { + for (mut object, segments, compact, value, expect) in cases { let path = Path::new_unchecked(segments); - assert_eq!(Object::remove(&mut object, &path, compact), Ok(())); - assert_eq!(Object::get(&object, &Path::root()), Ok(expect)) + assert_eq!(Object::remove(&mut object, &path, compact), Ok(value)); + assert_eq!(Object::get(&object, &Path::root()), Ok(expect)); } } diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 7e21bfffacda0..2e2b0fd509bfc 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -266,14 +266,16 @@ impl Object for LogEvent { Ok(value) } - fn remove(&mut self, path: &Path, compact: bool) -> Result<(), String> { + fn remove(&mut self, path: &Path, compact: bool) -> Result, String> { if path.is_root() { - for key in self.keys().collect::>() { - self.remove_prune(key, compact); - } - - return Ok(()); - }; + return Ok(Some( + std::mem::take(&mut self.fields) + .into_iter() + .map(|(key, value)| (key, value.into())) + .collect::>() + .into(), + )); + } // loop until we find a path that exists. for key in path.to_alternative_strings() { @@ -281,11 +283,10 @@ impl Object for LogEvent { continue; } - self.remove_prune(&key, compact); - break; + return Ok(self.remove_prune(&key, compact).map(Into::into)); } - Ok(()) + Ok(None) } fn insert(&mut self, path: &Path, value: remap::Value) -> Result<(), String> { @@ -658,8 +659,9 @@ mod test { for (object, segments, compact, expect) in cases { let mut event = LogEvent::from(object); let path = Path::new_unchecked(segments); + let removed = Object::get(&event, &path).unwrap(); - assert_eq!(Object::remove(&mut event, &path, compact), Ok(())); + assert_eq!(Object::remove(&mut event, &path, compact), Ok(removed)); assert_eq!(Object::get(&event, &Path::root()), Ok(expect)) } } diff --git a/src/event/metric.rs b/src/event/metric.rs index b02d995db2de7..db3798f88fae2 100644 --- a/src/event/metric.rs +++ b/src/event/metric.rs @@ -10,6 +10,7 @@ use std::{ use std::{ convert::TryFrom, fmt::{self, Display, Formatter}, + iter::FromIterator, }; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] @@ -320,9 +321,9 @@ impl Metric { .insert(name, value); } - /// Deletes the tag, if it exists. - pub fn delete_tag(&mut self, name: &str) { - self.tags.as_mut().and_then(|tags| tags.remove(name)); + /// Deletes the tag, if it exists, returns the old tag value. + pub fn delete_tag(&mut self, name: &str) -> Option { + self.tags.as_mut().and_then(|tags| tags.remove(name)) } } @@ -517,6 +518,12 @@ impl Object for Metric { Ok(self.timestamp.map(Into::into)) } [Segment::Field(kind)] if kind.as_str() == "kind" => Ok(Some(self.kind.clone().into())), + [Segment::Field(tags)] if tags.as_str() == "tags" => { + Ok(self.tags.as_ref().map(|map| { + let iter = map.iter().map(|(k, v)| (k.to_owned(), v.to_owned().into())); + remap::Value::from_iter(iter) + })) + } [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { Ok(self.tag_value(field.as_str()).map(|value| value.into())) } @@ -542,6 +549,7 @@ impl Object for Metric { result.push(Path::from_str("timestamp").expect("invalid path")); } if let Some(tags) = &self.tags { + result.push(Path::from_str("tags").expect("invalid path")); for name in tags.keys() { result.push(Path::from_str(&format!("tags.{}", name)).expect("invalid path")); } @@ -552,23 +560,28 @@ impl Object for Metric { Ok(result) } - fn remove(&mut self, path: &remap::Path, _compact: bool) -> Result<(), String> { + fn remove( + &mut self, + path: &remap::Path, + _compact: bool, + ) -> Result, String> { if path.is_root() { return Err(MetricPathError::SetPathError.to_string()); } match path.segments() { [Segment::Field(namespace)] if namespace.as_str() == "namespace" => { - self.namespace = None; - Ok(()) + Ok(self.namespace.take().map(Into::into)) } [Segment::Field(timestamp)] if timestamp.as_str() == "timestamp" => { - self.timestamp = None; - Ok(()) + Ok(self.timestamp.take().map(Into::into)) } + [Segment::Field(tags)] if tags.as_str() == "tags" => Ok(self.tags.take().map(|map| { + let iter = map.into_iter().map(|(k, v)| (k, v.into())); + remap::Value::from_iter(iter) + })), [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { - self.delete_tag(field.as_str()); - Ok(()) + Ok(self.delete_tag(field.as_str()).map(Into::into)) } _ => Err(MetricPathError::InvalidPath { path: &path.to_string(), @@ -969,12 +982,18 @@ mod test { }; assert_eq!( - Ok( - ["name", "namespace", "timestamp", "tags.tig", "kind", "type"] - .iter() - .map(|path| Path::from_str(path).expect("invalid path")) - .collect() - ), + Ok([ + "name", + "namespace", + "timestamp", + "tags", + "tags.tig", + "kind", + "type" + ] + .iter() + .map(|path| Path::from_str(path).expect("invalid path")) + .collect()), metric.paths() ); } @@ -1022,10 +1041,10 @@ mod test { assert_eq!(Ok(current), metric.get(&path)); assert_eq!(Ok(()), metric.insert(&path, new.clone())); - assert_eq!(Ok(Some(new)), metric.get(&path)); + assert_eq!(Ok(Some(new.clone())), metric.get(&path)); if delete { - assert_eq!(Ok(()), metric.remove(&path, true)); + assert_eq!(Ok(Some(new)), metric.remove(&path, true)); assert_eq!(Ok(None), metric.get(&path)); } }