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

Redis missing/invalid meta data triggers fatal error #110

Closed m-idler closed 1 year ago

m-idler commented 1 year ago

Hello, we encounter some issues when updating from the old jimdo/prometheus_client_php to promphp/prometheus_client_php. For any (so far) unknown reasons, there seem to be some invalid entries existing in redis which will result in a fatal error when trying to parse the non existing __meta data. In our case, collectCounters() is affected, but in general it could affect the other collect...() methods as well.

In https://github.com/PromPHP/prometheus_client_php/blob/main/src/Prometheus/Storage/Redis.php#L608 the meta data is parsed to the json_decode method, in case of non existing meta data this will result in a fatal error due to the strict_types and null being passed as the first string parameter:

$raw = $this->redis->hGetAll(str_replace($this->redis->_prefix(''), '', $key));
$counter = json_decode($raw['__meta'], true);

triggering

PHP Fatal error:  Uncaught TypeError: json_decode() expects parameter 1 to be string, null given in ...

To make the code less error prone i would suggest to ignore entries with invalid __meta data, e.g. like that, maybe with some error logging as well:

foreach ($keys as $key) {
    $raw = $this->redis->hGetAll(str_replace($this->redis->_prefix(''), '', $key));
    if (!isset($raw['__meta'])) {
        // maybe add some error reporting as well?!
        trigger_error(sprintf('prometheus_client_php got invalid raw data from redis for key %s', $key), E_USER_NOTICE);
        continue;
    }
    $counter = json_decode($raw['__meta'], true);
    // ....
}

(similar json_decode($raw['__meta'], true); code exists in collectHistograms() and collectGauges() as well)

Before creating a PR it would be really nice to get some feedback and opinions, regarding this topic as I'm currently not that much into the details of prometheus and redis.