MAINT: StringPrepper use Event model #753
Conversation
Signed-off-by: Chen <[email protected]>
| final Collection<Record<Event>> modifiedRecords = new ArrayList<>(records.size()); | ||
| for (Record<Event> record : records) { | ||
| final Event recordEvent = record.getData(); | ||
| final String recordData = recordEvent.toJsonString(); |
There was a problem hiding this comment.
I don't think this is the correct approach. This will capitalize keys as well, which is incorrect.
I'd think the closest feature parity here is to iterate over every key. For each string-value, make it uppercase.
This processor should probably be obsoleted and replaced with the one being created for #697. But, that is a breaking change and we still need something in 1.3.
There was a problem hiding this comment.
I'd think the closest feature parity here is to iterate over every key. For each string-value, make it uppercase.
I agree this might make more sense. Yeah, the Prepper itself is a bit outdated.
There was a problem hiding this comment.
There seems to be some overlap between the mutate processor and the string_converter proposed here. I think separating out between string mutations and generic mutations makes some sense, but it is possible that doing so could lead to a worse user experience as well.
There was a problem hiding this comment.
I added a comment in #508. We should discuss in that issue how granular we want these different plugins to be. Please read and comment.
Signed-off-by: Chen <[email protected]>
Signed-off-by: Chen <[email protected]>
Signed-off-by: Chen <[email protected]>
Signed-off-by: Chen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #753 +/- ##
============================================
- Coverage 91.59% 91.56% -0.03%
- Complexity 624 630 +6
============================================
Files 75 75
Lines 1844 1862 +18
Branches 158 159 +1
============================================
+ Hits 1689 1705 +16
- Misses 117 120 +3
+ Partials 38 37 -1
Continue to review full report at Codecov.
|
| } | ||
| return modifiedRecords; | ||
| } | ||
|
|
||
| private Map<String, Object> processEventJson(final String data) throws JsonProcessingException { |
There was a problem hiding this comment.
We probably need some new features in Event if this is the necessary approach! But, this is fine for this processor.
There was a problem hiding this comment.
Agree. Did not find an API to iterate through the keys in Event data, neither the Event::toMap() method.
Signed-off-by: Chen [email protected]
Description
This PR migrates StringPrepper to use Event model.
Issues Resolved
Resolves #608
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.