Skip to content

Added parsing otel json to span from kafka topic to Span#5681

Open
Mamol27 wants to merge 9 commits intoopensearch-project:mainfrom
Mamol27:kafka-json-to-otel-trace
Open

Added parsing otel json to span from kafka topic to Span#5681
Mamol27 wants to merge 9 commits intoopensearch-project:mainfrom
Mamol27:kafka-json-to-otel-trace

Conversation

@Mamol27
Copy link
Copy Markdown

@Mamol27 Mamol27 commented May 6, 2025

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.

Sorry for creating new PR I've got a problem with solving DCO. (Old PR #5590)

Issues Resolved

Resolves #5446

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

Signed-off-by: mamol27 <mamol27@yandex.ru>
Copy link
Copy Markdown
Collaborator

@KarstenSchnitter KarstenSchnitter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing this PR. I did not review the tests. But I have some comments on the main implementation.

final long receivedTimeStamp = getRecordTimeStamp(consumerRecord, Instant.now().toEpochMilli());

KafkaOtelJsonTraceParser kafkaOtelJsonTraceParser = new KafkaOtelJsonTraceParser();
kafkaOtelJsonTraceParser.parse(consumerRecord.value().toString(), Instant.ofEpochMilli(receivedTimeStamp), (record) -> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkondaka: I think this implementation could be improved, if the type of consumerRecord.value() was known. If it was byte[], ByteBuffer or Bytes, it could be wrapped in an java.io.Reader implementation that avoids generating an intermediate string. The JsonFormat.parser().merge(...) would be fine with such a reader. Unfortunately, if I understand the code correctly, the type T is a free placeholder. Unless any valid assumptions on the type can be made, this improvement is not possible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an InputCodec as I suggested elsewhere would allow us to read from an InputStream. So you could have new ByteArrayInputStream(consumerRecord.value()).

Copy link
Copy Markdown
Author

@Mamol27 Mamol27 May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consumerRecord.value() isn't byte[] it has type T

Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Mamol27 for this great contribution!

processRecord(acknowledgementSet, record);
}
} else if (schema == MessageFormat.JSON_OTEL_TRACE) {
final long receivedTimeStamp = getRecordTimeStamp(consumerRecord, Instant.now().toEpochMilli());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this code not common to all reads?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it process many records instead one to other

final long receivedTimeStamp = getRecordTimeStamp(consumerRecord, Instant.now().toEpochMilli());

KafkaOtelJsonTraceParser kafkaOtelJsonTraceParser = new KafkaOtelJsonTraceParser();
kafkaOtelJsonTraceParser.parse(consumerRecord.value().toString(), Instant.ofEpochMilli(receivedTimeStamp), (record) -> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an InputCodec as I suggested elsewhere would allow us to read from an InputStream. So you could have new ByteArrayInputStream(consumerRecord.value()).

Signed-off-by: mamol27 <mamol27@yandex.ru>
Mamol27 added 2 commits May 14, 2025 11:01
Parsing with TracesData.Builder have some issuies

Signed-off-by: mamol27 <mamol27@yandex.ru>
Parsing with TracesData.Builder have some issuies

Signed-off-by: mamol27 <mamol27@yandex.ru>
@Mamol27
Copy link
Copy Markdown
Author

Mamol27 commented May 14, 2025

@dlvenable @KarstenSchnitter
image
All IT tests passed locally, is that GitHub issue? Could you please give me some advice?

Mamol27 added 4 commits June 4, 2025 10:57
# Conflicts:
#	data-prepper-plugins/kafka-plugins/src/main/java/org/opensearch/dataprepper/plugins/kafka/consumer/KafkaCustomConsumer.java
#	data-prepper-plugins/kafka-plugins/src/main/java/org/opensearch/dataprepper/plugins/kafka/consumer/KafkaCustomConsumerFactory.java
Signed-off-by: mamol27 <mamol27@yandex.ru>
@kkondaka
Copy link
Copy Markdown
Collaborator

kkondaka commented Dec 8, 2025

@Mamol27 sorry for the delay in reviewing this. Please address the comments.

Signed-off-by: mamol27 <mamol27@yandex.ru>
@Mamol27 Mamol27 requested a review from kkondaka December 11, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTel Trace from kafka source with otel_trace_raw

4 participants