axum icon indicating copy to clipboard operation
axum copied to clipboard

[axum-extra] `metrics` integration?

Open oxalica opened this issue 1 year ago • 3 comments

  • [x] I have looked for existing issues (including closed) about this

Feature Request

Motivation

In #671, we added an example to manually instrument a single endpoint to support metrics. But the boilerplate is quite large for a middleware that wraps all requests track about request or response body, eg. body size, time-to-first-byte, time of round-trip and etc.

Proposal

Add a feature-gated module metrics in axum-extra to provide a MetricLayer middleware that instrument all requests and responses with metrics, maybe in a configurable way.

Since the middleware only does instrumentation, it only depends on metrics crate and is generic over export methods. It could be done in a few hundreds of lines, and users are free to customize export methods on their own: push metrics elsewhere, expose prometheus scrape endpoint as a route, or listen on another port for metrics.

This solution contrasts to axum-prometheus which is strongly coupled with prometheus exporter and does many hard-coded non-trivial things (e.g. always listen on localhost:9000).

Alternatives

  1. Do this in a separated crate like axum-metrics. This may be better depending on how many users want this feature.
  2. Do this in tower-http::metrics. This would not be easy to use since we almost always want a MatchedPath label. We could split the instrumentation with a generic trait ExtractLabels and impl ExtractLabels in axum-extra. But I doubt whether the increased complexity worth it, and this requires more boilerplate for axum users.

oxalica avatar Oct 14 '24 13:10 oxalica

Can you check again https://github.com/Ptrskay3/axum-prometheus?

I'm using since years, and it's using metrics, and does not force using localhost:9000. You put the route("/metrics", get(|| async move { metric_handle.render() })) where you want.

yanns avatar Oct 14 '24 13:10 yanns

Can you check again https://github.com/Ptrskay3/axum-prometheus?

I'm using since years, and it's using metrics, and does not force using localhost:9000. You put the route("/metrics", get(|| async move { metric_handle.render() })) where you want.

:thinking: Ah, seems I'm encountering https://github.com/Ptrskay3/axum-prometheus/issues/66 because axum-prometheus enables http-listener feature of metrics-exporter-prometheus unconditionally. I'm not sure which crate is to blame.

But that does not change the point though, axum-prometheus is more specialized than what I'm proposed here. This issue is more like: to upstream generic instrumentation code into axum-extra.

oxalica avatar Oct 14 '24 13:10 oxalica

axum-prometheus — despite its name — is not tightly coupled with Prometheus, it's rather centered around metrics.rs. You can just disable the default features, and bring your own exporter.

And if you plan to use Prometheus, then the http-listener feature is explicitly enabled, because .. well, you have to use something, and I assume that's the first thing people reach for. Let's note that metrics-exporter-prometheus also has that feature enabled by default (and also push-gateway).

https://github.com/Ptrskay3/axum-prometheus/issues/66 actually comes from an unfortunate API in metrics-exporter-prometheus itself.. you may read more about it here: https://github.com/metrics-rs/metrics/issues/501

However, you are not stuck with it, it's still possible to drop down lower level APIs of axum-prometheus, and avoid the issue yourself, using metrics_exporter_prometheus::PrometheusBuilder

Based on the discussion at this issue, I have a feeling that you're proposing here is probably 80-85% the same thing that axum-prometheus already provides.

Also tangentially related: https://github.com/Ptrskay3/axum-prometheus/issues/66#issuecomment-2411943117

Ptrskay3 avatar Oct 14 '24 17:10 Ptrskay3

I don't think I'd want to integrate non-trivial features like this in axum-extra. metrics seem complex enough to be a separate crate.

jplatte avatar Nov 07 '24 19:11 jplatte

For one who may be interested, I published axum-metrics as a miniature middleware implementation, which is only about ~300 LoC.

oxalica avatar Nov 08 '24 14:11 oxalica