Skip to content
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

Const Metrics / Custom Collectors #36

Open
phyber opened this issue Jan 20, 2022 · 10 comments
Open

Const Metrics / Custom Collectors #36

phyber opened this issue Jan 20, 2022 · 10 comments

Comments

@phyber
Copy link
Contributor

phyber commented Jan 20, 2022

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!

@mxinden
Copy link
Member

mxinden commented Jan 20, 2022

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

Happy that it is of some use ❤️

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.

@phyber
Copy link
Contributor Author

phyber commented Jan 20, 2022

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 #11.

@phyber phyber changed the title Const Metrics Const Metrics / Custom Collectors Jan 20, 2022
@phyber
Copy link
Contributor Author

phyber commented Jan 20, 2022

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.

@mxinden
Copy link
Member

mxinden commented Jan 24, 2022

Oh my bad. Thanks @phyber for raising this.

Would you mind giving #41 a review? It exposes the methods and adds an example on how to implement a custom metric type.

@mxinden
Copy link
Member

mxinden commented Feb 2, 2022

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

@phyber
Copy link
Contributor Author

phyber commented Feb 3, 2022

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. :)

@mxinden
Copy link
Member

mxinden commented Feb 3, 2022

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:

pub struct Family<S, M, C = fn() -> M> {
metrics: Arc<RwLock<HashMap<S, M>>>,

@mxinden
Copy link
Member

mxinden commented Aug 29, 2022

Cross referencing proposal here: #82

@phyber
Copy link
Contributor Author

phyber commented Aug 31, 2022

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.

@08d2
Copy link

08d2 commented Jan 24, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants