-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set time unit on metrics.Timer #610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Couple of small things before I can merge. The CI failures look unrelated, I'll try to fix those.
metrics/timer.go
Outdated
if d < 0 { | ||
d = 0 | ||
} | ||
t.h.Observe(d) | ||
} | ||
|
||
// Unit sets the timer time unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this a proper sentence with punctuation, and provide more detail, e.g.
Unit sets the unit of the float64 emitted by the timer. By default, the timer emits seconds.
metrics/timer_test.go
Outdated
@@ -31,3 +31,29 @@ func TestTimerSlow(t *testing.T) { | |||
t.Errorf("want %.3f, have %.3f", want, have) | |||
} | |||
} | |||
|
|||
func TestTimerUnit(t *testing.T) { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extraneous newline.
Thanks for your feedback @peterbourgon! I implemented the suggested changes, please have a further review. |
Thanks! Test failures are unrelated. |
Set time unit on metrics.Timer
Issue #608.