Repository navigation
Change from dtoa to zmij - #369
BentEngbers wants to merge 1 commit into
Conversation
Signed-off-by: Bent Engbers <bentengbers@dreamsolution.nl>
2fd9523 to
bd61497
Compare
krisztianfekete
left a comment
krisztianfekete
left a comment
There was a problem hiding this comment.
Thanks,
Before merging:
- #368 is about to land and adds a few dtoa calls in
prometheus_text.rs. Could you rebase after it merges and switch those to zmij too? - Please add a test with fixed expected strings (see my inline comment).
- Please add a CHANGELOG.md line under Changed saying float output now uses the shortest form, so some f64 label values (including le) can look different.
I'd like to ship this together with #368 in 0.26.
| impl EncodeLabelValue for f64 { | ||
| fn encode(&self, encoder: &mut LabelValueEncoder) -> Result<(), std::fmt::Error> { | ||
| encoder.write_str_unescaped(dtoa::Buffer::new().format(*self)) | ||
| encoder.write_str_unescaped(zmij::Buffer::new().format(*self)) |
There was a problem hiding this comment.
This is also used for histogram le labels, so some label strings will change.
For example, with linear_buckets(0.1, 0.1, 20):
- today:
le="0.30000000000000007" - with this PR:
le="0.30000000000000004"
Very small and very large values change too: 0.000001 becomes 1e-6, and 10000000000000000.0 becomes 1e+16.
The number is the same, but the label is different, so Prometheus sees a new series. I think the change is fine. We just need to tell users (see my top comment).
| + "# UNIT my_counter_with_exemplar_seconds seconds\n" | ||
| + "my_counter_with_exemplar_seconds_total 2 # {user_id=\"99\"} 1.0 " | ||
| + dtoa::Buffer::new().format(now.duration_since(UNIX_EPOCH).unwrap().as_secs_f64()) | ||
| + zmij::Buffer::new().format(now.duration_since(UNIX_EPOCH).unwrap().as_secs_f64()) |
There was a problem hiding this comment.
This test builds the expected value with zmij too, so it can't catch a format change.
Could you add one small test with fixed expected strings? For example: 1e-6, 1e16, 0.1 + 0.2, and one le from linear_buckets. Then a future zmij update can't change our output without us noticing.
|
#368 is merged now. Could you rebase on master? |
Both crates are written by dtolnay:
https://github.com/dtolnay/dtoa
https://crates.io/crates/dtoa
https://github.com/dtolnay/zmij
https://crates.io/crates/zmij
Zmij seems more commonly used and should be faster According to dtolnay's own benchmarks (although running the benchmarks in this project on my machine did not show any significant changes). Zmij was already part of this project (See changes in the Cargo.lock).