Closed bernd closed 11 years ago
The way Coda Hale's metrics library works is it has you register a callback to be used when a gauge is reported.
I think I would prefer that style. Does that make sense?
Yes I saw that in Coda's library and my first version of this diff actually used a callback to collect the gauge. Then I thought about all the things that can go wrong when using the callback based approach (blocking calls, thread-safety issues - the callback is executed in the reporter thread etc.) and decided to do a "safe" version first. ;)
I liked the callback approach as well so I'll go ahead and update the diff with the code I dropped.
Maybe it makes sense to support both...
On Thu, Feb 28, 2013 at 3:51 PM, Bernd Ahlers notifications@github.com wrote:
Yes I saw that in Coda's library and my first version of this diff actually used a callback to collect the gauge. Then I thought about all the things that can go wrong when using the callback based approach (blocking calls, thread-safety issues - the callback is executed in the reporter thread etc.) and decided to do a "safe" version first. ;)
I liked the callback approach as well so I'll go ahead and update the diff with the code I dropped.
Reply to this email directly or view it on GitHub: https://github.com/eric/metriks/pull/33#issuecomment-14265379
I added a callback method in a235d2e.
I thought some more about the API and I don't really like my first attempt with the #callback
method.
The following I like better.
# With block
gauge = Metriks.gauge('queue_size') { queue.size }
# With callable object
callable = proc { queue.size }
gauge = Metriks.gauge('queue_size', callable)
Any opinions?
I think that's a lot better. It matches how it works in coda hale's metrics and generally just looks nice and easy to use.
I changed the interface in 6f1802a.
I just had some minor comments, but I like this interface much better.
I changed the default value from 0 to nil.
Looks great! Thanks for working though this.
Thank you!
This adds gauges to record arbitrary values.
Please let me know what you think. Thanks!