Skip to content

Feature/hyper instrumentation#188

Draft
Travispersson wants to merge 3 commits intomainfrom
feature/hyper-instrumentation
Draft

Feature/hyper instrumentation#188
Travispersson wants to merge 3 commits intomainfrom
feature/hyper-instrumentation

Conversation

@Travispersson
Copy link
Copy Markdown
Contributor

@Travispersson Travispersson commented Apr 17, 2026

  • instrument hyper senders for http1 and http2 in separate features.
  • from_parts now takes an UrlParts struct 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.. 🤷
  • removed url crate since reqwest already exports its url type

Comment on lines +17 to +24
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>,
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +3 to +4
//! This module instruments `hyper::client::conn::http1::SendRequest` and
//! `hyper::client::conn::http2::SendRequest`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to instrument hyper_util::client::legacy::Client as well, ideally behind an additional feature

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why we may want a separate feature is because it's guarded by the "client" feature in hyper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants