-
Notifications
You must be signed in to change notification settings - Fork 440
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 Jaeger Thrift HTTP exporter #926
Conversation
Codecov Report
@@ Coverage Diff @@
## main #926 +/- ##
==========================================
+ Coverage 95.43% 95.44% +0.02%
==========================================
Files 158 158
Lines 6749 6749
==========================================
+ Hits 6440 6441 +1
+ Misses 309 308 -1
|
otlp-http exporter uses the test http server provided as part of this repo for functional tests :
|
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. Thanks for adding the support.
exporters/jaeger/include/opentelemetry/exporters/jaeger/jaeger_exporter.h
Outdated
Show resolved
Hide resolved
|
||
bool isOpen() const override; | ||
|
||
uint32_t read(uint8_t *buf, uint32_t len); |
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.
Do we need to provide read
as it is not used?
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.
The base Transport class (TTransport::read_virt
) will throw if this is invoked and I wasn't sure if the Thrift binary protocol will actually call this method, probably not, but just to be on the safe side 😨
Adds capability to upload Thrift over HTTP for Jaeger exporter.
Not sure how to automatically test it since we don't have integration tests anywhere, or do we? The UDP exporter is pretty much in the same state.
CHANGELOG.md
updated for non-trivial changes