Histogram's Max() and probably others about go-metrics HOT 8 CLOSED

I think #26 addresses this?

from go-metrics.

@dpiddy partially. Although Histogram.Max() is still broken in that branch.

from go-metrics.

This is indeed very similar to #26 and I'm still very torn. I think it's poor form for us to throw away accurate min/max data in order to agree with a purposefully inaccurate (optimized for size) sample.

@paulhammond pointed out that we could just do both. What do you guys (and @albanpeignier) think of exposing the Sample behind the histogram and adding Min(), Max(), and so on to the Sample interface?

from go-metrics.

I think there needs to be a way to get these stats on an interval basis. I just started playing with go-metrics along with go-datadog and seeing max for a metric only go up (modulo process restarts) is not ideal. Same with count.

Some ideas:

• Expose the Sample as you said
• Make the current methods sample/snapshot-based and add corresponding AllTime*() methods
• Make the source of the current methods tweakable per histogram, either from all-time data or a current snapshot

PS: go-metrics is really nice, thanks!

from go-metrics.

@rcrowley You really need to move it to the Snapshot to be consistent with other values you may read.

from go-metrics.

To clarify ... IMO reporting should work like this ...
<-tick
s := histogam.Snapshot()
s.Min()
s.Max()
s.p99()
etc...
send data...
then let s get garbage collected.

This nicely de-couples the Histogram / Samples / Snapshots.

If you don't do this ... additional samples can (and will) be added to the Sample between reading min/max/p99, etc, skewing results. The snapshot doesn't need to be thread safe either, so wouldn't require any mutexes.

I don't think this sacrifices anything and actually gives me accurate Min/Max/etc data given the snapshot in time of the histogram data.

Again, FWIW @codahale's metrics does it this way and IMO it's the "right way" (tm). ;-)

from go-metrics.

Every time you @-mention me, I get re-subscribed to this issue. I love you all, but I don't have anything to add to the discussion. :)

from go-metrics.

Closed by #32.

from go-metrics.