-
Notifications
You must be signed in to change notification settings - Fork 210
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
Enhance copy_values processor to selectively copy entries from lists #4085
Conversation
Signed-off-by: Hai Yan <[email protected]>
Signed-off-by: Hai Yan <[email protected]>
Signed-off-by: Hai Yan <[email protected]>
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.
Minor changes. I suppose there will be certain metrics added but can be in a separate PR
if (!recordEvent.containsKey(entry.getToKey()) || entry.getOverwriteIfToKeyExists()) { | ||
final Object source = recordEvent.get(entry.getFromKey(), Object.class); | ||
recordEvent.put(entry.getToKey(), source); | ||
if (!recordEvent.containsKey(entry.getToKey()) || entry.getOverwriteIfToKeyExists()) { |
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.
Suggest refactor L77-85 into a private method shouldCopyEntry
or skipCopyEntry
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.
Good one. Made the change.
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.
Also opened an issue for adding metrics: #4088
Signed-off-by: Hai Yan <[email protected]>
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.
Please make necessary changes to README.md file as well.
} | ||
try { | ||
final Event recordEvent = record.getData(); | ||
if (config.getFromList() != null || config.getToList() != null) { |
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.
Is this check correct? What is the expected behavior if getFromList()
returns null but getToList()
is not null?
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 have a validation in CopyValueProcessorConfig
that checks from_list
and to_list
should be both null or both non-null. So I assume this situation that getFromList()
returns null but getToList()
is not null won't happen.
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.
Regarding documentation, since we are adopting the new process to use the documentation website and remove readme (see #2740). I have this issue created to update over there: opensearch-project/documentation-website#6390
Description
The configurations are like this:
Example input:
Example output:
Issues Resolved
Resolves #3962
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.