-
Notifications
You must be signed in to change notification settings - Fork 197
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
Support for dynamic renaming of keys #5074
base: main
Are you sure you want to change the base?
Conversation
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Outdated
Show resolved
Hide resolved
...t/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessorTests.java
Show resolved
Hide resolved
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.
Nice work! A few minor issues in addition to David's comments.
Also, not blocking this PR, have you got a chance to take a look at adding the to_key_pattern
option as well? It would be great value added. We can add it in a separate PR if it's straightforward to do.
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Outdated
Show resolved
Hide resolved
.../java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessorConfig.java
Outdated
Show resolved
Hide resolved
@oeyh Haven't got a chance yet to look at the to_key_pattern. I can analyse the complexity and see how it goes. Will raise a separate PR if it is trivial. |
import java.util.List; | ||
import java.util.Map; | ||
import java.util.UUID; | ||
import java.util.*; |
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.
Build is failing due to this. Imports need to be explicit.
if (fromKeyPattern != null) { | ||
fromKeyCompiledPattern = Pattern.compile(fromKeyPattern); |
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.
This doesn't work when Data Prepper reads config from a yaml file. This constructor is not called. We probably want to move it to getFromKeyCompiledPattern()
method.
public Pattern getFromKeyCompiledPattern() {
if (fromKeyPattern != null && fromKeyCompiledPattern == null) {
fromKeyCompiledPattern = Pattern.compile(fromKeyPattern);
}
return fromKeyCompiledPattern;
}
...c/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessor.java
Show resolved
Hide resolved
1f76593
to
5225f66
Compare
@@ -161,7 +161,8 @@ private static boolean isValidKey(final String key) { | |||
|| c == '@' | |||
|| c == '/' | |||
|| c == '[' | |||
|| c == ']')) { | |||
|| c == ']' | |||
|| c == '|')) { |
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.
@dlvenable Are you OK with this approach? This change is for it to work with the example data provided in #4849, where "|" appears in field keys. Without it, the processor fails to delete the original field due to invalid char "|" in the key with a config like this:
- rename_keys:
entries:
- from_key_pattern: "delivered_at.+"
to_key: "delivered_at"
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.
Discussed offline. We agree on skipping the validation for the delete method.
|
||
public Pattern getFromKeyCompiledPattern() { |
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 you need to add @JsonIgnore
to this method.
d35eb71
to
0bba246
Compare
Signed-off-by: Divyansh Bokadia <[email protected]> Dynamic renaming of keys Signed-off-by: Divyansh Bokadia <[email protected]>
0bba246
to
86ba9b7
Compare
Description
It includes support for dynamic renaming of keys in rename processor where the user can now give a regex pattern to match the keys that needs to be replaced.
Issues Resolved
Resolves #4849
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.