-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add grpc storage plugin OTEL exporter #2229
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2229 +/- ##
==========================================
+ Coverage 96.16% 96.18% +0.01%
==========================================
Files 219 219
Lines 10632 10635 +3
==========================================
+ Hits 10224 10229 +5
+ Misses 352 351 -1
+ Partials 56 55 -1
Continue to review full report at Codecov.
|
Signed-off-by: Pavol Loffay <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just a naming issue to ensure distinct from the gRPC used in the jaeger exporter.
Might be good to get someone familiar with the grpc plugin to also review.
@@ -28,6 +28,7 @@ import ( | |||
|
|||
"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/cassandra" | |||
"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/elasticsearch" | |||
"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/grpc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor, but think the exporter should be in package grpc-plugin
to avoid confusion with the jaeger-grpc protocol exporter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, I will refactor it
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Resolves #2158