From bca8dcbad33b5796e2fbe105454569f0bf0e2a61 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Sat, 19 Dec 2020 21:30:30 -0800 Subject: [PATCH 01/24] Make del function only take one arg Signed-off-by: Luc Perkins --- lib/remap-functions/src/del.rs | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index d6688565391dd..33062a3cd6d4d 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -9,39 +9,28 @@ 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: "field", + 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 field = arguments.required_path("field")?; - Ok(Box::new(DelFn { paths })) + Ok(Box::new(DelFn { field })) } } #[derive(Debug, Clone)] pub struct DelFn { - paths: Vec, + path: 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))?; + object.remove(self.path.as_ref(), false)?; Ok(Value::Null) } From ca18075ff022a7e5cb1968ad03d83586ab276efa Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 21 Dec 2020 07:37:49 -0800 Subject: [PATCH 02/24] Update function logic to use remove_and_get Signed-off-by: Luc Perkins --- lib/remap-functions/src/del.rs | 27 +++++++++++++------ lib/remap-lang/src/object.rs | 6 +++++ lib/remap-lang/src/value/object.rs | 43 +++++++++++++++++++++++++++++- 3 files changed, 67 insertions(+), 9 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 33062a3cd6d4d..01e8b3cdfd73f 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -25,20 +25,31 @@ impl Function for Del { #[derive(Debug, Clone)] pub struct DelFn { - path: Path, + field: Path, } impl Expression for DelFn { fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result { - object.remove(self.path.as_ref(), false)?; - - Ok(Value::Null) + match object.remove_and_get(self.field.as_ref(), false) { + Ok(Some(val)) => Ok(val), + _ => Ok(Value::Null), + } } fn type_def(&self, _: &state::Compiler) -> TypeDef { + use value::Kind; + TypeDef { - fallible: true, - kind: value::Kind::Null, + fallible: false, + kind: Kind::Bytes + | Kind::Integer + | Kind::Float + | Kind::Boolean + | Kind::Map + | Kind::Array + | Kind::Timestamp + | Kind::Regex + | Kind::Null, ..Default::default() } } @@ -50,10 +61,10 @@ mod tests { test_type_def![static_type_def { expr: |_| DelFn { - paths: vec![Path::from("foo")] + field: Path::from("foo"), }, def: TypeDef { - fallible: true, + fallible: false, kind: value::Kind::Null, ..Default::default() }, diff --git a/lib/remap-lang/src/object.rs b/lib/remap-lang/src/object.rs index 069987ff2169c..91c2c30ed0a86 100644 --- a/lib/remap-lang/src/object.rs +++ b/lib/remap-lang/src/object.rs @@ -56,4 +56,10 @@ 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>; + + /// Remove the given path from the object and return the value if it exists. + /// + /// If `compact` is true, after deletion, if an empty object or array is + /// left behind, it should be removed as well. + fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, ()>; } diff --git a/lib/remap-lang/src/value/object.rs b/lib/remap-lang/src/value/object.rs index 39c2f3fc2e08a..366178c724529 100644 --- a/lib/remap-lang/src/value/object.rs +++ b/lib/remap-lang/src/value/object.rs @@ -18,6 +18,16 @@ impl Object for Value { self.remove_by_path(path, compact); Ok(()) } + + fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, ()> { + match self.get(path) { + Ok(Some(val)) => { + self.remove_by_path(path, compact); + Ok(Some(val)) + } + _ => Ok(None), + } + } } #[cfg(test)] @@ -248,7 +258,38 @@ mod tests { 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::get(&object, &Path::root()), Ok(expect)); + } + } + + #[test] + fn object_remove_and_get() { + let cases = vec![ + ( + value![{foo: "bar"}], + vec![Field(Regular("foo".to_owned()))], + true, + Some(value!("bar")), + Some(value!({})), + ), + ( + value![{foo: "bar", boop: "bop"}], + vec![Field(Regular("boop".to_owned()))], + true, + Some(value!("bop")), + Some(value!({foo: "bar"})), + ), + ]; + + for (mut object, segments, compact, expect, end_result) in cases { + let path = Path::new_unchecked(segments); + + assert_eq!( + Object::remove_and_get(&mut object, &path, compact), + Ok(expect) + ); + assert_eq!(Object::get(&object, &path), Ok(None)); + assert_eq!(Object::get(&object, &Path::root()), Ok(end_result)); } } From 7c4ec319db2453c482b3c94879ed72edecd8a941 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 12:30:15 -0800 Subject: [PATCH 03/24] Add append function Signed-off-by: Luc Perkins --- lib/remap-functions/Cargo.toml | 2 + lib/remap-functions/src/append.rs | 103 ++++++++++++++++++++++++++++++ lib/remap-functions/src/lib.rs | 22 ++++--- 3 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 lib/remap-functions/src/append.rs diff --git a/lib/remap-functions/Cargo.toml b/lib/remap-functions/Cargo.toml index d207e897c39eb..00ac38c0b7678 100644 --- a/lib/remap-functions/Cargo.toml +++ b/lib/remap-functions/Cargo.toml @@ -35,6 +35,7 @@ anyhow = "1" [features] default = [ + "append", "assert", "ceil", "compact", @@ -92,6 +93,7 @@ default = [ "uuid_v4", ] +append = [] assert = [] ceil = [] compact = [] diff --git a/lib/remap-functions/src/append.rs b/lib/remap-functions/src/append.rs new file mode 100644 index 0000000000000..1e9a5e47e52d3 --- /dev/null +++ b/lib/remap-functions/src/append.rs @@ -0,0 +1,103 @@ +use remap::prelude::*; + +#[derive(Clone, Copy, Debug)] +pub struct Append; + +impl Function for Append { + fn identifier(&self) -> &'static str { + "append" + } + + fn parameters(&self) -> &'static [Parameter] { + &[ + Parameter { + keyword: "value", + accepts: |v| matches!(v, Value::Array(_)), + required: true, + }, + Parameter { + keyword: "item", + accepts: |_| true, + required: true, + }, + ] + } + + fn compile(&self, mut arguments: ArgumentList) -> Result> { + let value = arguments.required("value")?.boxed(); + let item = arguments.required("item")?.boxed(); + + Ok(Box::new(AppendFn { value, item })) + } +} + +#[derive(Debug, Clone)] +struct AppendFn { + value: Box, + item: Box, +} + +impl Expression for AppendFn { + fn execute(&self, state: &mut state::Program, object: &mut dyn Object) -> Result { + let mut list = self.value.execute(state, object)?.try_array()?; + let item = self.item.execute(state, object)?; + + list.push(item); + + Ok(list.into()) + } + + fn type_def(&self, state: &state::Compiler) -> TypeDef { + use value::Kind; + + self.value + .type_def(state) + .fallible_unless(Kind::Array) + .merge(self.item.type_def(state)) + .with_constraint(Kind::Array) + .with_inner_type(None) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use value::Kind; + + test_type_def![ + value_array_infallible { + expr: |_| AppendFn { + value: Array::from(vec!["foo", "bar"]).boxed(), + item: Literal::from("baz").boxed(), + }, + def: TypeDef { kind: Kind::Array, ..Default::default() }, + } + + value_non_array_fallible { + expr: |_| AppendFn { + value: Literal::from(27).boxed(), + item: Literal::from("foo").boxed(), + }, + def: TypeDef { kind: Kind::Array, fallible: true, ..Default::default() }, + } + ]; + + test_function![ + append => Append; + + empty_array { + args: func_args![value: value!([]), item: value!("foo")], + want: Ok(value!(["foo"])), + } + + new_item { + args: func_args![value: value!([11, false, 42.5]), item: value!("foo")], + want: Ok(value!([11, false, 42.5, "foo"])), + } + + already_exists_item { + args: func_args![value: value!([11, false, 42.5]), item: value!(42.5)], + want: Ok(value!([11, false, 42.5, 42.5])), + } + ]; +} diff --git a/lib/remap-functions/src/lib.rs b/lib/remap-functions/src/lib.rs index 7879c037f4cb7..cb1ff2eab4234 100644 --- a/lib/remap-functions/src/lib.rs +++ b/lib/remap-functions/src/lib.rs @@ -1,5 +1,7 @@ mod util; +#[cfg(feature = "append")] +mod append; #[cfg(feature = "assert")] mod assert; #[cfg(feature = "ceil")] @@ -113,10 +115,8 @@ mod uuid_v4; // ----------------------------------------------------------------------------- -#[cfg(feature = "md5")] -pub use crate::md5::Md5; -#[cfg(feature = "sha1")] -pub use crate::sha1::Sha1; +#[cfg(feature = "append")] +pub use append::Append; #[cfg(feature = "assert")] pub use assert::Assert; #[cfg(feature = "ceil")] @@ -155,6 +155,8 @@ pub use ip_to_ipv6::IpToIpv6; pub use ipv6_to_ipv4::Ipv6ToIpV4; #[cfg(feature = "log")] pub use log::Log; +#[cfg(feature = "md5")] +pub use crate::md5::Md5; #[cfg(feature = "merge")] pub use merge::Merge; #[cfg(feature = "now")] @@ -187,6 +189,8 @@ pub use redact::Redact; pub use replace::Replace; #[cfg(feature = "round")] pub use round::Round; +#[cfg(feature = "sha1")] +pub use crate::sha1::Sha1; #[cfg(feature = "sha2")] pub use sha2::Sha2; #[cfg(feature = "sha3")] @@ -226,10 +230,8 @@ pub use uuid_v4::UuidV4; pub fn all() -> Vec> { vec![ - #[cfg(feature = "md5")] - Box::new(Md5), - #[cfg(feature = "sha1")] - Box::new(Sha1), + #[cfg(feature = "append")] + Box::new(Append), #[cfg(feature = "assert")] Box::new(Assert), #[cfg(feature = "ceil")] @@ -268,6 +270,8 @@ pub fn all() -> Vec> { Box::new(Ipv6ToIpV4), #[cfg(feature = "log")] Box::new(Log), + #[cfg(feature = "md5")] + Box::new(Md5), #[cfg(feature = "merge")] Box::new(Merge), #[cfg(feature = "now")] @@ -300,6 +304,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")] From e69b2aacbd1221744f674455fbbafefed1342ec1 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 12:42:37 -0800 Subject: [PATCH 04/24] Add CUE docs sources for append function Signed-off-by: Luc Perkins --- docs/reference/remap.cue | 2 +- docs/reference/remap/append.cue | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 docs/reference/remap/append.cue diff --git a/docs/reference/remap.cue b/docs/reference/remap.cue index 1501cffab2369..5d9ee48667e59 100644 --- a/docs/reference/remap.cue +++ b/docs/reference/remap.cue @@ -49,7 +49,7 @@ remap: { arguments: [...#Argument] // Allow for empty list return: [#RemapReturnTypes, ...#RemapReturnTypes] - category: "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp" + category: "Array" | "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp" description: string examples?: [#RemapExample, ...#RemapExample] name: Name diff --git a/docs/reference/remap/append.cue b/docs/reference/remap/append.cue new file mode 100644 index 0000000000000..e2b830cec88e4 --- /dev/null +++ b/docs/reference/remap/append.cue @@ -0,0 +1,39 @@ +package metadata + +remap: functions: append: { + arguments: [ + { + name: "value" + description: "The array" + required: true + type: ["array"] + }, + { + name: "item" + description: "The item to append" + required: true + type: ["any"] + }, + ] + return: ["array"] + category: "Array" + description: """ + Appends the specified item to an array. The item is appended regardless + of what is currently in the array. + """ + examples: [ + { + title: "Mixed array" + input: { + kitchen_sink: [72.5, false, [1, 2, 3]] + item: "booper" + } + source: """ + .kitchen_sink = append(.kitchen_sink, .item) + """ + output: { + kitchen_sink: [72.5, false, [1, 2, 3], "booper"] + } + } + ] +} From b7d4aa060bbfc0c3695c0bfc53a14279f86bc18d Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 13:32:51 -0800 Subject: [PATCH 05/24] Add TOML unit test for append function Signed-off-by: Luc Perkins --- tests/behavior/transforms/remap.toml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/behavior/transforms/remap.toml b/tests/behavior/transforms/remap.toml index 7e4c9bd46f9ee..995adfc198b35 100644 --- a/tests/behavior/transforms/remap.toml +++ b/tests/behavior/transforms/remap.toml @@ -46,6 +46,25 @@ "b[1].equals" = "" "b[2].equals" = "two" +[transforms.append_to_array] + inputs = [] + type = "remap" + source = """ + .fruits = append(.fruits, .fruit) + """ +[[tests]] + name = "append_to_array" + [tests.input] + insert_at = "append_to_array" + type = "log" + [tests.input.log_fields] + fruits = ["apple", "orange", "banana"] + fruit = "mango" + [[tests.outputs]] + extract_from = "append_to_array" + [[tests.outputs.conditions]] + "fruits.equals" = ["apple", "orange", "banana", "mango"] + [transforms.remap_arithmetic] inputs = [] type = "remap" From 40b22f1f2824166a9e1f61527dc17dbbf22cf07c Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 13:37:15 -0800 Subject: [PATCH 06/24] Fix formatting Signed-off-by: Luc Perkins --- lib/remap-functions/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/remap-functions/src/lib.rs b/lib/remap-functions/src/lib.rs index cb1ff2eab4234..f7f9cbb264155 100644 --- a/lib/remap-functions/src/lib.rs +++ b/lib/remap-functions/src/lib.rs @@ -156,7 +156,7 @@ pub use ipv6_to_ipv4::Ipv6ToIpV4; #[cfg(feature = "log")] pub use log::Log; #[cfg(feature = "md5")] -pub use crate::md5::Md5; +pub use md5::Md5; #[cfg(feature = "merge")] pub use merge::Merge; #[cfg(feature = "now")] @@ -190,7 +190,7 @@ pub use replace::Replace; #[cfg(feature = "round")] pub use round::Round; #[cfg(feature = "sha1")] -pub use crate::sha1::Sha1; +pub use sha1::Sha1; #[cfg(feature = "sha2")] pub use sha2::Sha2; #[cfg(feature = "sha3")] From fb0c35e8a323374423c490ff749ec6e6429097f7 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 14:08:50 -0800 Subject: [PATCH 07/24] Fix CUE formatting Signed-off-by: Luc Perkins --- docs/reference/remap/append.cue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/remap/append.cue b/docs/reference/remap/append.cue index e2b830cec88e4..936efca2a815d 100644 --- a/docs/reference/remap/append.cue +++ b/docs/reference/remap/append.cue @@ -3,9 +3,9 @@ package metadata remap: functions: append: { arguments: [ { - name: "value" + name: "value" description: "The array" - required: true + required: true type: ["array"] }, { @@ -34,6 +34,6 @@ remap: functions: append: { output: { kitchen_sink: [72.5, false, [1, 2, 3], "booper"] } - } + }, ] } From 62957dc151592aa683126091f31452c1b320ac49 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 28 Dec 2020 19:49:23 -0800 Subject: [PATCH 08/24] Resolve md5/sha1 import ambiguity Signed-off-by: Luc Perkins --- lib/remap-functions/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/remap-functions/src/lib.rs b/lib/remap-functions/src/lib.rs index f7f9cbb264155..d4496014e1438 100644 --- a/lib/remap-functions/src/lib.rs +++ b/lib/remap-functions/src/lib.rs @@ -115,6 +115,10 @@ mod uuid_v4; // ----------------------------------------------------------------------------- +#[cfg(feature = "md5")] +pub use crate::md5::Md5; +#[cfg(feature = "sha1")] +pub use crate::sha1::Sha1; #[cfg(feature = "append")] pub use append::Append; #[cfg(feature = "assert")] @@ -155,8 +159,6 @@ pub use ip_to_ipv6::IpToIpv6; pub use ipv6_to_ipv4::Ipv6ToIpV4; #[cfg(feature = "log")] pub use log::Log; -#[cfg(feature = "md5")] -pub use md5::Md5; #[cfg(feature = "merge")] pub use merge::Merge; #[cfg(feature = "now")] @@ -189,8 +191,6 @@ pub use redact::Redact; pub use replace::Replace; #[cfg(feature = "round")] pub use round::Round; -#[cfg(feature = "sha1")] -pub use sha1::Sha1; #[cfg(feature = "sha2")] pub use sha2::Sha2; #[cfg(feature = "sha3")] From 7dc4407ddff138e6249ff77ded840fb9a9951d68 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 29 Dec 2020 11:58:13 -0800 Subject: [PATCH 09/24] Clarify append semantics in docs Signed-off-by: Luc Perkins --- docs/reference/remap/append.cue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/remap/append.cue b/docs/reference/remap/append.cue index 936efca2a815d..ee0b48a2db372 100644 --- a/docs/reference/remap/append.cue +++ b/docs/reference/remap/append.cue @@ -18,8 +18,8 @@ remap: functions: append: { return: ["array"] category: "Array" description: """ - Appends the specified item to an array. The item is appended regardless - of what is currently in the array. + Appends the specified item to an array and returns the new array. The item can be of any VRL + type and is appended even if an item with the same value is already present in the array. """ examples: [ { From 3599d3da0c85c31d7c7c44bb05425ec62f849ce5 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 29 Dec 2020 13:17:26 -0800 Subject: [PATCH 10/24] Revert behavior to null only on non-existing field Signed-off-by: Luc Perkins --- lib/remap-functions/src/del.rs | 70 +++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 01e8b3cdfd73f..1ae12cd24f161 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -28,6 +28,13 @@ pub struct DelFn { field: Path, } +impl DelFn { + #[cfg(test)] + fn new(field: Path) -> Self { + Self { field } + } +} + impl Expression for DelFn { fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result { match object.remove_and_get(self.field.as_ref(), false) { @@ -58,15 +65,58 @@ impl Expression for DelFn { #[cfg(test)] mod tests { use super::*; + use crate::map; - test_type_def![static_type_def { - expr: |_| DelFn { - field: Path::from("foo"), - }, - def: TypeDef { - fallible: false, - 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")), + ) + ]; + + 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); + } + } } From f5bcf6f30a5163d83185da8af7627512827b7a26 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 29 Dec 2020 13:25:26 -0800 Subject: [PATCH 11/24] Add dummy non-implementations of remove_and_get for LogEvent and Metric Signed-off-by: Luc Perkins --- lib/remap-functions/src/del.rs | 2 +- src/event/log_event.rs | 4 ++++ src/event/metric.rs | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 1ae12cd24f161..cdec352395b60 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -105,7 +105,7 @@ mod tests { map!["exists": 127], Ok(value!(127)), DelFn::new(Path::from("exists")), - ) + ), ]; let mut state = state::Program::default(); diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 7e21bfffacda0..9e382bda9a3a9 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -320,6 +320,10 @@ impl Object for LogEvent { .map(|key| remap::Path::from_alternative_string(key).map_err(|err| err.to_string())) .collect() } + + fn remove_and_get(&mut self, _: &Path, _: bool) -> Result, ()> { + unimplemented!(); + } } #[cfg(test)] diff --git a/src/event/metric.rs b/src/event/metric.rs index 8b8d4cdcaf092..c0c45e889deef 100644 --- a/src/event/metric.rs +++ b/src/event/metric.rs @@ -577,6 +577,10 @@ impl Object for Metric { .to_string()), } } + + fn remove_and_get(&mut self, _: &Path, _: bool) -> Result, ()> { + unimplemented!(); + } } fn write_list( From 3ef679a30ac1a2daefc97fd17bab39690d04a531 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 29 Dec 2020 13:27:19 -0800 Subject: [PATCH 12/24] Update docs to reflect new single-field logic Signed-off-by: Luc Perkins --- docs/reference/remap/del.cue | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index 659afb3788266..2a2c456f14875 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. + Removed 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; if the field does not exist, `null` + is returned. """# examples: [ { @@ -21,10 +22,9 @@ remap: functions: del: { input: { "field1": 1 "field2": 2 - "field3": 3 } source: #""" - del(.field1, .field3) + del(.field1) """# output: { "field2": 2 From f766a9ff3c4de6738f7c3bcc213d3fe13f8414a3 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 29 Dec 2020 13:54:05 -0800 Subject: [PATCH 13/24] Update function description re: return value Signed-off-by: Luc Perkins --- docs/reference/remap/append.cue | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/reference/remap/append.cue b/docs/reference/remap/append.cue index ee0b48a2db372..b7ca8f1333a2b 100644 --- a/docs/reference/remap/append.cue +++ b/docs/reference/remap/append.cue @@ -18,8 +18,9 @@ remap: functions: append: { return: ["array"] category: "Array" description: """ - Appends the specified item to an array and returns the new array. The item can be of any VRL - type and is appended even if an item with the same value is already present in the array. + Appends the specified item to an array and returns the modified array. The item can be of + any VRL type and is appended even if an item with the same value is already present in the + array. """ examples: [ { From 1b71b24a8770dbd1eb89e9a7915b1fca1bf6dc73 Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Thu, 31 Dec 2020 19:05:54 +0000 Subject: [PATCH 14/24] Implemented remove_and_get for LogEvent Signed-off-by: Stephen Wakely --- src/event/log_event.rs | 109 ++++++++++++++++++++++++++- tests/behavior/transforms/remap.toml | 21 ++++++ 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 9e382bda9a3a9..6e6d1dc23babc 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -288,6 +288,31 @@ impl Object for LogEvent { Ok(()) } + /// Remove the value from the event and return it. + /// Avoid just calling get to retreive the value since that clones and we can avoid that. + fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, ()> { + if path.is_root() { + return Ok(Some( + std::mem::replace(&mut self.fields, BTreeMap::new()) + .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() { + if !self.contains(&key) { + continue; + } + + return Ok(self.remove_prune(&key, compact).map(Into::into)); + } + + Ok(None) + } + fn insert(&mut self, path: &Path, value: remap::Value) -> Result<(), String> { if path.is_root() { match value { @@ -320,10 +345,6 @@ impl Object for LogEvent { .map(|key| remap::Path::from_alternative_string(key).map_err(|err| err.to_string())) .collect() } - - fn remove_and_get(&mut self, _: &Path, _: bool) -> Result, ()> { - unimplemented!(); - } } #[cfg(test)] @@ -668,6 +689,86 @@ mod test { } } + #[test] + fn object_remove_and_get() { + use crate::map; + use remap::{Field::*, Object, Path, Segment::*}; + + let cases = vec![ + ( + map!["foo": "bar"], + vec![Field(Regular("foo".to_owned()))], + false, + Some(map![].into()), + ), + ( + map!["foo": "bar"], + vec![Coalesce(vec![ + Quoted("foo bar".to_owned()), + Regular("foo".to_owned()), + ])], + false, + Some(map![].into()), + ), + ( + map!["foo": "bar", "baz": "qux"], + vec![], + false, + Some(map![].into()), + ), + ( + map!["foo": "bar", "baz": "qux"], + vec![], + true, + Some(map![].into()), + ), + ( + map!["foo": vec![0]], + vec![Field(Regular("foo".to_owned())), Index(0)], + false, + Some(map!["foo": Value::Array(vec![])].into()), + ), + ( + map!["foo": vec![0]], + vec![Field(Regular("foo".to_owned())), Index(0)], + true, + Some(map![].into()), + ), + ( + map!["foo": map!["bar baz": vec![0]], "bar": "baz"], + vec![ + Field(Regular("foo".to_owned())), + Field(Quoted("bar baz".to_owned())), + Index(0), + ], + false, + Some(map!["foo": map!["bar baz": Value::Array(vec![])], "bar": "baz"].into()), + ), + ( + map!["foo": map!["bar baz": vec![0]], "bar": "baz"], + vec![ + Field(Regular("foo".to_owned())), + Field(Quoted("bar baz".to_owned())), + Index(0), + ], + true, + Some(map!["bar": "baz"].into()), + ), + ]; + + 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_and_get(&mut event, &path, compact), + Ok(removed) + ); + assert_eq!(Object::get(&event, &Path::root()), Ok(expect)) + } + } + #[test] fn object_paths() { use crate::map; diff --git a/tests/behavior/transforms/remap.toml b/tests/behavior/transforms/remap.toml index 7e4c9bd46f9ee..2975756e1ec1d 100644 --- a/tests/behavior/transforms/remap.toml +++ b/tests/behavior/transforms/remap.toml @@ -151,6 +151,27 @@ "buz.first.exists" = false "buz.second.equals" = "buz second value" +[transforms.remap_rename_fields] + inputs = [] + type = "remap" + source = """ + .bar = del(.foo.second) + .buz = .foo + """ +[[tests]] + name = "remap_rename_fields" + [tests.input] + insert_at = "remap_rename_fields" + type = "log" + [tests.input.log_fields] + "foo.first" = "foo first value" + "foo.second" = "foo second value" + [[tests.outputs]] + extract_from = "remap_rename_fields" + [[tests.outputs.conditions]] + "bar.equals" = "foo second value" + "buz.first.equals" = "foo first value" + [transforms.remap_coercion] inputs = [] type = "remap" From b5d976442794e047d6c54663df2327d441db5794 Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Fri, 1 Jan 2021 10:50:25 +0000 Subject: [PATCH 15/24] Implemented remove_and_get for metrics Signed-off-by: Stephen Wakely --- lib/remap-lang/src/object.rs | 2 +- lib/remap-lang/src/value/object.rs | 2 +- src/event/log_event.rs | 2 +- src/event/metric.rs | 55 +++++++++++++++++++++++++----- 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/lib/remap-lang/src/object.rs b/lib/remap-lang/src/object.rs index 91c2c30ed0a86..f180c2558377e 100644 --- a/lib/remap-lang/src/object.rs +++ b/lib/remap-lang/src/object.rs @@ -61,5 +61,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_and_get(&mut self, path: &Path, compact: bool) -> Result, ()>; + fn remove_and_get(&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 366178c724529..1b3ee6ceaf36f 100644 --- a/lib/remap-lang/src/value/object.rs +++ b/lib/remap-lang/src/value/object.rs @@ -19,7 +19,7 @@ impl Object for Value { Ok(()) } - fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, ()> { + fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, String> { match self.get(path) { Ok(Some(val)) => { self.remove_by_path(path, compact); diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 6e6d1dc23babc..91016a775b0fa 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -290,7 +290,7 @@ impl Object for LogEvent { /// Remove the value from the event and return it. /// Avoid just calling get to retreive the value since that clones and we can avoid that. - fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, ()> { + fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, String> { if path.is_root() { return Ok(Some( std::mem::replace(&mut self.fields, BTreeMap::new()) diff --git a/src/event/metric.rs b/src/event/metric.rs index c0c45e889deef..cfc3050af5620 100644 --- a/src/event/metric.rs +++ b/src/event/metric.rs @@ -320,9 +320,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 +517,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(|tags| { + tags.iter() + .map(|(name, value)| (name.clone(), value.clone().into())) + .collect::>() + .into() + })), [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { Ok(self.tag_value(field.as_str()).map(|value| value.into())) } @@ -542,6 +548,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")); } @@ -566,6 +573,10 @@ impl Object for Metric { self.timestamp = None; Ok(()) } + [Segment::Field(tags)] if tags.as_str() == "tags" => { + self.tags = None; + Ok(()) + } [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { self.delete_tag(field.as_str()); Ok(()) @@ -578,8 +589,36 @@ impl Object for Metric { } } - fn remove_and_get(&mut self, _: &Path, _: bool) -> Result, ()> { - unimplemented!(); + fn remove_and_get(&mut self, path: &Path, _: bool) -> Result, String> { + if path.is_root() { + return Err(MetricPathError::SetPathError.to_string()); + } + + match path.segments() { + [Segment::Field(namespace)] if namespace.as_str() == "namespace" => { + Ok(self.namespace.take().map(Into::into)) + } + [Segment::Field(timestamp)] if timestamp.as_str() == "timestamp" => { + Ok(self.timestamp.take().map(Into::into)) + } + [Segment::Field(tags)] if tags.as_str() == "tags" => Ok(self + .tags + .take() + .map(|tags| { + tags.into_iter() + .map(|(name, value)| (name, value.into())) + .collect::>() + }) + .map(Into::into)), + [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { + Ok(self.delete_tag(field.as_str()).map(Into::into)) + } + _ => Err(MetricPathError::InvalidPath { + path: &path.to_string(), + expected: VALID_METRIC_PATHS_SET, + } + .to_string()), + } } } @@ -974,7 +1013,7 @@ mod test { assert_eq!( Ok( - ["name", "namespace", "timestamp", "tags.tig", "kind", "type"] + ["name", "namespace", "timestamp", "tags", "tags.tig", "kind", "type"] .iter() .map(|path| Path::from_str(path).expect("invalid path")) .collect() @@ -1026,10 +1065,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_and_get(&path, true)); assert_eq!(Ok(None), metric.get(&path)); } } From e9e24248be6dd33fd28e82e3160a7156bde51721 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 4 Jan 2021 11:19:17 -0700 Subject: [PATCH 16/24] Update docs with rename field example Signed-off-by: Luc Perkins --- docs/reference/remap/del.cue | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index 2a2c456f14875..21e8bbd37a0f3 100644 --- a/docs/reference/remap/del.cue +++ b/docs/reference/remap/del.cue @@ -12,9 +12,9 @@ remap: functions: del: { return: ["any"] category: "Event" description: #""" - Removed 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; if the field does not exist, `null` - is returned. + 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: [ { @@ -23,12 +23,20 @@ remap: functions: del: { "field1": 1 "field2": 2 } - source: #""" - del(.field1) - """# + source: "del(.field1)" output: { "field2": 2 } }, + { + title: "Rename field" + input: { + old_field: "please rename me" + } + source: ".new_field = del(.old_field)" + output: { + new_field: "please rename me" + } + } ] } From dff9788b2a16ff6fc9f8dcaf0af96f9842bbb839 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Mon, 4 Jan 2021 11:20:44 -0700 Subject: [PATCH 17/24] Rename field to path Signed-off-by: Luc Perkins --- lib/remap-functions/src/del.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index cdec352395b60..6ce7715d60b14 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -10,34 +10,34 @@ impl Function for Del { fn parameters(&self) -> &'static [Parameter] { &[Parameter { - keyword: "field", + keyword: "path", accepts: |_| true, required: true, }] } fn compile(&self, mut arguments: ArgumentList) -> Result> { - let field = arguments.required_path("field")?; + let path = arguments.required_path("path")?; - Ok(Box::new(DelFn { field })) + Ok(Box::new(DelFn { path })) } } #[derive(Debug, Clone)] pub struct DelFn { - field: Path, + path: Path, } impl DelFn { #[cfg(test)] - fn new(field: Path) -> Self { - Self { field } + fn new(path: Path) -> Self { + Self { path } } } impl Expression for DelFn { fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result { - match object.remove_and_get(self.field.as_ref(), false) { + match object.remove_and_get(self.path.as_ref(), false) { Ok(Some(val)) => Ok(val), _ => Ok(Value::Null), } From fc6241e15f52515a6c1b6e942641819c438cd56c Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Tue, 5 Jan 2021 10:15:21 -0700 Subject: [PATCH 18/24] Remove errant CUE doc Signed-off-by: Luc Perkins --- docs/reference/remap/append.cue | 40 --------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 docs/reference/remap/append.cue diff --git a/docs/reference/remap/append.cue b/docs/reference/remap/append.cue deleted file mode 100644 index b7ca8f1333a2b..0000000000000 --- a/docs/reference/remap/append.cue +++ /dev/null @@ -1,40 +0,0 @@ -package metadata - -remap: functions: append: { - arguments: [ - { - name: "value" - description: "The array" - required: true - type: ["array"] - }, - { - name: "item" - description: "The item to append" - required: true - type: ["any"] - }, - ] - return: ["array"] - category: "Array" - description: """ - Appends the specified item to an array and returns the modified array. The item can be of - any VRL type and is appended even if an item with the same value is already present in the - array. - """ - examples: [ - { - title: "Mixed array" - input: { - kitchen_sink: [72.5, false, [1, 2, 3]] - item: "booper" - } - source: """ - .kitchen_sink = append(.kitchen_sink, .item) - """ - output: { - kitchen_sink: [72.5, false, [1, 2, 3], "booper"] - } - }, - ] -} From a06a80618f68710bc62d20c3b419ce0ed9546387 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 6 Jan 2021 17:02:26 +0100 Subject: [PATCH 19/24] changes Signed-off-by: Jean Mertz --- lib/remap-functions/src/del.rs | 23 +++++- lib/remap-functions/src/only_fields.rs | 2 +- lib/remap-lang/src/object.rs | 8 +- lib/remap-lang/src/value/object.rs | 64 +++++---------- src/event/log_event.rs | 107 +------------------------ src/event/metric.rs | 84 +++++++------------ 6 files changed, 74 insertions(+), 214 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 6ce7715d60b14..ba939eddbdcf8 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -37,10 +37,25 @@ impl DelFn { impl Expression for DelFn { fn execute(&self, _: &mut state::Program, object: &mut dyn Object) -> Result { - match object.remove_and_get(self.path.as_ref(), false) { - Ok(Some(val)) => Ok(val), - _ => 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 infallible field. + // + // After some debating, we've decided to _silently ignore_ deletions + // of infallible fields for now, but we'll circle back to this in + // the near future to potentially improve this situation. + // + // see tracking issue: + Ok(object + .remove(self.path.as_ref(), false) + .ok() + .flatten() + .unwrap_or(Value::Null)) } fn type_def(&self, _: &state::Compiler) -> TypeDef { 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 f180c2558377e..193bd1a5c8350 100644 --- a/lib/remap-lang/src/object.rs +++ b/lib/remap-lang/src/object.rs @@ -55,11 +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>; - - /// Remove the given path from the object and return the value if it exists. - /// - /// If `compact` is true, after deletion, if an empty object or array is - /// left behind, it should be removed as well. - fn remove_and_get(&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 1b3ee6ceaf36f..25c015a7623fb 100644 --- a/lib/remap-lang/src/value/object.rs +++ b/lib/remap-lang/src/value/object.rs @@ -14,19 +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(()) - } - fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, String> { - match self.get(path) { - Ok(Some(val)) => { - self.remove_by_path(path, compact); - Ok(Some(val)) - } - _ => Ok(None), - } + Ok(value) } } @@ -193,10 +185,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 +206,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 +245,7 @@ mod tests { Index(0), ], false, + Some(value!(0)), Some(value!({foo: {"bar baz": []}, bar: "baz"})), ), ( @@ -250,49 +256,19 @@ 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::remove(&mut object, &path, compact), Ok(value)); assert_eq!(Object::get(&object, &Path::root()), Ok(expect)); } } - #[test] - fn object_remove_and_get() { - let cases = vec![ - ( - value![{foo: "bar"}], - vec![Field(Regular("foo".to_owned()))], - true, - Some(value!("bar")), - Some(value!({})), - ), - ( - value![{foo: "bar", boop: "bop"}], - vec![Field(Regular("boop".to_owned()))], - true, - Some(value!("bop")), - Some(value!({foo: "bar"})), - ), - ]; - - for (mut object, segments, compact, expect, end_result) in cases { - let path = Path::new_unchecked(segments); - - assert_eq!( - Object::remove_and_get(&mut object, &path, compact), - Ok(expect) - ); - assert_eq!(Object::get(&object, &path), Ok(None)); - assert_eq!(Object::get(&object, &Path::root()), Ok(end_result)); - } - } - #[test] fn object_paths() { let cases = vec![ diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 91016a775b0fa..4944662b1b621 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -266,31 +266,7 @@ impl Object for LogEvent { Ok(value) } - 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(()); - }; - - // loop until we find a path that exists. - for key in path.to_alternative_strings() { - if !self.contains(&key) { - continue; - } - - self.remove_prune(&key, compact); - break; - } - - Ok(()) - } - - /// Remove the value from the event and return it. - /// Avoid just calling get to retreive the value since that clones and we can avoid that. - fn remove_and_get(&mut self, path: &Path, compact: bool) -> Result, String> { + fn remove(&mut self, path: &Path, compact: bool) -> Result, String> { if path.is_root() { return Ok(Some( std::mem::replace(&mut self.fields, BTreeMap::new()) @@ -680,91 +656,12 @@ mod test { ), ]; - for (object, segments, compact, expect) in cases { - let mut event = LogEvent::from(object); - let path = Path::new_unchecked(segments); - - assert_eq!(Object::remove(&mut event, &path, compact), Ok(())); - assert_eq!(Object::get(&event, &Path::root()), Ok(expect)) - } - } - - #[test] - fn object_remove_and_get() { - use crate::map; - use remap::{Field::*, Object, Path, Segment::*}; - - let cases = vec![ - ( - map!["foo": "bar"], - vec![Field(Regular("foo".to_owned()))], - false, - Some(map![].into()), - ), - ( - map!["foo": "bar"], - vec![Coalesce(vec![ - Quoted("foo bar".to_owned()), - Regular("foo".to_owned()), - ])], - false, - Some(map![].into()), - ), - ( - map!["foo": "bar", "baz": "qux"], - vec![], - false, - Some(map![].into()), - ), - ( - map!["foo": "bar", "baz": "qux"], - vec![], - true, - Some(map![].into()), - ), - ( - map!["foo": vec![0]], - vec![Field(Regular("foo".to_owned())), Index(0)], - false, - Some(map!["foo": Value::Array(vec![])].into()), - ), - ( - map!["foo": vec![0]], - vec![Field(Regular("foo".to_owned())), Index(0)], - true, - Some(map![].into()), - ), - ( - map!["foo": map!["bar baz": vec![0]], "bar": "baz"], - vec![ - Field(Regular("foo".to_owned())), - Field(Quoted("bar baz".to_owned())), - Index(0), - ], - false, - Some(map!["foo": map!["bar baz": Value::Array(vec![])], "bar": "baz"].into()), - ), - ( - map!["foo": map!["bar baz": vec![0]], "bar": "baz"], - vec![ - Field(Regular("foo".to_owned())), - Field(Quoted("bar baz".to_owned())), - Index(0), - ], - true, - Some(map!["bar": "baz"].into()), - ), - ]; - 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_and_get(&mut event, &path, compact), - Ok(removed) - ); + 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 cfc3050af5620..50663ba326620 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)] @@ -517,12 +518,14 @@ 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(|tags| { - tags.iter() - .map(|(name, value)| (name.clone(), value.clone().into())) - .collect::>() - .into() - })), + [Segment::Field(tags)] if tags.as_str() == "tags" => { + Ok(self.tags.as_ref().map(|tags| { + tags.iter() + .map(|(name, value)| (name.clone(), value.clone().into())) + .collect::>() + .into() + })) + } [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { Ok(self.tag_value(field.as_str()).map(|value| value.into())) } @@ -559,37 +562,11 @@ impl Object for Metric { Ok(result) } - 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(()) - } - [Segment::Field(timestamp)] if timestamp.as_str() == "timestamp" => { - self.timestamp = None; - Ok(()) - } - [Segment::Field(tags)] if tags.as_str() == "tags" => { - self.tags = None; - Ok(()) - } - [Segment::Field(tags), Segment::Field(field)] if tags.as_str() == "tags" => { - self.delete_tag(field.as_str()); - Ok(()) - } - _ => Err(MetricPathError::InvalidPath { - path: &path.to_string(), - expected: VALID_METRIC_PATHS_SET, - } - .to_string()), - } - } - - fn remove_and_get(&mut self, path: &Path, _: bool) -> Result, String> { + fn remove( + &mut self, + path: &remap::Path, + _compact: bool, + ) -> Result, String> { if path.is_root() { return Err(MetricPathError::SetPathError.to_string()); } @@ -601,15 +578,10 @@ impl Object for Metric { [Segment::Field(timestamp)] if timestamp.as_str() == "timestamp" => { Ok(self.timestamp.take().map(Into::into)) } - [Segment::Field(tags)] if tags.as_str() == "tags" => Ok(self - .tags - .take() - .map(|tags| { - tags.into_iter() - .map(|(name, value)| (name, value.into())) - .collect::>() - }) - .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" => { Ok(self.delete_tag(field.as_str()).map(Into::into)) } @@ -1012,12 +984,18 @@ mod test { }; assert_eq!( - Ok( - ["name", "namespace", "timestamp", "tags", "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() ); } @@ -1068,7 +1046,7 @@ mod test { assert_eq!(Ok(Some(new.clone())), metric.get(&path)); if delete { - assert_eq!(Ok(Some(new)), metric.remove_and_get(&path, true)); + assert_eq!(Ok(Some(new)), metric.remove(&path, true)); assert_eq!(Ok(None), metric.get(&path)); } } From 04f82cf3f619bb89e3095ae685b5738c63fef069 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 6 Jan 2021 17:09:38 +0100 Subject: [PATCH 20/24] more changes Signed-off-by: Jean Mertz --- lib/remap-functions/Cargo.toml | 2 - lib/remap-functions/src/append.rs | 103 --------------------------- lib/remap-functions/src/del.rs | 13 +--- lib/remap-functions/src/lib.rs | 6 -- src/event/metric.rs | 8 +-- tests/behavior/transforms/remap.toml | 40 ----------- 6 files changed, 4 insertions(+), 168 deletions(-) delete mode 100644 lib/remap-functions/src/append.rs diff --git a/lib/remap-functions/Cargo.toml b/lib/remap-functions/Cargo.toml index 09f39cf8365e6..db2eeb94f6132 100644 --- a/lib/remap-functions/Cargo.toml +++ b/lib/remap-functions/Cargo.toml @@ -35,7 +35,6 @@ anyhow = "1" [features] default = [ - "append", "assert", "ceil", "compact", @@ -95,7 +94,6 @@ default = [ "uuid_v4", ] -append = [] assert = [] ceil = [] compact = [] diff --git a/lib/remap-functions/src/append.rs b/lib/remap-functions/src/append.rs deleted file mode 100644 index 1e9a5e47e52d3..0000000000000 --- a/lib/remap-functions/src/append.rs +++ /dev/null @@ -1,103 +0,0 @@ -use remap::prelude::*; - -#[derive(Clone, Copy, Debug)] -pub struct Append; - -impl Function for Append { - fn identifier(&self) -> &'static str { - "append" - } - - fn parameters(&self) -> &'static [Parameter] { - &[ - Parameter { - keyword: "value", - accepts: |v| matches!(v, Value::Array(_)), - required: true, - }, - Parameter { - keyword: "item", - accepts: |_| true, - required: true, - }, - ] - } - - fn compile(&self, mut arguments: ArgumentList) -> Result> { - let value = arguments.required("value")?.boxed(); - let item = arguments.required("item")?.boxed(); - - Ok(Box::new(AppendFn { value, item })) - } -} - -#[derive(Debug, Clone)] -struct AppendFn { - value: Box, - item: Box, -} - -impl Expression for AppendFn { - fn execute(&self, state: &mut state::Program, object: &mut dyn Object) -> Result { - let mut list = self.value.execute(state, object)?.try_array()?; - let item = self.item.execute(state, object)?; - - list.push(item); - - Ok(list.into()) - } - - fn type_def(&self, state: &state::Compiler) -> TypeDef { - use value::Kind; - - self.value - .type_def(state) - .fallible_unless(Kind::Array) - .merge(self.item.type_def(state)) - .with_constraint(Kind::Array) - .with_inner_type(None) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use value::Kind; - - test_type_def![ - value_array_infallible { - expr: |_| AppendFn { - value: Array::from(vec!["foo", "bar"]).boxed(), - item: Literal::from("baz").boxed(), - }, - def: TypeDef { kind: Kind::Array, ..Default::default() }, - } - - value_non_array_fallible { - expr: |_| AppendFn { - value: Literal::from(27).boxed(), - item: Literal::from("foo").boxed(), - }, - def: TypeDef { kind: Kind::Array, fallible: true, ..Default::default() }, - } - ]; - - test_function![ - append => Append; - - empty_array { - args: func_args![value: value!([]), item: value!("foo")], - want: Ok(value!(["foo"])), - } - - new_item { - args: func_args![value: value!([11, false, 42.5]), item: value!("foo")], - want: Ok(value!([11, false, 42.5, "foo"])), - } - - already_exists_item { - args: func_args![value: value!([11, false, 42.5]), item: value!(42.5)], - want: Ok(value!([11, false, 42.5, 42.5])), - } - ]; -} diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index ba939eddbdcf8..8607bf119e706 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -59,19 +59,8 @@ impl Expression for DelFn { } fn type_def(&self, _: &state::Compiler) -> TypeDef { - use value::Kind; - TypeDef { - fallible: false, - kind: Kind::Bytes - | Kind::Integer - | Kind::Float - | Kind::Boolean - | Kind::Map - | Kind::Array - | Kind::Timestamp - | Kind::Regex - | Kind::Null, + kind: value::Kind::all(), ..Default::default() } } diff --git a/lib/remap-functions/src/lib.rs b/lib/remap-functions/src/lib.rs index 9296b3ed308de..c8437fde5c941 100644 --- a/lib/remap-functions/src/lib.rs +++ b/lib/remap-functions/src/lib.rs @@ -1,7 +1,5 @@ mod util; -#[cfg(feature = "append")] -mod append; #[cfg(feature = "assert")] mod assert; #[cfg(feature = "ceil")] @@ -123,8 +121,6 @@ mod uuid_v4; pub use crate::md5::Md5; #[cfg(feature = "sha1")] pub use crate::sha1::Sha1; -#[cfg(feature = "append")] -pub use append::Append; #[cfg(feature = "assert")] pub use assert::Assert; #[cfg(feature = "ceil")] @@ -238,8 +234,6 @@ pub use uuid_v4::UuidV4; pub fn all() -> Vec> { vec![ - #[cfg(feature = "append")] - Box::new(Append), #[cfg(feature = "assert")] Box::new(Assert), #[cfg(feature = "ceil")] diff --git a/src/event/metric.rs b/src/event/metric.rs index 50663ba326620..b35d30207bc47 100644 --- a/src/event/metric.rs +++ b/src/event/metric.rs @@ -519,11 +519,9 @@ impl Object for Metric { } [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(|tags| { - tags.iter() - .map(|(name, value)| (name.clone(), value.clone().into())) - .collect::>() - .into() + 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" => { diff --git a/tests/behavior/transforms/remap.toml b/tests/behavior/transforms/remap.toml index 2819b2f7b14d9..5ed188a6a7f24 100644 --- a/tests/behavior/transforms/remap.toml +++ b/tests/behavior/transforms/remap.toml @@ -46,25 +46,6 @@ "b[1].equals" = "" "b[2].equals" = "two" -[transforms.append_to_array] - inputs = [] - type = "remap" - source = """ - .fruits = append(.fruits, .fruit) - """ -[[tests]] - name = "append_to_array" - [tests.input] - insert_at = "append_to_array" - type = "log" - [tests.input.log_fields] - fruits = ["apple", "orange", "banana"] - fruit = "mango" - [[tests.outputs]] - extract_from = "append_to_array" - [[tests.outputs.conditions]] - "fruits.equals" = ["apple", "orange", "banana", "mango"] - [transforms.remap_arithmetic] inputs = [] type = "remap" @@ -170,27 +151,6 @@ "buz.first.exists" = false "buz.second.equals" = "buz second value" -[transforms.remap_rename_fields] - inputs = [] - type = "remap" - source = """ - .bar = del(.foo.second) - .buz = .foo - """ -[[tests]] - name = "remap_rename_fields" - [tests.input] - insert_at = "remap_rename_fields" - type = "log" - [tests.input.log_fields] - "foo.first" = "foo first value" - "foo.second" = "foo second value" - [[tests.outputs]] - extract_from = "remap_rename_fields" - [[tests.outputs.conditions]] - "bar.equals" = "foo second value" - "buz.first.equals" = "foo first value" - [transforms.remap_coercion] inputs = [] type = "remap" From 6f1366bf9817cec307130b535de5b383101cdbfa Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 6 Jan 2021 17:25:03 +0100 Subject: [PATCH 21/24] some more changes Signed-off-by: Jean Mertz --- docs/reference/remap/del.cue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index 21e8bbd37a0f3..4c22b8f4516fe 100644 --- a/docs/reference/remap/del.cue +++ b/docs/reference/remap/del.cue @@ -37,6 +37,6 @@ remap: functions: del: { output: { new_field: "please rename me" } - } + }, ] } From 1feae2b9d2b97c0378c04438bff224ca816777e6 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 6 Jan 2021 18:41:05 +0100 Subject: [PATCH 22/24] improvements Signed-off-by: Jean Mertz --- docs/reference/remap.cue | 2 +- docs/reference/remap/del.cue | 18 ++++++++++++++++++ lib/remap-functions/src/del.rs | 9 ++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/docs/reference/remap.cue b/docs/reference/remap.cue index 5d9ee48667e59..1501cffab2369 100644 --- a/docs/reference/remap.cue +++ b/docs/reference/remap.cue @@ -49,7 +49,7 @@ remap: { arguments: [...#Argument] // Allow for empty list return: [#RemapReturnTypes, ...#RemapReturnTypes] - category: "Array" | "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp" + category: "Coerce" | "Encode" | "Enumerate" | "Event" | "Hash" | "IP" | "Map" | "Number" | "Parse" | "Random" | "String" | "Test" | "Timestamp" description: string examples?: [#RemapExample, ...#RemapExample] name: Name diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index 4c22b8f4516fe..d4aba924d29fc 100644 --- a/docs/reference/remap/del.cue +++ b/docs/reference/remap/del.cue @@ -28,6 +28,24 @@ remap: functions: del: { "field2": 2 } }, + { + title: "Delete existing only" + input: { + "user_id": 1 + } + source: #""" + .user.id = if exists(.user_id) { + del(.user_id) + } else { + "unknown" + } + """# + output: { + user: { + "id": 1 + } + } + }, { title: "Rename field" input: { diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 8607bf119e706..0fd21b865fb62 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -50,7 +50,7 @@ impl Expression for DelFn { // of infallible fields for now, but we'll circle back to this in // the near future to potentially improve this situation. // - // see tracking issue: + // see tracking issue: https://github.com/timberio/vector/issues/5887 Ok(object .remove(self.path.as_ref(), false) .ok() @@ -70,6 +70,7 @@ impl Expression for DelFn { mod tests { use super::*; use crate::map; + use std::str::FromStr; #[test] fn del() { @@ -110,6 +111,12 @@ mod tests { 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(); From 9c0f32bb3dfea8c01d7c057f7aa4cc9b96cbc38f Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Wed, 6 Jan 2021 21:46:36 -0700 Subject: [PATCH 23/24] Fix formatting issue in CUE source Signed-off-by: Luc Perkins --- docs/reference/remap/del.cue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/remap/del.cue b/docs/reference/remap/del.cue index d4aba924d29fc..4fd1cd54e7120 100644 --- a/docs/reference/remap/del.cue +++ b/docs/reference/remap/del.cue @@ -35,9 +35,9 @@ remap: functions: del: { } source: #""" .user.id = if exists(.user_id) { - del(.user_id) + del(.user_id) } else { - "unknown" + "unknown" } """# output: { From 2c83be4fd86eb7eb48b9731493d41789f00d58d7 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Thu, 7 Jan 2021 10:15:02 +0100 Subject: [PATCH 24/24] changes Signed-off-by: Jean Mertz --- lib/remap-functions/src/del.rs | 14 +++++++------- src/event/log_event.rs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/remap-functions/src/del.rs b/lib/remap-functions/src/del.rs index 0fd21b865fb62..7985da7c042b3 100644 --- a/lib/remap-functions/src/del.rs +++ b/lib/remap-functions/src/del.rs @@ -40,15 +40,15 @@ impl Expression for DelFn { // 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. + // 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 infallible field. + // 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 infallible fields for now, but we'll circle back to this in - // the near future to potentially improve this situation. + // 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 diff --git a/src/event/log_event.rs b/src/event/log_event.rs index 4944662b1b621..2e2b0fd509bfc 100644 --- a/src/event/log_event.rs +++ b/src/event/log_event.rs @@ -269,7 +269,7 @@ impl Object for LogEvent { fn remove(&mut self, path: &Path, compact: bool) -> Result, String> { if path.is_root() { return Ok(Some( - std::mem::replace(&mut self.fields, BTreeMap::new()) + std::mem::take(&mut self.fields) .into_iter() .map(|(key, value)| (key, value.into())) .collect::>()