Skip to content
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

Feature: Added Support accepting OTLP via Kafka #4299

Closed
wants to merge 0 commits into from

Conversation

metapox
Copy link

@metapox metapox commented Mar 13, 2023

Which problem is this PR solving?

Short description of the changes

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

please add tests for all code changes

Comment on lines 44 to 48
newSpan := &model.Span{}
spans := []*model.Span{newSpan}
err := proto.Unmarshal(msg, newSpan)
return newSpan, err

return spans, err
Copy link
Member

Choose a reason for hiding this comment

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

	newSpan := &model.Span{}
	err := proto.Unmarshal(msg, newSpan)
	return []*model.Span{newSpan}, err

@@ -34,6 +34,10 @@ const (
EncodingProto = "protobuf"
// EncodingZipkinThrift is used for spans encoded as Zipkin Thrift.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// EncodingZipkinThrift is used for spans encoded as Zipkin Thrift.
// EncodingZipkinThrift is used for spans encoded as Zipkin Thrift array.

cmd/ingester/app/processor/span_processor.go Outdated Show resolved Hide resolved
// EncodingOtlpJSON is used for spans encoded as OTLP JSON.
EncodingOtlpJSON = "otlp-json"
// EncodingOtlpProto is used for spans encoded as OTLP Proto.
EncodingOtlpProto = "otlp-proto"
Copy link
Member

Choose a reason for hiding this comment

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

add to description of encoding flag

@@ -34,6 +34,10 @@ const (
EncodingProto = "protobuf"
// EncodingZipkinThrift is used for spans encoded as Zipkin Thrift.
EncodingZipkinThrift = "zipkin-thrift"
// EncodingOtlpJSON is used for spans encoded as OTLP JSON.
Copy link
Member

Choose a reason for hiding this comment

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

clarify which Protobuf type is expected in the Kafka message

Copy link
Author

@metapox metapox Mar 16, 2023

Choose a reason for hiding this comment

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

@metapox metapox marked this pull request as ready for review March 19, 2023 08:46
@metapox metapox requested a review from a team as a code owner March 19, 2023 08:46
@metapox metapox requested a review from vprithvi March 19, 2023 08:46
@metapox
Copy link
Author

metapox commented Mar 19, 2023

@yurishkuro
thank you for review.
Can you please review it again?

@metapox metapox changed the title Kafka otlp Feature: Added Support accepting OTLP via Kafka Mar 19, 2023
@metapox metapox requested review from yurishkuro and removed request for vprithvi March 20, 2023 23:20
@yurishkuro
Copy link
Member

please resolve merge conflicts

@metapox
Copy link
Author

metapox commented Mar 23, 2023

Sorry,,,,
By mistake, I synchronized this branch with the main branch....

A new PR has been created because a PR with no commits cannot be reopened.
#4333

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.

2 participants