client_rust icon indicating copy to clipboard operation
client_rust copied to clipboard

Const Metrics / Custom Collectors

Open phyber opened this issue 3 years ago • 10 comments

Hi,

Thanks for creating an official Rust crate for Prometheus. :)

I'm wondering how we should handle Const Metrics with this crate. Are these supported at the moment? If they are, it's a little non-obvious.

My use case is for exporting counters from the operating system. The OS itself guarantees that these are always increasing (unless the counter wraps, etc), but they're difficult to export with only the inc and inc_by methods as additional variables must be maintained to work out the difference between the old OS counter and the current OS counter, so we can finally inc_by.

If Const Metrics are not currently supported, please consider this a feature request.

Thanks!

phyber avatar Jan 20 '22 10:01 phyber

Thanks for creating an official Rust crate for Prometheus. :)

Happy that it is of some use :heart:

My use case is for exporting counters from the operating system. The OS itself guarantees that these are always increasing (unless the counter wraps, etc), but they're difficult to export with only the inc and inc_by methods as additional variables must be maintained to work out the difference between the old OS counter and the current OS counter, so we can finally inc_by.

In this particular use-case, would a metric type called constant not be misleading? In other words, the use-case here produces a non-constant metric, thus one would not expect to be using a metric type with const in its name, no?

The crate allows you to access the inner (atomic) type of a Counter via Counter::inner which then allows you to set the value of the counter. The API contract though is, that you uphold the guarantees of a Prometheus Counter:

    /// Exposes the inner atomic type of the [`Counter`].
    ///
    /// This should only be used for advanced use-cases which are not directly
    /// supported by the library.
    ///
    /// The caller of this function has to uphold the property of an Open
    /// Metrics counter namely that the value is monotonically increasing, i.e.
    /// either stays the same or increases.
    pub fn inner(&self) -> &A {

With this little trick in mind, would the following work for you?

let counter: Counter = Counter::default();

// Register the counter with a registry.

// Update the counter value to represent the OS value.
counter.inner().store(os_value);

In addition to achieving your goal, your metric would also properly be tagged with the OpenMetrics Counter type.

If they are, it's a little non-obvious.

I agree that this is not obvious. On top of that, there is an additional solution to the same problem, namely to implement EncodeMetric on a custom type, though I would argue that is over-engineered for your use-case. I am happy for any suggestions to make this more intuitive, as a start, a simple example using the initial proposal above might suffice.

mxinden avatar Jan 20 '22 12:01 mxinden

In this particular use-case, would a metric type called constant not be misleading?

I agree, but that's the terminology in the official Go Prometheus client, see for example the mysql_exporter where it used for exporting counters from the database itself, or node_exporter where it's used for host CPU stats. The examples from the linked Const Metrics Golang documentation also mention the use case of using const metrics for external values.

It appears to be "const" because after you create it there are no methods to do fancy things (inc, set, etc), you just get an exportable metric that lasts some short period of time (probably until you're done with the scrape). This matches the comment for the type:

// NewConstMetric returns a metric with one fixed value that cannot be
// changed. Users of this package will not have much use for it in regular
// operations. However, when implementing custom Collectors, it is useful as a
// throw-away metric that is generated on the fly to send it to Prometheus in
// the Collect method.

The Python client calls them Custom Collectors which is probably a less confusing name, and also registers the custom collector with the registry. The Python version seems far clearer in intent and usage.

On top of that, there is an additional solution to the same problem, namely to implement EncodeMetric on a custom type, though I would argue that is over-engineered for your use-case.

That's actually a pretty interesting approach, and one I may try out. It would probably simplify my existing code quite a bit by taking care of a few weird edge cases.

For now, as long as rust counter.inner().store(value); works and doesn't mind the occasional reset (my interpretation of "A MetricPoint in a Metric's Counter's Total MAY reset to 0. If present, the corresponding Created time MUST also be set to the timestamp of the reset." leads me to believe it should be fine), then this should be good enough.

Thanks a lot for the discussion :)

Edit: Minor title update to reflect what this type of metric is called in other clients. I should have read more of the issues too, this is probably the same request as https://github.com/prometheus/client_rust/issues/11.

phyber avatar Jan 20 '22 13:01 phyber

Looking at the EncodeMetric stuff, an example in this area would be appreciated. I was trying to write one myself to contribute, but I'm bumping into issues while looking at how other types get encoded as they're able to use private methods from certain structs/traits.

For example, when looking at how existing Counters are encoded there's an issue due to no_bucket and encode_value being private. I'm unsure if those being private are bugs, or if I'm going about the encoding the wrong way.

phyber avatar Jan 20 '22 14:01 phyber

Oh my bad. Thanks @phyber for raising this.

Would you mind giving https://github.com/prometheus/client_rust/pull/41 a review? It exposes the methods and adds an example on how to implement a custom metric type.

mxinden avatar Jan 24 '22 17:01 mxinden

With #41 merged, can this crate support your use-case now @phyber?

mxinden avatar Feb 02 '22 20:02 mxinden

Possibly. I'm going to have to take a while and try to adapt it to something much closer to what I'm actually doing at the moment. My actual code that's going to use this is at jail_exporter/exporter.rs (this was my first real Rust project, apologies for the state of the code). This is an exporter for FreeBSD jails, it currently uses the tikv/prometheus crate.

Most of the metrics there will be easy to adapt, they're simple gauges and don't need anything fancy. The cputime_seconds_total and the wallclock_seconds_total are the metrics that require custom collectors, since I'm exporting OS counters.

I also need the ability to remove label sets from a metric family, I'm not sure that this is implemented yet. This is because jails may come and go while the exporter is running.

In the tests and some code there, you can see the horrific counter bookkeeping that this custom collector feature should help me avoid. :)

phyber avatar Feb 03 '22 00:02 phyber

I'm going to have to take a while and try to adapt it to something much closer to what I'm actually doing at the moment. My actual code that's going to use this is at jail_exporter/exporter.rs

Feel free to tag me on any pull request for a review in case I can be of some help.

I also need the ability to remove label sets from a metric family, I'm not sure that this is implemented yet. This is because jails may come and go while the exporter is running.

Correct. That is not supported today, though is easy to add as one only has to expose HashMap::remove on Family::metrics:

https://github.com/prometheus/client_rust/blob/16aa166225a54759db96431b73f9b6b5045e75bb/src/metrics/family.rs#L100-L101

mxinden avatar Feb 03 '22 17:02 mxinden

Cross referencing proposal here: https://github.com/prometheus/client_rust/pull/82

mxinden avatar Aug 29 '22 04:08 mxinden

Thanks, that proposal is looking good. I may also have a PR shortly for the HashMap::remove stuff to allow us to remove labels sets from a metric family.

phyber avatar Aug 31 '22 14:08 phyber

You definitely can't reset a counter to 0 arbitrarily :) A counter can only be incremented, it must never be decremented or set to an arbitrary value, resets to 0 are acceptable under the assumption that the process hosting that counter has restarted.

08d2 avatar Jan 24 '23 05:01 08d2