metrics-rs / metrics

A metrics ecosystem for Rust.
MIT License
1.1k stars 151 forks source link

Add `record_many` to histogram #509

Open 0xdeafbeef opened 3 weeks ago

0xdeafbeef commented 3 weeks ago

I want to reexport tokio metrics, they already provide bucketed data. To reexport it, you should write such code:

 let hist = metrics::histogram!("tokio_poll_count_time");
  for (idx, value) in histogram.iter().enumerate() {
      let bucket_value = LOG_BUCKETS[idx];
      for _ in 0..*value {
          hist.record(bucket_value);
      }
   }

Is it possible to add

fn record_many(&self, value: f64, count: u64);

to HistogramFn to not burn cpu cycles? Or maybe there is a better way?

0xdeafbeef commented 3 weeks ago

Maybe even for backward compatibility

pub trait HistogramFn {
    /// Records a value into the histogram.
    fn record(&self, value: f64);

    fn record_many(&self, value: f64, count: u64) {
        for _ in 0..count {
            self.record(value);
        }
    }
}

I can open a pr if you agree with such an interface.

tobz commented 3 weeks ago

Overall, I'm fine with the idea of adding HistogramFn::record_many with the proposed method signature. I would definitely want the second version so that we can add it as a non-breaking change, and I would gladly accept a PR for that implementation. 👍🏻