-
Notifications
You must be signed in to change notification settings - Fork 0
Support "http" protocol #7
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
Conversation
430feaa
to
5460df6
Compare
#[cfg(feature = "integration_test")] | ||
#[cfg(feature = "test")] |
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.
Not sure if I should rename this, but it's only used by profiles right now, so it should be fine
hyper = { version = "0.14", default-features = false, features = ["http1", "client"], optional = true } | ||
hyper-v1 = { package = "hyper", version = "1.1", default-features = false, features = ["http1", "client"], optional = true } |
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.
opentelemetry-http
uses [email protected] and it's not really compatible with the latest one.
And we need hyper@1 for out test setup, but this one is hidden behind test
feature.
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.
Perhaps we should add an http
feature and hide opentelemetry-http
and hyper
behind it.
But it will complicate protocol detection quite a lot since we'll have to insert a lot of cfg
macros to handle cases when http support is disabled.
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.
It might take some time for hyper 1.x support and progress can be followed here open-telemetry/opentelemetry-rust#1427
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.
Yeah, looks like it.
I even tried to implement my own HyperClient
for the latest hyper version, but it didn't work since new hyper uses http@1, but opentelemetry-http
is written on top of [email protected] and http@0.
No description provided.