Remove handling of connection from predis adapter

Jimdo · Feb 26, 2018 · 5f5113a · 5f5113a
1 parent 69cf060
commit 5f5113a
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 81 deletions.
diff --git a/examples/flush_adapter.php b/examples/flush_adapter.php
@@ -9,9 +9,9 @@
     $redisAdapter = new Prometheus\Storage\Redis(array('host' => REDIS_HOST));
     $redisAdapter->flushRedis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
     $adapter->flushRedis();
 } elseif ($adapter === 'apc') {
     $apcAdapter = new Prometheus\Storage\APC();

diff --git a/examples/metrics.php b/examples/metrics.php
@@ -12,9 +12,9 @@
     Redis::setDefaultOptions(array('host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'));
     $adapter = new Prometheus\Storage\Redis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
 } elseif ($adapter === 'apc') {
     $adapter = new Prometheus\Storage\APC();
 } elseif ($adapter === 'in-memory') {

diff --git a/examples/pushgateway.php b/examples/pushgateway.php
@@ -10,9 +10,9 @@
     Redis::setDefaultOptions(array('host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'));
     $adapter = new Prometheus\Storage\Redis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
 } elseif ($adapter === 'apc') {
     $adapter = new Prometheus\Storage\APC();
 } elseif ($adapter === 'in-memory') {

diff --git a/examples/some_counter.php b/examples/some_counter.php
@@ -11,9 +11,9 @@
     Redis::setDefaultOptions(array('host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'));
     $adapter = new Prometheus\Storage\Redis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
 } elseif ($adapter === 'apc') {
     $adapter = new Prometheus\Storage\APC();
 } elseif ($adapter === 'in-memory') {

diff --git a/examples/some_gauge.php b/examples/some_gauge.php
@@ -14,9 +14,9 @@
     Redis::setDefaultOptions(array('host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'));
     $adapter = new Prometheus\Storage\Redis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
 } elseif ($adapter === 'apc') {
     $adapter = new Prometheus\Storage\APC();
 } elseif ($adapter === 'in-memory') {

diff --git a/examples/some_histogram.php b/examples/some_histogram.php
@@ -13,9 +13,9 @@
     Redis::setDefaultOptions(array('host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'));
     $adapter = new Prometheus\Storage\Redis();
 } elseif ($adapter === 'predis') {
-    $adapter = new Prometheus\Storage\Predis([
-        'host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'
-    ]);
+    $adapter = new Prometheus\Storage\Predis(
+        new \Predis\Client(['host' => isset($_SERVER['REDIS_HOST']) ? $_SERVER['REDIS_HOST'] : '127.0.0.1'])
+    );
 } elseif ($adapter === 'apc') {
     $adapter = new Prometheus\Storage\APC();
 } elseif ($adapter === 'in-memory') {

diff --git a/src/Prometheus/Storage/Predis.php b/src/Prometheus/Storage/Predis.php
@@ -13,33 +13,16 @@ class Predis implements Adapter
 {
     const PROMETHEUS_METRIC_KEYS_SUFFIX = '_METRIC_KEYS';
 
-    private static $defaultOptions = [
-        'host' => '127.0.0.1',
-        'port' => 6379,
-        'timeout' => 0.1,
-        'read_timeout' => 10,
-        'persistent_connections' => false,
-    ];
-
     private static $prefix = 'PROMETHEUS_';
 
-    private $options;
     /**
      * @var Client
      */
     private $predis;
 
-    public function __construct(array $options = array())
+    public function __construct(Client $predis)
     {
-        $this->options = array_merge(self::$defaultOptions, $options);
-    }
-
-    /**
-     * @param array $options
-     */
-    public static function setDefaultOptions(array $options)
-    {
-        self::$defaultOptions = array_merge(self::$defaultOptions, $options);
+        $this->predis = $predis;
     }
 
     public static function setPrefix($prefix)
@@ -49,7 +32,6 @@ public static function setPrefix($prefix)
 
     public function flushRedis()
     {
-        $this->openConnection();
         $this->predis->flushall();
     }
 
@@ -59,7 +41,6 @@ public function flushRedis()
      */
     public function collect()
     {
-        $this->openConnection();
         $metrics = $this->collectHistograms();
         $metrics = array_merge($metrics, $this->collectGauges());
         $metrics = array_merge($metrics, $this->collectCounters());
@@ -71,30 +52,8 @@ function (array $metric) {
         );
     }
 
-    /**
-     * @throws StorageException
-     */
-    private function openConnection()
-    {
-        try {
-
-            $this->predis = new Client([
-                'scheme' => 'tcp',
-                'host'   => $this->options['host'],
-                'port'   => $this->options['port'],
-                'timeout' => $this->options['timeout'],
-                'read_write_timeout' => $this->options['read_timeout'],
-                'persistent' => $this->options['persistent_connections'],
-            ]);
-
-        } catch (\RedisException $e) {
-            throw new StorageException("Can't connect to Redis server", 0, $e);
-        }
-    }
-
     public function updateHistogram(array $data)
     {
-        $this->openConnection();
         $bucketToIncrease = '+Inf';
         foreach ($data['buckets'] as $bucket) {
             if ($data['value'] <= $bucket) {
@@ -105,6 +64,7 @@ public function updateHistogram(array $data)
         $metaData = $data;
         unset($metaData['value']);
         unset($metaData['labelValues']);
+
         $this->predis->eval(<<<LUA
 local increment = redis.call('hIncrByFloat', KEYS[1], KEYS[2], ARGV[1])
 redis.call('hIncrBy', KEYS[1], KEYS[3], 1)
@@ -126,7 +86,6 @@ public function updateHistogram(array $data)
 
     public function updateGauge(array $data)
     {
-        $this->openConnection();
         $metaData = $data;
         unset($metaData['value']);
         unset($metaData['labelValues']);
@@ -159,7 +118,6 @@ public function updateGauge(array $data)
 
     public function updateCounter(array $data)
     {
-        $this->openConnection();
         $metaData = $data;
         unset($metaData['value']);
         unset($metaData['labelValues']);
@@ -219,22 +177,16 @@ private function collectHistograms()
                 $acc = 0;
                 foreach ($histogram['buckets'] as $bucket) {
                     $bucketKey = json_encode(array('b' => $bucket, 'labelValues' => $labelValues));
-                    if (!isset($raw[$bucketKey])) {
-                        $histogram['samples'][] = array(
-                            'name' => $histogram['name'] . '_bucket',
-                            'labelNames' => array('le'),
-                            'labelValues' => array_merge($labelValues, array($bucket)),
-                            'value' => $acc
-                        );
-                    } else {
+                    if (isset($raw[$bucketKey])) {
                         $acc += $raw[$bucketKey];
-                        $histogram['samples'][] = array(
-                            'name' => $histogram['name'] . '_bucket',
-                            'labelNames' => array('le'),
-                            'labelValues' => array_merge($labelValues, array($bucket)),
-                            'value' => $acc
-                        );
                     }
+
+                    $histogram['samples'][] = array(
+                        'name' => $histogram['name'] . '_bucket',
+                        'labelNames' => array('le'),
+                        'labelValues' => array_merge($labelValues, array($bucket)),
+                        'value' => $acc
+                    );
                 }
 
                 // Add the count

diff --git a/tests/Test/Prometheus/Predis/CollectorRegistryTest.php b/tests/Test/Prometheus/Predis/CollectorRegistryTest.php
@@ -10,7 +10,7 @@ class CollectorRegistryTest extends AbstractCollectorRegistryTest
 {
     public function configureAdapter()
     {
-        $this->adapter = new Predis(array('host' => REDIS_HOST));
+        $this->adapter = new Predis(new \Predis\Client(['host' => REDIS_HOST]));
         $this->adapter->flushRedis();
     }
 }
diff --git a/tests/Test/Prometheus/Predis/CounterTest.php b/tests/Test/Prometheus/Predis/CounterTest.php
@@ -14,7 +14,7 @@ class CounterTest extends AbstractCounterTest
 
     public function configureAdapter()
     {
-        $this->adapter = new Predis(array('host' => REDIS_HOST));
+        $this->adapter = new Predis(new \Predis\Client(['host' => REDIS_HOST]));
         $this->adapter->flushRedis();
     }
 }
diff --git a/tests/Test/Prometheus/Predis/GaugeTest.php b/tests/Test/Prometheus/Predis/GaugeTest.php
@@ -14,7 +14,7 @@ class GaugeTest extends AbstractGaugeTest
 
     public function configureAdapter()
     {
-        $this->adapter = new Predis(array('host' => REDIS_HOST));
+        $this->adapter = new Predis(new \Predis\Client(['host' => REDIS_HOST]));
         $this->adapter->flushRedis();
     }
 }
diff --git a/tests/Test/Prometheus/Predis/HistogramTest.php b/tests/Test/Prometheus/Predis/HistogramTest.php
@@ -14,7 +14,7 @@ class HistogramTest extends AbstractHistogramTest
 
     public function configureAdapter()
     {
-        $this->adapter = new Predis(array('host' => REDIS_HOST));
+        $this->adapter = new Predis(new \Predis\Client(['host' => REDIS_HOST]));
         $this->adapter->flushRedis();
     }
 }