search

AlexanderThaller / prometheus_exporter

Helper libary to export prometheus metrics using tiny_http and rust-prometheus.
MIT License
28 stars 10 forks source link

Allow using a custom registry #26

Closed girstenbrei closed 2 years ago

girstenbrei commented 2 years ago

Greets! first of all, awesome work! This lib is my favorite way (across multiple languages, but especially in Rust) to write exporters. Just does exactly what i want.

One nice to have, would be #14 . My use case is to configure an application prefix, which is currently only possible by 'manually' adding it to every metric. So, I started working on this and wanted to get some feedback. This implementation should work without braking any API. But, the currently global request metrics from the internal_metrics feature won't show up, as those are registered with the default registry, of course.

One solution is, to move those, maybe combined with the registry, as private fields inside the server struct. We could then reference it in handler_metrics and process_request, initializing the internal metrics in the Server::start. Another one would be to use a new struct, just for passing this "bunch of metrics related things" around.

What do you think?

AlexanderThaller commented 2 years ago

Hello. Sorry for the delay. The pull-request looks good to me. Thanks for contributing.

AlexanderThaller commented 2 years ago

I think one cargo +nightly fmt --all would be needed for the new example.

girstenbrei commented 2 years ago

Hey there, no problem. So I added the docs line and ran formatting.

Regarding the internal_metrics feature: I added some documentation under the features section and the custom registry setter. This should make it clear, that this is not yet working.

I would really like to make the two work together, but this might be a bigger change, just because the metrics need to be passed through to where they are needed. Maybe better to do this in a separate change.

Thanks for your work! Christoph

AlexanderThaller commented 2 years ago

I think you can merge it now. Thank you for the contribution!

girstenbrei commented 2 years ago

I think you can merge it now. Thank you for the contribution!

Sounds good! But I can't merge, no write access to this repo, so feel free to merge! Thx for creating the enhancement issue regarding the custom registry.

Very happy about the contribution!