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

Summary is calculated wrong? #78

Open
pjarmalavicius opened this issue Nov 26, 2021 · 3 comments
Open

Summary is calculated wrong? #78

pjarmalavicius opened this issue Nov 26, 2021 · 3 comments

Comments

@pjarmalavicius
Copy link

I have the following samples [1.33, 2.33, 2.33].
Summary metric is returned like this:

some_duration_seconds{quantile="0.01"} 1.35
some_duration_seconds{quantile="0.05"} 1.43
some_duration_seconds{quantile="0.5"} 2.33
some_duration_seconds{quantile="0.95"} 2.33
some_duration_seconds{quantile="0.99"} 2.33
some_duration_seconds_sum 5.99
some_duration_seconds_count 3

Compared results with Go prometheus client and it return different result

some_duration_seconds{quantile="0.01"} 1.33
some_duration_seconds{quantile="0.05"} 1.33
some_duration_seconds{quantile="0.5"} 2.33
some_duration_seconds{quantile="0.95"} 2.33
some_duration_seconds{quantile="0.99"} 2.33
some_duration_seconds_sum 5.99
some_duration_seconds_count 3

Also tried some online percentile calculators:

So, I assume there is something wrong on PHP client

@mp3000mp
Copy link
Contributor

Hi, this is because the quantile is calculated using a linear interpolation when the quantile is between two values.

But to be honest I implemented the function this way because I found a source on the php.net documentation (as mentionned here) and I didn't even know there were more than one way to calculate quantiles !

English wikipedia mentions 9 different ways to calculate it !!!

So the questions are :

  • Which one should we use ? (i would be in favor of aligning with other libraries)
  • Should we leave the choice with an option ?

@LKaemmerling what do you think ?

@LKaemmerling
Copy link
Member

Hey,

I guess we should follow the same way as the go client. @mp3000mp do you want to prepare the change :)? I guess we can do it as a bug fix. If not, I will try to come up with a fix in the next two weeks.

@mp3000mp
Copy link
Contributor

mp3000mp commented Dec 8, 2021

Hey,

I guess we should follow the same way as the go client. @mp3000mp do you want to prepare the change :)? I guess we can do it as a bug fix. If not, I will try to come up with a fix in the next two weeks.

Thanks for the answer, I've just submitted the PR #81.

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

No branches or pull requests

3 participants