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

U64 and U128 Guage Types #470

Open
roshaans opened this issue Dec 5, 2022 · 7 comments
Open

U64 and U128 Guage Types #470

roshaans opened this issue Dec 5, 2022 · 7 comments

Comments

@roshaans
Copy link

roshaans commented Dec 5, 2022

There are currently two types of Guage types supported by this library. IntGauge supports i64 values and gauge supports f64 values. It would be great if there was a Guage type that supported u64 values as well as u128 values.

We currently cast our u64 value into an i64 type so we can use the IntGuage type. This approach can possibly throw an exception when the u64 value goes out of bounds and we would ideally like to avoid that.

We considered using the simple guage type which supports f64 values but there seems to be a lot more performance overhead in using that according to the docs here.

Regarding the need for a u128 type guage, we would like to store a timestamp in nanoseconds which requires this, but it would be better to prioritize the u64 type first.

@asomers
Copy link

asomers commented Dec 8, 2022

I have the same problem, though in my case I actually did overflow an i64. A u64 gauge would be perfect for me. Or, failing that, if IntCounter had a set method, that would work too.

@lucab
Copy link
Member

lucab commented Dec 8, 2022

All the Int* types offer some basic additional sugar, but by their own design they can't cover all possibilities. And handling all the possible cases with custom types will result in a unmaintainable matrix.

If your usecases do not fit in what the i64 helpers provide, then I'd recommend to use the usual f64 methods instead as they are actually meant to cover a larger set of cases.

That said, maybe there is a way to accommodate the u64 without adding a lot of new dedicated types, given that the AtomicF64 in this crate internally uses an atomic u64.

@asomers
Copy link

asomers commented Dec 8, 2022

For our my case, f64 would be even worse. I have a counter, and I need to measure the difference between adjacent values, so f64 would result in a big precision loss.

@lucab
Copy link
Member

lucab commented Dec 8, 2022

But that's not something that this type of instrumentation can provide, nor a problem that we can solve in this library.
At the bottom of all this, values only exist as f64 and Prometheus doesn't do integers.

@asomers
Copy link

asomers commented Dec 8, 2022

So does this crate already convert i64 types info f64 internally?

@lucab
Copy link
Member

lucab commented Dec 9, 2022

Yes:

pub fn metric(&self) -> Metric {
let mut m = Metric::default();
m.set_label(from_vec!(self.label_pairs.clone()));
let val = self.get();
match self.val_type {
ValueType::Counter => {
let mut counter = Counter::default();
counter.set_value(val.into_f64());
m.set_counter(counter);
}
ValueType::Gauge => {
let mut gauge = Gauge::default();
gauge.set_value(val.into_f64());
m.set_gauge(gauge);
}
}
m
}

But, once again, f64 is really how the whole Prometheus universe works.

@asomers
Copy link

asomers commented Dec 9, 2022

Lame. Well, that's what I'll do now. I'll just use the f64 Gauge type.

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