-
Notifications
You must be signed in to change notification settings - Fork 93
Example metrics server using Hyper #94
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
Example metrics server using Hyper #94
Conversation
9400593
to
5021289
Compare
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 for the help. Very much appreciated.
Response::builder() | ||
.header( | ||
hyper::header::CONTENT_TYPE, | ||
"application/openmetrics-text; version=1.0.0; charset=utf-8", |
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.
🙏
examples/hyper.rs
Outdated
loop { | ||
EXAMPLE_COUNTER.inc(); | ||
tokio::time::sleep(Duration::from_secs(1)).await; | ||
} |
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.
This is a neat idea, though for the sake of consistency with the other examples, I would prefer showcasing a standard HTTP request counter in this example.
See tide example:
Lines 47 to 57 in 12c1de5
#[derive(Clone, Hash, PartialEq, Eq, Encode)] | |
struct Labels { | |
method: Method, | |
path: String, | |
} | |
#[derive(Clone, Hash, PartialEq, Eq, Encode)] | |
enum Method { | |
Get, | |
Put, | |
} |
I think an example should showcase a single thing only. (Though it is hard where the boundaries of a thing are.) I am fine for something like the above to be shown in a separate example.
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.
I agree that examples should only show a single thing. From my point of view, having an example counter simplifies the example, because this is really just teaching users how to serve a metrics endpoint using Hyper. It's not really about how metrics are created and used, just about serving them.
53f15ad
to
99c62f4
Compare
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.
Just two comments. Otherwise looks good to me. Thanks for the follow-ups.
examples/hyper.rs
Outdated
async fn main() { | ||
let request_counter = Counter::default(); | ||
|
||
let registry = register_metrics(&request_counter); |
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.
I find it confusing that the logic for registering a metric is extracted into a separate function. It is only called once. Say one would have more than one metric to register, the function would register all of them under the same name.
Can you expand on the rational for the extraction? With what I understand today, I would prefer for it to be removed.
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.
I removed the function, you're right
examples/hyper.rs
Outdated
Box::pin(async move { | ||
let mut buf = Vec::new(); | ||
encode(&mut buf, ®.clone()).map(|_| { | ||
let body = std::str::from_utf8(buf.as_slice()).unwrap().to_string(); |
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.
Given that one can build a Body
from a [u8]
why is the conversion to a string necessary here?
https://docs.rs/hyper/latest/hyper/struct.Body.html#impl-From%3C%26%27static%20%5Bu8%5D%3E-for-Body
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.
Ah great point, thanks for pointing that out. Simplified it.
Signed-off-by: Adam Chalmers <[email protected]>
6d909c6
to
c73f09c
Compare
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 for the help. Thanks for making it easier for others to get started.
Say your Rust app is not a web server. You probably don't need to pull in a whole web framework like Axum or Actix-web just to serve one little OpenMetrics endpoint. You can just use Hyper instead. Here's an example of how to do that.
I also added this an example target in Cargo.toml, so that you can run it with cargo: