introduce MilliTimer #131
introduce MilliTimer #131
Conversation
This provides an API so that we can move away from microsecond-based timers. The StatsD spec suggests using milliseconds, and they're a great fit for the typical RPCs we are measuring. Most RPCs take about 1-1000ms, which makes it easy for a human to spot and tell the difference vs. microseconds which would be 1000us-1000000us. We don't need that amount of fidelity. Consumers can continue using NewTimer, using microseconds, which might be appropriate for other use cases like timing functions or tight in-memory queues. But most new callers should move toward MilliTimer functions.
|
Two questions/comments:
|
I think the consumer should have their own interface that they can change in that case. It's Not Great that we expose an interface in this package in the first place, to be honest. That is why interfaces should really be kept at the consumer as much as possible.
Some folks did some archaeology on this but the trail stops cold in 2016. Either way there's no great reason for it. |
| t.AddDuration(dur) | ||
| } | ||
|
|
||
| func (t *timer) AddDuration(dur time.Duration) { |
There was a problem hiding this comment.
silly nit: can this be below AddValue for consistency of ordering?
| } | ||
|
|
||
| func (t *timer) AddDuration(dur time.Duration) { | ||
| t.sink.FlushTimer(t.name, float64(dur/t.base)) |
There was a problem hiding this comment.
this is maybe a "i'm not a go programmer" piece of feedback, but the float64(dur/t.base) doesn't make a lot of sense unless you happen to know that:
- durations in go are actually nanoseconds
time.Millisecondandtime.Microsecondare the amount of nanoseconds in a ms or a µs
thoughts on adding a comment explicating what this does?
This provides an API so that we can move away from microsecond-based
timers.
The StatsD spec suggests using milliseconds, and they're a great fit
for the typical RPCs we are measuring. Most RPCs take about
1-1000ms, which makes it easy for a human to spot and tell the
difference vs. microseconds which would be 1000us-1000000us. We don't
need that amount of fidelity.
Consumers can continue using NewTimer, using microseconds, which might
be appropriate for other use cases like timing functions. But most new
callers should move toward MilliTimer functions.