feat(new transform): New `remap` transform #3341

Jeffail · 2020-08-04T09:32:32Z

Implements a new remap transform following https://github.com/timberio/vector/blob/master/rfcs/2020-07-21-2744-remapping-syntax.md, which provides a mapping language for expressing a series of translations for events:

[transforms.mapper]
  inputs = [ "foo" ]
  type = "remap"
  mapping = """
    .foo = .bar
    del(.bar)
    .baz = "something literal"
  """

In order to mutate events with the existing API I've had to make a few value utility functions public, I'm hoping this is only temporary and that https://github.com/timberio/vector/blob/master/rfcs/2020-05-25-2692-more-usable-logevents.md will address some of the oddness of the API.

I've added TODO comments anywhere that uses event API's that are obsolete or just seem wrong to me, but in terms of functionality we seem to be okay for now, so no need to rush in those changes.

Path escaping is not currently implemented, my goal was to implement quoted segments like .foo."bar.baz".buz but I haven't dug into the event APIs enough to see if that's convenient. I'll take another look after arithmetic is added but from what I've seen thus far it looks like we're only able to use slash escapes like .foo.bar\.baz.buz so I'll need to translate manually.

Closes #3119
Closes #3467
Closes #3722
Closes https://github.com/timberio/vector-product/issues/70

binarylogic · 2020-08-04T14:24:26Z

Nice work!

@Jeffail if there was anything obviously wrong with the data model, could you open issues and add them to https://github.com/timberio/vector-product/issues/8?

src/transforms/remap.rs

src/mapping/query/path.rs

src/mapping/mod.rs

src/mapping/parser/mod.rs

src/mapping/query/path.rs

Hoverbear · 2020-08-17T17:47:52Z

Where is the CI on this? 🤔

.meta/transforms/remap.toml.erb

Hoverbear · 2020-08-17T18:09:23Z

benches/bench.rs

@@ -665,6 +673,90 @@ fn bench_elasticsearch_index(c: &mut Criterion) {
    );
 }

+fn benchmark_remap(c: &mut Criterion) {


test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out Running target/release/deps/bench-0f9a98460f4dba0d add fields with remap time: [1.3603 us 1.3614 us 1.3625 us] change: [-1.5879% +0.1667% +1.5785%] (p = 0.84 > 0.05) No change in performance detected. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild add fields with add_fields time: [2.2038 us 2.2086 us 2.2152 us] change: [-0.9292% -0.6531% -0.2980%] (p = 0.00 < 0.05) Change within noise threshold. Found 6 outliers among 100 measurements (6.00%) 1 (1.00%) low mild 2 (2.00%) high mild 3 (3.00%) high severe

Very nice. Good job. :)

src/mapping/query/arithmetic.rs

Hoverbear · 2020-08-17T18:42:31Z

src/mapping/query/path.rs

+    use serde_json::json;
+
+    #[test]
+    fn check_path_query() {


It'd be useful to see whitespace including tests. Eg .foo\ bar\ baz

Realised from this that I hadn't even implemented quoted path segments yet so good catch!

src/mapping/mod.rs

Signed-off-by: Ashley Jeffs <[email protected]>

Signed-off-by: Jean Mertz <[email protected]>

StephenWakely · 2020-09-08T14:26:31Z

src/mapping/parser/grammar.pest

+        statement ~
+    NEWLINE* ~ "}" )?
+}
+


Would it be worth putting NEWLINE* on either side of the brackets so people can indent Allman style? Someone is inevitably going to try.

bruceg

I left some comments for cleanups you could consider, but they shouldn't change behavior or correctness.

src/mapping/parser/mod.rs

bruceg · 2020-09-08T17:20:34Z

src/mapping/query/arithmetic.rs

+                let (left, right) = coerce_number_types(left?, right?);
+                match left {
+                    Value::Float(fl) => match right {
+                        Value::Float(fr) => Value::Float(fl * fr),
+                        vr => {
+                            return Err(format!(
+                                "unable to multiply right-hand field type {:?}",
+                                vr
+                            ))
+                        }
+                    },
+                    Value::Integer(il) => match right {
+                        Value::Integer(ir) => Value::Integer(il * ir),
+                        vr => {
+                            return Err(format!(
+                                "unable to multiply right-hand field type {:?}",
+                                vr
+                            ))
+                        }
+                    },
+                    vl => return Err(format!("unable to multiply left-hand field type {:?}", vl)),


Most of the rest of the arms of this could use the match coerce_number_types(left?, right?) treatment.

src/mapping/query/functions.rs

Hoverbear · 2020-09-08T18:16:20Z

src/mapping/query/path.rs

+use string_cache::DefaultAtom as Atom;
+
+#[derive(Debug)]
+pub(in crate::mapping) struct Path {


It's kind of annoying how much logic gets duplicated with https://github.com/timberio/vector/blob/master/src/event/util/log/path_iter.rs but I understand why

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

Signed-off-by: Jean Mertz <[email protected]>

JeanMertz · 2020-09-09T14:34:45Z

reported flaky tests at:

binarylogic · 2020-09-09T14:40:17Z

Nice work 🎉

Co-authored-by: Bruce Guenter <[email protected]> Co-authored-by: Jean Mertz <[email protected]> Signed-off-by: Brian Menges <[email protected]>

binarylogic requested review from bruceg, lukesteensen and Hoverbear August 4, 2020 14:23

bruceg reviewed Aug 4, 2020

View reviewed changes

binarylogic mentioned this pull request Aug 7, 2020

More clearly define component purpose/concern boundaries #1731

Closed

2 tasks

Jeffail force-pushed the remap-transform branch 2 times, most recently from 3eed634 to b48d2b0 Compare August 11, 2020 09:56

Jeffail marked this pull request as ready for review August 11, 2020 14:30

binarylogic mentioned this pull request Aug 15, 2020

Transform to allowlist fields #3467

Closed

Hoverbear reviewed Aug 17, 2020

View reviewed changes

.meta/transforms/remap.toml.erb Outdated Show resolved Hide resolved

Hoverbear reviewed Aug 17, 2020

View reviewed changes

.meta/transforms/remap.toml.erb Outdated Show resolved Hide resolved

Hoverbear reviewed Aug 17, 2020

View reviewed changes

src/mapping/query/arithmetic.rs Outdated Show resolved Hide resolved

Hoverbear reviewed Aug 17, 2020

View reviewed changes

src/mapping/mod.rs Show resolved Hide resolved

Jeffail added 13 commits August 18, 2020 10:07

Add remap transform

ae9e84e

Signed-off-by: Ashley Jeffs <[email protected]>

Allow newlines* at the end of maps

f6311ca

Signed-off-by: Ashley Jeffs <[email protected]>

WIP: Implement arithmetic parser

00e2629

Signed-off-by: Ashley Jeffs <[email protected]>

WIP: Implement op parser and grouping

e3471fc

Signed-off-by: Ashley Jeffs <[email protected]>

Finish arithmetic implementations

645976c

Signed-off-by: Ashley Jeffs <[email protected]>

Add remap meta file

3850561

Signed-off-by: Ashley Jeffs <[email protected]>

Addressing comments

2a449ae

Signed-off-by: Ashley Jeffs <[email protected]>

Add remap example

6d21cf6

Signed-off-by: Ashley Jeffs <[email protected]>

More tidy up

b01a940

Signed-off-by: Ashley Jeffs <[email protected]>

fmt

96629fb

Signed-off-by: Ashley Jeffs <[email protected]>

Clippy stuff

904d7bf

Signed-off-by: Ashley Jeffs <[email protected]>

Implement remap if statements

10d5ee5

Signed-off-by: Ashley Jeffs <[email protected]>

Add if statement example to remap docs

97fae47

Signed-off-by: Ashley Jeffs <[email protected]>

JeanMertz added 3 commits September 7, 2020 11:15

add field with spacing test

6ff7d4a

Signed-off-by: Jean Mertz <[email protected]>

add "quoted path segment" behavior test

975d854

Signed-off-by: Jean Mertz <[email protected]>

update remap internal event for dropped events

66ea912

Signed-off-by: Jean Mertz <[email protected]>

This was referenced Sep 7, 2020

Allow multiplying strings/arrays in remap transform #3728

Closed

Support plain (non-float) integer division in remap transform #3729

Closed

binarylogic changed the title ~~feat(new transform): Add remap transform~~ feat(new transform): New remap transform Sep 7, 2020

JeanMertz mentioned this pull request Sep 8, 2020

feat(remap transform): Add new set of functions #3767

Merged

15 tasks

JeanMertz requested a review from bruceg September 8, 2020 13:31

StephenWakely reviewed Sep 8, 2020

View reviewed changes

bruceg approved these changes Sep 8, 2020

View reviewed changes

Hoverbear reviewed Sep 8, 2020

View reviewed changes

Hoverbear approved these changes Sep 8, 2020

View reviewed changes

JeanMertz and others added 5 commits September 9, 2020 12:38

Update src/mapping/parser/mod.rs

2f2b5d4

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

Update src/mapping/parser/mod.rs

9a68ebc

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

Update src/mapping/parser/mod.rs

1a15382

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

Update src/mapping/query/functions.rs

4c15291

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

Update src/mapping/query/functions.rs

18acbbc

Co-authored-by: Bruce Guenter <[email protected]> Signed-off-by: Jean Mertz <[email protected]>

JeanMertz force-pushed the remap-transform branch from 1601b2c to 18acbbc Compare September 9, 2020 10:39

fix code suggestions

a996bd0

Signed-off-by: Jean Mertz <[email protected]>

JeanMertz merged commit 7c5aecb into master Sep 9, 2020

JeanMertz deleted the remap-transform branch September 9, 2020 14:36

JeanMertz added type: tech debt A code change that does not add user value. and removed type: tech debt A code change that does not add user value. labels Sep 14, 2020

JeanMertz mentioned this pull request Sep 14, 2020

Lift named remap functions out of the parser #3864

Closed

vector-vic mentioned this pull request Sep 16, 2020

Log schemas #3910

Closed

JeanMertz mentioned this pull request Sep 23, 2020

feat(remap transform): named function arguments #3927

Merged

JeanMertz mentioned this pull request Jan 19, 2021

switch VRL parser from Pest to Nom #6139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(new transform): New `remap` transform #3341

feat(new transform): New `remap` transform #3341

Jeffail commented Aug 4, 2020 •

edited by binarylogic

Loading

binarylogic commented Aug 4, 2020

Hoverbear commented Aug 17, 2020

Hoverbear Aug 17, 2020

Hoverbear Aug 17, 2020

Jeffail Aug 20, 2020

StephenWakely Sep 8, 2020

bruceg left a comment

bruceg Sep 8, 2020

Hoverbear Sep 8, 2020

JeanMertz commented Sep 9, 2020

binarylogic commented Sep 9, 2020

feat(new transform): New remap transform #3341

feat(new transform): New remap transform #3341

Conversation

Jeffail commented Aug 4, 2020 • edited by binarylogic Loading

binarylogic commented Aug 4, 2020

Hoverbear commented Aug 17, 2020

Hoverbear Aug 17, 2020

Choose a reason for hiding this comment

Hoverbear Aug 17, 2020

Choose a reason for hiding this comment

Jeffail Aug 20, 2020

Choose a reason for hiding this comment

StephenWakely Sep 8, 2020

Choose a reason for hiding this comment

bruceg left a comment

Choose a reason for hiding this comment

bruceg Sep 8, 2020

Choose a reason for hiding this comment

Hoverbear Sep 8, 2020

Choose a reason for hiding this comment

JeanMertz commented Sep 9, 2020

binarylogic commented Sep 9, 2020

feat(new transform): New `remap` transform #3341

feat(new transform): New `remap` transform #3341

Jeffail commented Aug 4, 2020 •

edited by binarylogic

Loading