metrics-clojure icon indicating copy to clipboard operation
metrics-clojure copied to clipboard

3-arity version of `metrics.timers/timer` is not implemented

Open sebastianpoeplau opened this issue 7 years ago • 2 comments

The arity-3 version of metrics.timers/deftimer tries to call timer with 3 arguments, which is not implemented. I guess it should either be dropped or call timer-with-reservoir. On a related note, is there a reason why timer passes the title through metrics.core/metric-name while timer-with-reservoir doesn't?

sebastianpoeplau avatar Oct 27 '16 12:10 sebastianpoeplau

I doubt there is a reason.

Feel free to look into a pull request.

michaelklishin avatar Oct 27 '16 12:10 michaelklishin

@michaelklishin I'm not sure if an arity-3 version of metrics.timers/timer makes sense: callers would create a new reservoir in vain on every call but the first. I think deftimer should rather delegate to metrics.timers/timer-with-reservoir in this case. Regarding naming, I can certainly add the call to metric-name in timer-with-reservoir, but that would not be a backwards-compatible change. Duplicating the function just for the sake of keeping naming consistent in deftimer doesn't seem elegant either, though. Do you have any preference?

sebastianpoeplau avatar Nov 01 '16 11:11 sebastianpoeplau