Added parsing otel json trace to span from kafka topic #5590
Conversation
Mamol27
requested review from
sb2k16,
chenqi0805,
engechas,
san81,
srikanthjg,
graytaylor0,
dinujoh,
kkondaka,
KarstenSchnitter,
dlvenable and
oeyh
as code owners
|
@Mamol27 Thank you for your submission. Wondering if it makes sense to use "JSON" message format (because the input is in json) and then have something like "codec" which would convert JSON to OTEL trace. |
There was a problem hiding this comment.
I think, there is an easier way leveraging the protobuf JsonFormat. This can lead to more code reusage in the mapping of the OTel data. This would be desirable to the project, as it has a lower maintenance effort.
I can only recommend to use the protobuf binary representation to store in Kafka over JSON. There is a much higher throughput in serialization and deserialization of protobuf over JSON.
| @@ -0,0 +1,201 @@ | |||
| package org.opensearch.dataprepper.plugins.kafka.parcer; | |||
There was a problem hiding this comment.
Probably a typo parcer -> parser
|
|
||
| import java.util.List; | ||
|
|
||
| public class JsonOtelTracePojo { |
There was a problem hiding this comment.
Why do you need a new class for the mapping?
Have you tried parsing the json via protobuf?
ResourceSpans.Builder builder = ResourceSpans.newBuilder();
JsonFormat.parser().merge(json, builder);There was a problem hiding this comment.
Thank you, I couldn’t find a standard parser.
|
|
||
| private List<Span> parseSpans(String inputJson, Instant receivedTimeStamp) throws IOException { | ||
|
|
||
| ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
Is it necessary to recreate the ObjectMapper on each call? Have a look at my comment in the JsonOtelTracePojo for an alternative. Basically, I think you could do this instead:
ResourceSpans.Builder builder = ResourceSpans.newBuilder();
JsonFormat.parser().merge(json, builder);You can now use the builder to access all fields from the data. Essentially, this then requires the same mapping as in the trace source.
Furthermore, the protobuf approach would allow you easily to consume protobuf binary data in a future extension. Replacing JSON with protobuf will provide a considerably higher throughput.
There was a problem hiding this comment.
Unfortunately, when sending a ProtoBuf message to Kafka, it gets split into multiple parts. Even after merging them back together and using the OTelTraceDecoder.parse method, I still received an error indicating an invalid format. Therefore, we decided to send traces through Kafka in JSON format instead.
Description
Parsing of OpenTelemetry trace in JSON format into Span has been implemented to support trace handling via Kafka.
We had to implement it, as the client requires all data to be sent exclusively through Kafka.
To implement this, a new MessageFormat was added.
Issues Resolved
Resolves #5446
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.