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 `wipeStorage` does not wipe prometheus related keys #130

Open bogdandubyk opened 10 months ago

bogdandubyk commented 10 months ago

library verstion v2.7.1 Redis version 3.2

Redis::wipeStorage() method does not work, I add some debug statements to get keys before wiping and after wiping and keys still exists, here is the method with debug statements

  public function wipeStorage(): void
    {
        $this->ensureOpenConnection();

        $searchPattern = "";

        $globalPrefix = $this->redis->getOption(\Redis::OPT_PREFIX);
        // @phpstan-ignore-next-line false positive, phpstan thinks getOptions returns int
        if (is_string($globalPrefix)) {
            $searchPattern .= $globalPrefix;
        }

        $searchPattern .= self::$prefix;
        $searchPattern .= '*';

        dump($this->redis->keys($searchPattern));

        $this->redis->eval(
            <<<LUA
local cursor = "0"
repeat 
    local results = redis.call('SCAN', cursor, 'MATCH', ARGV[1])
    cursor = results[1]
    for _, key in ipairs(results[2]) do
        redis.call('DEL', key)
    end
until cursor == "0"
LUA
            ,
            [$searchPattern],
            0
        );

        dump($this->redis->keys($searchPattern));
    }

so before the eval is called debug return output:

^ array:8 [
  0 => "PROMETHEUS_counter_METRIC_KEYS"
  1 => "PROMETHEUS_:histogram:http_response_body_size_bytes"
  2 => "PROMETHEUS_histogram_METRIC_KEYS"
  3 => "PROMETHEUS_:gauge:php_info"
  4 => "PROMETHEUS_gauge_METRIC_KEYS"
  5 => "PROMETHEUS_:histogram:http_request_duration_milliseconds"
  6 => "PROMETHEUS_:histogram:http_request_body_size_bytes"
  7 => "PROMETHEUS_:counter:http_responses_total"
]

and after eval dump output is identical, so nothing is deleted

bogdandubyk commented 10 months ago

hmm, okay if I use Redis 5.0 all works fine, but does not work with 3.2

bogdandubyk commented 10 months ago

I think this needs to be either fixed, to we need add to readme which redis version is supported?