Added support of newline in output codec#6423
Conversation
Subrahmanyam-Gollapalli
requested review from
KarstenSchnitter,
chenqi0805,
dinujoh,
dlvenable,
engechas,
graytaylor0,
kkondaka,
oeyh,
san81,
sb2k16 and
srikanthjg
as code owners
Signed-off-by: Subrahmanyam-Gollapalli <subrahmanyam.gollapalli@freshworks.com>
Subrahmanyam-Gollapalli
force-pushed
the
newline-output-codec
branch
from
d2003d9 to
ae66059
Compare
dlvenable
left a comment
dlvenable
left a comment
There was a problem hiding this comment.
Thank you @Subrahmanyam-Gollapalli for making this contribution! I have a few comments, but overall this looks like a good change.
| if (event.containsKey(MESSAGE_FIELD)) { | ||
| Object messageObj = event.get(MESSAGE_FIELD, Object.class); | ||
| if (messageObj != null) { | ||
| message = messageObj.toString(); |
There was a problem hiding this comment.
I tend to think that we should only write this if it is a string. If there is an object here like an array, you'd get some Java identifier that is not very helpful.
Maybe this could be a configuration. But, I think the default should be to only use known types.
| } | ||
|
|
||
| // If message is null or empty, write empty string | ||
| if (message == null) { |
There was a problem hiding this comment.
I think the default behavior should be not to write anything if the data is empty. You could add a configuration to write empty events.
Perhaps a configuration named include_empty_objects similar to the ndjson input codec. Here is that configuration:
|
|
||
| @Override | ||
| public void writeEvent(final Event event, final OutputStream outputStream) throws IOException { | ||
| Objects.requireNonNull(event); |
There was a problem hiding this comment.
Can this call doWriteEvent to consolidate the logic?
| @@ -0,0 +1,123 @@ | |||
| /* | |||
There was a problem hiding this comment.
We have an updated license header format. Please update all the files.
See: https://github.com/opensearch-project/data-prepper/blob/main/CONTRIBUTING.md#license-headers
| final OutputCodecContext codecContext = new OutputCodecContext(); | ||
|
|
||
| final Map<String, Object> eventData = new HashMap<>(); | ||
| eventData.put("message", "r_id=2bb2bd0aeece11f0bea286fb87d48915-ticket_update,tp=00-13a3bb055e6b589dbc0f952e0d75020a-1f2e986790c19742-01"); |
There was a problem hiding this comment.
Rather than this static string, generate the value as a new UUID. This is a pattern found throughout the project. Then compare against that.
Signed-off-by: Subrahmanyam-Gollapalli <subrahmanyam.gollapalli@freshworks.com>
Subrahmanyam-Gollapalli
force-pushed
the
newline-output-codec
branch
from
0d576a4 to
ab3c85f
Compare
Please update the license headers per the comment above. |
Signed-off-by: Subrahmanyam-Gollapalli <subrahmanyam.gollapalli@freshworks.com>
| return NEWLINE; | ||
| } | ||
|
|
||
| private static void doWriteEvent(final OutputStream outputStream, final Event event, final OutputCodecContext codecContext, final boolean includeEmptyObjects) throws IOException { |
There was a problem hiding this comment.
codecContext is not used anywhere in this method.
3 participants
Description
Added support of newline in the output codec
Issues Resolved
This new support hasn't resolved anything. It gives support to use newline in the output codec. We have this in Logstash, and we need it in Data Prepper. Test cases are also added.
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.