Histogram bucket helpers #177
Conversation
lawrencejones
force-pushed
the
lawrence-add-histogram-helpers
branch
from
9fdbdaf
to
d872743
Compare
|
FYI @dmagliola |
|
Any love @Sinjo? |
lib/prometheus/client.rb
Outdated
|
|
||
| def self.exponential_buckets(start:, factor: 2, count:) | ||
| count.times.map { |idx| start.to_f * factor ** idx } | ||
| end |
There was a problem hiding this comment.
I love this feature, it was actually on our TO-DO list, so glad you tackled it.
I'm not sure about the location of these two methods, though...
Could you move these to be static methods in the Histogram class?
Also, should we validate the parameters, like the Go client does?
There was a problem hiding this comment.
Perhaps validating is an idea? Will have a look at that.
On the class method part, is it worth moving them there? Bearing in mind that would be different from the other clients.
There was a problem hiding this comment.
I think it's worth moving.
That's where I would look for them as a user of this library.
And unless I misread the Go code, that's where they are in the Go client too.
There was a problem hiding this comment.
Ah, I did misread the Go code. I saw it in Histogram.go and assumed it'd be there.
I think the conventions in these two languages are different.
Whereas you do NewHistogram in Go, you go Histogram.new in Ruby. Everything Histogram related is in the Histogram class itself.
The Client class is only used once for setup on startup, or not at all. It doesn't look like a place where someone would look when creating a histogram.
On that note, though... Could we also add a little section in the README documenting these two methods? :D
There was a problem hiding this comment.
Having them on Histogram would definitely match my expectations when writing Ruby. I think going with typical idiom within the language is a very fair shout. In client_java they're chainable methods on the Histogram builder.
|
Guess who forgot to specifically set up notifications for all activity on this project 🤦🏻♂️ Didn't realise when we were working super actively on 1.0 and only thought to go looking after you tagged me! Gonna look over this now. |
Most client libraries provide helpers for generating linear/exponential histogram buckets. This provides the same interface as the golang client. Signed-off-by: Lawrence Jones <lawrjone@gmail.com>
dmagliola
force-pushed
the
lawrence-add-histogram-helpers
branch
from
d872743
to
844e451
Compare
|
Ok, i've made the changes I mentioned above (moving the helper methods to the |
Most client libraries provide helpers for generating linear/exponential
histogram buckets. This provides the same interface as the golang
client.