-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(reduce transform): improve array handling #3076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Luke Steensen <[email protected]>
@@ -77,12 +77,11 @@ impl ReduceState { | |||
fn new(e: LogEvent, strategies: &IndexMap<String, MergeStrategy>) -> Self { | |||
Self { | |||
stale_since: Instant::now(), | |||
// TODO: all_fields alternative that consumes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r#" | ||
identifier_fields = [ "request_id" ] | ||
|
||
merge_strategies.foo = "array" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about one for concat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to consider the concat strategy as well!
I actually started down this path and backed out because it seemed slightly out of scope. The main thing that stopped me was that the current |
Signed-off-by: Luke Steensen <[email protected]>
Ok that's fine with me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to fix the issue!
Signed-off-by: Luke Steensen <[email protected]>
Just kidding, it wasn't that bad. Just pushed an implementation. |
Signed-off-by: Luke Steensen <[email protected]> Signed-off-by: Brian Menges <[email protected]>
Closes #3074
Two bugs fixed here:
all_fields
instead ofinto_iter
gaves us paths instead of keys, so thing like arrays and objects would not get strategized correctly.ArrayMerger
meant that the initial array would be flattened in the result but subsequent additions would not.