Skip to content

Conversation

@asaaki
Copy link
Contributor

@asaaki asaaki commented Dec 6, 2020

Those metrics then can be consumed via the response extensions.
They contain very intersesting request timing data, useful for stats, tracing, benchmarks and similar use cases.

let res: http_types::Response = some_req.await?;
if let Some(metrics) = &res.ext::<isahc::Metrics>() {
  // use the metrics ...
}

@asaaki asaaki marked this pull request as ready for review December 6, 2020 16:01
@Fishrock123
Copy link
Member

Don't these need to be manually enabled?

@Fishrock123
Copy link
Member

We probably don't want them on all the time, so this should either have some kind of api or a configuration flag if this crate is going to be turning them on.

Or are these available if you http_client::isahc::IsahcClient::from_client()?

@asaaki
Copy link
Contributor Author

asaaki commented Dec 8, 2020

The metrics have to be manually enabled when building the client, so they are not always populated.
That's why it uses the let binding, because .metrics() returns an option based on the enablement of this feature.

An example how to create a surf client with isahc's metrics enabled:
https://github.com/asaaki/opentelemetry-surf/blob/d23ea22991c92abab8ce0b24c6d1f0f2c13c1689/examples/metrics.rs#L34-L45

So yes, only via http_client::isahc::IsahcClient::from_client() works. The default client would not have metrics at all.

Those metrics then can be consumed via the response extensions.

```rust
let res: http_types::Response = some_req.await?;
if let Some(metrics) = &res.ext::<isahc::Metrics>() {
  // use the metrics ...
}
```
@asaaki
Copy link
Contributor Author

asaaki commented Dec 28, 2020

Hi there,
is anything blocking the merge?
Can I do something to unblock it?

@Fishrock123
Copy link
Member

Note: a new release is kinda blocked on figuring out http-rs/surf#271 atm.

@Fishrock123 Fishrock123 merged commit a9928a0 into http-rs:main Jan 19, 2021
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