prometheus_client_php icon indicating copy to clipboard operation
prometheus_client_php copied to clipboard

Summary is calculated wrong?

Open pjarmalavicius opened this issue 2 years ago • 3 comments

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:

  • https://goodcalculators.com/percentile-calculator/ - same result as on Go client
  • https://www.dcode.fr/numbers-quantile - same result as on Go client

So, I assume there is something wrong on PHP client

pjarmalavicius avatar Nov 26 '21 15:11 pjarmalavicius

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 ?

mp3000mp avatar Nov 30 '21 20:11 mp3000mp

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.

LKaemmerling avatar Dec 01 '21 05:12 LKaemmerling

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.

mp3000mp avatar Dec 08 '21 18:12 mp3000mp