Feature/hyper instrumentation#188
Conversation
| pub(crate) struct UrlParts { | ||
| pub(crate) full_url: Option<String>, | ||
| pub(crate) path: String, | ||
| pub(crate) host: Option<String>, | ||
| pub(crate) scheme: Option<String>, | ||
| pub(crate) port: Option<u16>, | ||
| pub(crate) query: Option<String>, | ||
| } |
There was a problem hiding this comment.
The issue here is that hyper uses http::Uri, while reqwest relies on url::Url instead.
I suggest we define some trait to represent any Url-like structure and then implement it for both.
The trait should just return all those values in some way (probably as &str).
I can do it if you want.
| //! This module instruments `hyper::client::conn::http1::SendRequest` and | ||
| //! `hyper::client::conn::http2::SendRequest`. |
There was a problem hiding this comment.
We'll have to instrument hyper_util::client::legacy::Client as well, ideally behind an additional feature
There was a problem hiding this comment.
The reason why we may want a separate feature is because it's guarded by the "client" feature in hyper
There was a problem hiding this comment.
yeah sure! I can do it after you merge #189
| //! HTTP client instrumentation utilities. | ||
|
|
||
| #[cfg(feature = "reqwest")] | ||
| #[cfg(any(feature = "reqwest", feature = "hyper-http1", feature = "hyper-http2"))] |
There was a problem hiding this comment.
We have the same condition in src/instrumentations/mod.rs.
We should probably only keep one.
There was a problem hiding this comment.
For example:
| #[cfg(any(feature = "reqwest", feature = "hyper-http1", feature = "hyper-http2"))] |
from_partsnow takes anUrlPartsstruct instead because the url types used in reqwest and hyper differ. Maybe you rather want us to parse from one to the other type instead.. 🤷