-
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
[opentelemetry-otlp] adds an example HTTP exporter backed by Hyper #1861
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1861 +/- ##
=====================================
Coverage 75.0% 75.0%
=====================================
Files 122 122
Lines 20279 20279
=====================================
+ Hits 15211 15212 +1
+ Misses 5068 5067 -1 ☔ View full report in Codecov by Sentry. |
3f585e8
to
384f99b
Compare
From 6/4/ community call: |
@markdingram Would you be willing to change this based on this recommendation? Thanks in advance! |
5816ba9
to
2a8540b
Compare
I had a go |
@@ -52,6 +52,14 @@ Run the app which exports logs, metrics and traces via OTLP to the collector | |||
cargo run | |||
``` | |||
|
|||
|
|||
By default the app will use a `reqwest` client to send. A hyper 0.14 client can be used with the `hyper` feature enabled |
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.
By default the app will use a `reqwest` client to send. A hyper 0.14 client can be used with the `hyper` feature enabled | |
By default the app will use a `reqwest` client to send. A hyper client can be used with the `hyper` feature enabled |
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.
Thanks!
Will wait for one more approval to merge. (My own experience with hyper is limited, so need an extra pair of eyes)
@@ -0,0 +1,45 @@ | |||
use async_trait::async_trait; |
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 you think we can instead update the HttpConfig in opentelemetry-otlp to support the hyper client, and so use the hyper related code from opentelemetry-http. I don't know if hyper 1.0 is the blocker for that?
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.
@markdingram please see if this makes sense - #1861 (comment)
Anything done now will conflict with #1674 - I will look again once the crate upgrades are through. |
should be good, assuming we will revisit this once #1674 is through. |
Resolves #1659
Given the Pending Hyper 1.0 upgrade seems to make sense for Hyper 0.14 Client support to be provided as an example.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes