rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

ACP: Add `impl Display` variant to print `Duration` as decimal seconds #351

Closed dead-claudia closed 4 months ago

dead-claudia commented 4 months ago

Proposal

Problem statement

There's no easy way to print a duration as seconds without potential loss of precision. One could use format!("{}", duration.to_secs_f64()), but that loses precision after about 3 months (experimentally, I found 8388608.000000001 seconds to be the first duration printed imprecisely, equating to 97 days, 2 hours, 10 minutes, 7 seconds, and 1 nanosecond).

Motivating examples or use cases

[Run this program to see what I mean](https://rust.godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:rust,selection:(endColumn:45,endLineNumber:4,positionColumn:45,positionLineNumber:4,selectionStartColumn:45,selectionStartLineNumber:4,startColumn:45,startLineNumber:4),source:'use+std::time::Duration%3B%0A%0Apub+fn+main()+%7B%0A++++let+duration1+%3D+Duration::new(10_000_000,+1)%3B%0A++++let+duration2+%3D+Duration::new(1,+1)%3B%0A%0A++++println!!(%22duration1:%22)%3B%0A++++println!!(%22++++debug:+%7B:%3F%7D%22,+duration1)%3B%0A++++println!!(%22++++f32:+++%7B%7D%22,+duration1.as_secs_f32())%3B%0A++++println!!(%22++++f64:+++%7B%7D%22,+duration1.as_secs_f64())%3B%0A++++println!!(%22duration2:%22)%3B%0A++++println!!(%22++++debug:+%7B:%3F%7D%22,+duration2)%3B%0A++++println!!(%22++++f32:+++%7B%7D%22,+duration2.as_secs_f32())%3B%0A++++println!!(%22++++f64:+++%7B%7D%22,+duration2.as_secs_f64())%3B%0A%7D'),l:'5',n:'0',o:'Rust+source+%231',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:executor,i:(argsPanelShown:'1',compilationPanelShown:'0',compiler:r1760,compilerName:'',compilerOutShown:'0',execArgs:'',execStdin:'',fontScale:14,fontUsePx:'0',j:1,lang:rust,libs:!(),options:'',source:1,stdinPanelShown:'1',wrap:'1'),l:'5',n:'0',o:'Executor+rustc+1.76.0+(Rust,+Editor+%231)',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4):

use std::time::Duration;

pub fn main() {
    let duration1 = Duration::new(10_000_000, 1);
    let duration2 = Duration::new(1, 1);

    println!("duration1:");
    println!("    debug: {:?}", duration1);
    println!("    f32:   {}", duration1.as_secs_f32());
    println!("    f64:   {}", duration1.as_secs_f64());
    println!("duration2:");
    println!("    debug: {:?}", duration2);
    println!("    f32:   {}", duration2.as_secs_f32());
    println!("    f64:   {}", duration2.as_secs_f64());
}

For context, 10 million seconds is about 115 days. While this might seem nonsensical for most, this could come up in Prometheus expositions, where the protocol expects durations to be printed in decimal seconds at millisecond precision, and nanosecond-resolution timestamps are a thing in the relevant spec.

Also, it'd be nice to have a pretty printer that didn't rely on the FPU, for low-end MCUs and just generally smaller, simpler code.

Solution sketch

Maybe a duration.display_seconds() that returns an impl Display that prints a nanosecond-precision timestamp. Assuming this helper in impl fmt::Debug for Duration was extracted out:

pub struct DisplaySeconds(Duration);

impl fmt::Display for DisplaySeconds {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        // Print leading '+' sign if requested
        let prefix = if f.sign_plus() { "+" } else { "" };
        fmt_decimal(f, self.0.secs, self.0.nanos.0, NANOS_PER_SEC / 10, prefix, "")
    }
}

impl Duration {
    pub fn display_seconds(&self) -> DisplaySeconds {
        DisplaySeconds(*self)
    }
}

Alternatives

One could also provide that via impl Display for Duration. The Debug implementation for >1s durations already does exactly what I'm proposing here, so there's some precedent already.

Assuming this helper in impl fmt::Debug for Duration was extracted out, it'd look like this:

impl fmt::Display for Duration {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        // Print leading '+' sign if requested
        let prefix = if f.sign_plus() { "+" } else { "" };
        fmt_decimal(f, self.secs, self.nanos.0, NANOS_PER_SEC / 10, prefix, "")
    }
}

Links and related work

The existing workhorse implementation for impl fmt::Debug for core::Duration implements 99% of this already. Only difference is it also writes a suffix that, for this, would need to be an empty string.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

kennytm commented 4 months ago

Metric values in OpenMetrics MUST be either floating points or integers. Note that ingestors of the format MAY only support float64.

If your use case is a metric value for Prometheus AFAIK it will be consumed as f64 anyway, so even if you print .display_seconds() in the most precise way what is scraped and parsed is still 10000000.000000002.

package main

import (
    "fmt"
    "strconv"
)

func main() {
    fmt.Println(strconv.ParseFloat("10000000.000000001", 64))
    // 1.0000000000000002e+07 <nil>
}

and for the "nanosecond timestamp"... Prometheus only supported millisecond-precision :shrug:

In any case printing a nanosecond timestamp that is compatible with OpenMetrics should be as simple as (assuming you're OK with the extra trailing zeroes):

format_args!("{}.{:09}", dur.as_secs(), dur.subsec_nanos())
m-ou-se commented 4 months ago

We briefly discussed this in the libs-api meeting and are happy to see display_seconds as an unstable experimental feature.

Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature.

Thanks!