One thing that nagged at me though was the introduction of the .reverse.each in the fill method ... my hunch was that this reverse (needed so we can efficiently bail out of the fill loop) could be done once in the constructor instead of on each observation; then I googled a little more and realized that instead of keeping a reversed copy of the buckets around we could just use reverse_each. It's only a little faster but I figure in instrumentation code that could be running for a lot of observations in a busy system, every little bit helps.
SamSaffron
commented
11 months ago
Was looking at https://github.com/discourse/prometheus_exporter/pull/233, which is great ... thanks @clupprich! I should have looked at the spec and I hope it didn't cost you too much time.
One thing that nagged at me though was the introduction of the
.reverse.eachin the fill method ... my hunch was that this reverse (needed so we can efficiently bail out of the fill loop) could be done once in the constructor instead of on each observation; then I googled a little more and realized that instead of keeping a reversed copy of the buckets around we could just usereverse_each. It's only a little faster but I figure in instrumentation code that could be running for a lot of observations in a busy system, every little bit helps.Also in this PR: I noticed that the toc link for histogram custom buckets wasn't working because it should point to https://github.com/discourse/prometheus_exporter#histogram--custom-buckets (github keeps the hyphen that's already in the header text).
Hope this is useful. Thanks!