PromPHP / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
415 stars 91 forks source link

Histogram not working as expected #137

Closed rlehner-dk closed 9 months ago

rlehner-dk commented 9 months ago

In short, the histogram metric adds the observation to the bucket instead of filling a new bucket. For more details, please refer to the example:

Technology Used version
promphp/prometheus_client_php v2.7.2
Adapter InMemory
PHP 8.2

Registration of the metric

$registry = new \Prometheus\CollectorRegistry(new InMemory());
$counter = $registry->getOrRegisterHistogram(
    'namespace',
    'name',
    'help',
    ['label'],
    ['0.0167', '0.25', '0.5', '1', '2', '3', '4', '5', '7', '10', '15', '20']
);

# 0.0167
$counter->observe(floatval(0), ['value']);
# 0.25
$counter->observe(floatval(1), ['value']);
# 0.5
$counter->observe(floatval(2), ['value']);
# 1
$counter->observe(floatval(2), ['value']);
# 2
$counter->observe(floatval(2), ['value']);
# 3
$counter->observe(floatval(2), ['value']);
# 4
$counter->observe(floatval(2), ['value']);
# 5
$counter->observe(floatval(2), ['value']);
# 7
$counter->observe(floatval(2), ['value']);
# 10
$counter->observe(floatval(2), ['value']);
# 15
$counter->observe(floatval(2), ['value']);
# 20
$counter->observe(floatval(2), ['value']);

$renderer = new \Prometheus\RenderTextFormat();
$result = $renderer->render($registry->getMetricFamilySamples());

header('Content-type: ' . \Prometheus\RenderTextFormat::MIME_TYPE);
echo $result;

Actual Result

# HELP namespace_name help
# TYPE namespace_name histogram
namespace_name_bucket{label="value",le="0.0167"} 1
namespace_name_bucket{label="value",le="0.25"} 1
namespace_name_bucket{label="value",le="0.5"} 1
namespace_name_bucket{label="value",le="1"} 2
namespace_name_bucket{label="value",le="2"} 12
namespace_name_bucket{label="value",le="3"} 12
namespace_name_bucket{label="value",le="4"} 12
namespace_name_bucket{label="value",le="5"} 12
namespace_name_bucket{label="value",le="7"} 12
namespace_name_bucket{label="value",le="10"} 12
namespace_name_bucket{label="value",le="15"} 12
namespace_name_bucket{label="value",le="20"} 12
namespace_name_bucket{label="value",le="+Inf"} 12
namespace_name_count{label="value"} 12
namespace_name_sum{label="value"} 21

Expected Result

as needed for Prometheus - in an example: https://www.robustperception.io/how-does-a-prometheus-histogram-work/

# HELP namespace_name help
# TYPE namespace_name histogram
namespace_name_bucket{label="value",le="0.0167"} 1
namespace_name_bucket{label="value",le="0.25"} 1
namespace_name_bucket{label="value",le="0.5"} 1
namespace_name_bucket{label="value",le="1"} 2
namespace_name_bucket{label="value",le="2"} 2
namespace_name_bucket{label="value",le="3"} 2
namespace_name_bucket{label="value",le="4"} 2
namespace_name_bucket{label="value",le="5"} 2
namespace_name_bucket{label="value",le="7"} 2
namespace_name_bucket{label="value",le="10"} 2
namespace_name_bucket{label="value",le="15"} 2
namespace_name_bucket{label="value",le="20"} 2
namespace_name_bucket{label="value",le="+Inf"} 2
namespace_name_count{label="value"} 12
namespace_name_sum{label="value"} 21

The difference is, that because (my guess) the cumulative value in the observation does not change after le="1", the package does not increment the bucket counter and therefore adding the value to the wrong bucket.

I also suggest improving the examples, since it is not self descripting, how the histogram counter is used correctly.

drieschel commented 9 months ago

Hi @rlehner-dk,

the actual result looks correct to me. The single buckets work like counters for values which are less or equal than le. At least I am thinking it should work like that.

vlahanas commented 9 months ago

Hello @rlehner-dk,

The actual outcome is indeed correct, since Prometheus histogram is a cumulative one. You have submitted the following 12 values:

0, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2

Hence, you have all 12 values being less than or equal to 2, and hence all buckets from 2 and above should have a count of 12.

rlehner-dk commented 9 months ago

Hey folks, thank you very much for clarification!