Skip to content
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

add ExponentialBucketsRange function #899

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

sbunce
Copy link
Contributor

@sbunce sbunce commented Aug 4, 2021

This function calculates exponential buckets with different arguments
than the existing ExponentialBuckets function. Instead of specifying the
start and factor, the user can specify the min and max bucket value. We
have been doing it this way internally at my company for some time.

This function calculates exponential buckets with different arguments
than the existing ExponentialBuckets function. Instead of specifying the
start and factor, the user can specify the min and max bucket value. We
have been doing it this way internally at my company for some time.

Signed-off-by: Seth Bunce <[email protected]>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it, thank you for your contribution!

@bwplotka bwplotka merged commit 2261d5c into prometheus:master Aug 12, 2021
@dswarbrick
Copy link
Contributor

dswarbrick commented Sep 12, 2022

For the record, tests which expect bit-identical floating points will probably fail on s390x and possibly some other less mainstream archs, due to subtle differences in Go's floating point implementation on those archs, i.e. https://go.dev/ref/spec#Floating_point_operators, https://groups.google.com/g/golang-dev/c/Sti0bl2xUXQ

I'm seeing the exponential bucket range test fail on Debian's s390x buildds, and I suspect that it's for the same reason that this fails gohugoio/hugo#6387

=== RUN   TestBuckets
    histogram_test.go:364: exponential buckets range: got [1 1.6681005372000588 2.782559402207125 4.641588833612779 7.742636826811271 12.91549665014884 21.54434690031884 35.93813663804628 59.94842503189411 100.00000000000003],
                                                     want [1 1.6681005372000588 2.782559402207125 4.641588833612779 7.742636826811273 12.915496650148842 21.544346900318846 35.93813663804629 59.94842503189414 100.00000000000007]
--- FAIL: TestBuckets (0.00s)

@sbunce
Copy link
Contributor Author

sbunce commented Sep 13, 2022

@dswarbrick
I did not know that! Thank you for educating me. I created a change which switches to a comparison with a tolerance "abs(a-b)<tolerance". The tolerance I picked "0.001" looks higher than the differences in your test failure output.

The PR is here.
#1133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants