-
Notifications
You must be signed in to change notification settings - Fork 195
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
Support for dynamic renaming of keys Signed-off-by: Divyansh Bokadia <dbokadia@amazon.com>
...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
Signed-off-by: Divyansh Bokadia <dbokadia@amazon.com>
@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
c5ca2fe
to
1f76593
Compare
Dynamic renaming of keys Signed-off-by: Divyansh Bokadia <dbokadia@amazon.com>
Signed-off-by: Divyansh Bokadia <dbokadia@amazon.com>
1f76593
to
5225f66
Compare
Signed-off-by: Divyansh Bokadia <dbokadia@amazon.com>
@@ -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.
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.