metrics-rs / metrics

A metrics ecosystem for Rust.
MIT License
1.1k stars 151 forks source link

Allow inspection of PrometheusRecorder #387

Closed dfwilbanks395 closed 1 year ago

dfwilbanks395 commented 1 year ago

Hi! We've been using PrometheusRecorder to expose a metrics scrape endpoint from our application. We now want to inspect metrics from within our application, and we would like to be able to do this without calling PrometheusHandle::render() and parsing the output.

Would you be open to a PR to either allow direct access of recorder::Inner, or provide functions to obtain a snapshot of the metrics? We would like to be able to get a snapshot of just the counters or gauges or histograms and be able to provide an optional filter on Key rather than taking the full Snapshot.

dfwilbanks395 commented 1 year ago

We're currently using a fork with #388 to support our use case

tobz commented 1 year ago

After looking at the PR, I'm curious: what's the reason you want to access to the metrics when you're already exposing them externally?

dfwilbanks395 commented 1 year ago

Good question. We're building a database cache and store a lot of query execution metrics. Since those metrics are so important to the user experience, we want to show them in the console (below). Most users want to exclusively use the console for trying out and setting up the cache, whereas the prometheus endpoint is more useful in production.

In the future we might also query cache hit/miss, latency, and execution count metrics from our app to inform automatic cache evictions.

      query id      |    proxied query    | readyset supported | p50 (ms) | p90 (ms) | p99 (ms) 
--------------------+---------------------+--------------------+----------+----------+----------
 q_1b411ee325cb487  | SELECT "b" FROM "t" | yes                | 1.163    | 1.163    | 1.163
 q_e060331b8c7d2d88 | SELECT "a" FROM "t" | yes                | 1.081    | 1.081    | 1.081
tobz commented 1 year ago

Gotcha, makes sense.

In general, this has come up before and I'm against polluting exporters in this way.

I certainly understand the mindset -- "we're already tracking a bunch of metrics, it ought to be easy to get at a few of them programmatically" -- but it's a weird, off-label usage of the exporter when it's geared towards providing those metrics to a downstream system.

I tried to intentionally provide escape hatches for a lot of this "I want A and B" functionality -- such as layers, specifically ones like Fanout -- so that individual components could stay reasonably succinct and focused.

My recommendation here would be to consider looking at just installing a Registry<K, S> directly when in local/testing/console mode, and adding some logic around that to push bucketed histogram samples into your data structure of choice (HDRHistogram, DDSketch, basic/hard-coded histogram buckets). If you wanted to to expose both "console" metrics and Prometheus metrics (for example, in the potential future use case you describe), then something like Fanout could help write those metrics to both recorders/exporters.

A note: there's some theoretical universe where I/we/someone comes up with a way to layer an exporter over an existing registry such that the registry can be accessed directly out-of-band while sort of "feeding" the exporter that is wrapping it... thus avoiding having the data in two places. I don't really know exactly how that all might work but if this was something (the avoidance of duplicated data) that mattered deeply to you and you wanted to consider working on it, I would be willing to consider something along those lines.