Merge branch 'main' into fix-potential-infinite-loop

src/Prometheus/RenderTextFormat.php

@@ -5,16 +5,18 @@
 namespace Prometheus;
 
 use RuntimeException;
+use Throwable;
 
 class RenderTextFormat implements RendererInterface
 {
     const MIME_TYPE = 'text/plain; version=0.0.4';
 
     /**
      * @param MetricFamilySamples[] $metrics
+     * @param bool $silent If true, render value errors as comments instead of throwing them.
      * @return string
      */
-    public function render(array $metrics): string
+    public function render(array $metrics, bool $silent = false): string
     {
         usort($metrics, function (MetricFamilySamples $a, MetricFamilySamples $b): int {
             return strcmp($a->getName(), $b->getName());
@@ -25,7 +27,20 @@ public function render(array $metrics): string
             $lines[] = "# HELP " . $metric->getName() . " {$metric->getHelp()}";
             $lines[] = "# TYPE " . $metric->getName() . " {$metric->getType()}";
             foreach ($metric->getSamples() as $sample) {
-                $lines[] = $this->renderSample($metric, $sample);
+                try {
+                    $lines[] = $this->renderSample($metric, $sample);
+                } catch (Throwable $e) {
+                    // Redis and RedisNg allow samples with mismatching labels to be stored, which could cause ValueError
+                    // to be thrown when rendering. If this happens, users can decide whether to ignore the error or not.
+                    // These errors will normally disappear after the storage is flushed.
+                    if (!$silent) {
+                        throw $e;
+                    }
+
+                    $lines[] = "# Error: {$e->getMessage()}";
+                    $lines[] = "#   Labels: " . json_encode(array_merge($metric->getLabelNames(), $sample->getLabelNames()));
+                    $lines[] = "#   Values: " . json_encode(array_merge($sample->getLabelValues()));
+                }
             }
         }
         return implode("\n", $lines) . "\n";
@@ -40,7 +55,7 @@ private function renderSample(MetricFamilySamples $metric, Sample $sample): stri
     {
         $labelNames = $metric->getLabelNames();
         if ($metric->hasLabelNames() || $sample->hasLabelNames()) {
-            $escapedLabels = $this->escapeAllLabels($labelNames, $sample);
+            $escapedLabels = $this->escapeAllLabels($metric, $labelNames, $sample);
             return $sample->getName() . '{' . implode(',', $escapedLabels) . '} ' . $sample->getValue();
         }
         return $sample->getName() . ' ' . $sample->getValue();
@@ -56,19 +71,20 @@ private function escapeLabelValue(string $v): string
     }
 
     /**
+     * @param MetricFamilySamples $metric
      * @param string[] $labelNames
      * @param Sample $sample
      *
      * @return string[]
      */
-    private function escapeAllLabels(array $labelNames, Sample $sample): array
+    private function escapeAllLabels(MetricFamilySamples $metric, array $labelNames, Sample $sample): array
     {
         $escapedLabels = [];
 
         $labels = array_combine(array_merge($labelNames, $sample->getLabelNames()), $sample->getLabelValues());
 
         if ($labels === false) {
-            throw new RuntimeException('Unable to combine labels.');
+            throw new RuntimeException('Unable to combine labels for metric named ' . $metric->getName());
         }
 
         foreach ($labels as $labelName => $labelValue) {

src/Prometheus/Sample.php

@@ -4,6 +4,8 @@
 
 namespace Prometheus;
 
+use function is_infinite;
+
 class Sample
 {
     /**
@@ -67,6 +69,9 @@ public function getLabelValues(): array
      */
     public function getValue(): string
     {
+        if (is_float($this->value) && is_infinite($this->value)) {
+            return $this->value > 0 ? '+Inf' : '-Inf';
+        }
         return (string) $this->value;
     }
 

src/Prometheus/Storage/APC.php

@@ -130,11 +130,32 @@ public function updateSummary(array $data): void
     public function updateGauge(array $data): void
     {
         $valueKey = $this->valueKey($data);
+        $old = apcu_fetch($valueKey);
         if ($data['command'] === Adapter::COMMAND_SET) {
-            apcu_store($valueKey, $this->toBinaryRepresentationAsInteger($data['value']));
-            apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+            $new = $this->toBinaryRepresentationAsInteger($data['value']);
+            if ($old === false) {
+                apcu_store($valueKey, $new);
+                apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+                return;
+            } else {
+                // Taken from https://github.com/prometheus/client_golang/blob/66058aac3a83021948e5fb12f1f408ff556b9037/prometheus/value.go#L91
+                while (true) {
+                    if ($old !== false) {
+                        if (apcu_cas($valueKey, $old, $new)) {
+                            return;
+                        } else {
+                            $old = apcu_fetch($valueKey);
+                        }
+                    } else {
+                        // Cache got evicted under our feet? Just consider it a fresh/new insert and move on.
+                        apcu_store($valueKey, $new);
+                        apcu_store($this->metaKey($data), json_encode($this->metaData($data)));
+                        return;
+                    }
+                }
+            }
         } else {
-            if (!apcu_exists($valueKey)) {
+            if ($old === false) {
                 $new = apcu_add($valueKey, $this->toBinaryRepresentationAsInteger(0));
                 if ($new) {
                     apcu_store($this->metaKey($data), json_encode($this->metaData($data)));

src/Prometheus/Storage/APCng.php

@@ -169,16 +169,34 @@ public function updateSummary(array $data): void
     public function updateGauge(array $data): void
     {
         $valueKey = $this->valueKey($data);
+        $old = apcu_fetch($valueKey);
         if ($data['command'] === Adapter::COMMAND_SET) {
-            apcu_store($valueKey, $this->convertToIncrementalInteger($data['value']), 0);
-            $this->storeMetadata($data);
-            $this->storeLabelKeys($data);
+            $new = $this->convertToIncrementalInteger($data['value']);
+            if ($old === false) {
+                apcu_store($valueKey, $new, 0);
+                $this->storeMetadata($data);
+                $this->storeLabelKeys($data);
+
+                return;
+            }
+
+            for ($loops = 0; $loops < self::MAX_LOOPS; $loops++) {
+                if (apcu_cas($valueKey, $old, $new)) {
+                    break;
+                }
+                $old = apcu_fetch($valueKey);
+                if ($old === false) {
+                    apcu_store($valueKey, $new, 0);
+                    $this->storeMetadata($data);
+                    $this->storeLabelKeys($data);
+
+                    return;
+                }
+            }
 
             return;
         }
 
-        $old = apcu_fetch($valueKey);
-
         if ($old === false) {
             apcu_add($valueKey, 0, 0);
             $this->storeMetadata($data);
@@ -896,6 +914,10 @@ private function decodeLabelKey(string $str): string
     private function storeMetadata(array $data, bool $encoded = true): void
     {
         $metaKey = $this->metaKey($data);
+        if (apcu_exists($metaKey)) {
+            return;
+        }
+
         $metaData = $this->metaData($data);
         $toStore = $metaData;
 

src/Prometheus/Storage/Redis.php

@@ -226,10 +226,17 @@ private function connectToServer(): void
                 $connection_successful = $this->redis->connect($this->options['host'], (int) $this->options['port'], (float) $this->options['timeout']);
             }
             if (!$connection_successful) {
-                throw new StorageException("Can't connect to Redis server", 0);
+                throw new StorageException(
+                    sprintf("Can't connect to Redis server. %s", $this->redis->getLastError()),
+                    0
+                );
             }
         } catch (\RedisException $e) {
-            throw new StorageException("Can't connect to Redis server", 0, $e);
+            throw new StorageException(
+                sprintf("Can't connect to Redis server. %s", $e->getMessage()),
+                $e->getCode(),
+                $e
+            );
         }
     }
 

tests/Test/Prometheus/RenderTextFormatTest.php

@@ -10,6 +10,10 @@
 use Prometheus\RenderTextFormat;
 use PHPUnit\Framework\TestCase;
 use Prometheus\Storage\InMemory;
+use Prometheus\Storage\Redis;
+use ValueError;
+
+use const INF;
 
 class RenderTextFormatTest extends TestCase
 {
@@ -35,6 +39,10 @@ private function buildSamples(): array
                  ->inc(['bob', 'al\ice']);
         $registry->getOrRegisterGauge($namespace, 'gauge', 'gauge-help-text', ['label1', 'label2'])
                  ->inc(["bo\nb", 'ali\"ce']);
+        $registry->getOrRegisterGauge($namespace, 'gauge2', '', ['label1'])
+                 ->set(INF, ["infinity"]);
+        $registry->getOrRegisterGauge($namespace, 'gauge2', '', ['label1'])
+                 ->set(-INF, ["infinity-negative"]);
         $registry->getOrRegisterHistogram($namespace, 'histogram', 'histogram-help-text', ['label1', 'label2'], [0, 10, 100])
                  ->observe(5, ['bob', 'alice']);
         $registry->getOrRegisterSummary($namespace, 'summary', 'summary-help-text', ['label1', 'label2'], 60, [0.1, 0.5, 0.9])
@@ -52,6 +60,10 @@ private function getExpectedOutput(): string
 # HELP mynamespace_gauge gauge-help-text
 # TYPE mynamespace_gauge gauge
 mynamespace_gauge{label1="bo\\nb",label2="ali\\\\\"ce"} 1
+# HELP mynamespace_gauge2 
+# TYPE mynamespace_gauge2 gauge
+mynamespace_gauge2{label1="infinity"} +Inf
+mynamespace_gauge2{label1="infinity-negative"} -Inf
 # HELP mynamespace_histogram histogram-help-text
 # TYPE mynamespace_histogram histogram
 mynamespace_histogram_bucket{label1="bob",label2="alice",le="0"} 0
@@ -70,4 +82,57 @@ private function getExpectedOutput(): string
 
 TEXTPLAIN;
     }
+
+    public function testValueErrorThrownWithInvalidSamples(): void
+    {
+        $namespace = 'foo';
+        $counter = 'bar';
+        $storage = new Redis(['host' => REDIS_HOST]);
+        $storage->wipeStorage();
+
+        $registry = new CollectorRegistry($storage, false);
+        $registry->registerCounter($namespace, $counter, 'counter-help-text', ['label1', 'label2'])
+            ->inc(['bob', 'alice']);
+
+        // Reload the registry with an updated counter config
+        $registry = new CollectorRegistry($storage, false);
+        $registry->registerCounter($namespace, $counter, 'counter-help-text', ['label1', 'label2', 'label3'])
+            ->inc(['bob', 'alice', 'eve']);
+
+        $this->expectException(ValueError::class);
+
+        $renderer = new RenderTextFormat();
+        $renderer->render($registry->getMetricFamilySamples());
+    }
+
+    public function testOutputWithInvalidSamplesSkipped(): void
+    {
+        $namespace = 'foo';
+        $counter = 'bar';
+        $storage = new Redis(['host' => REDIS_HOST]);
+        $storage->wipeStorage();
+
+        $registry = new CollectorRegistry($storage, false);
+        $registry->registerCounter($namespace, $counter, 'counter-help-text', ['label1', 'label2'])
+            ->inc(['bob', 'alice']);
+
+        // Reload the registry with an updated counter config
+        $registry = new CollectorRegistry($storage, false);
+        $registry->registerCounter($namespace, $counter, 'counter-help-text', ['label1', 'label2', 'label3'])
+            ->inc(['bob', 'alice', 'eve']);
+
+        $expectedOutput = '
+# HELP foo_bar counter-help-text
+# TYPE foo_bar counter
+foo_bar{label1="bob",label2="alice"} 1
+# Error: array_combine(): Argument #1 ($keys) and argument #2 ($values) must have the same number of elements
+#   Labels: ["label1","label2"]
+#   Values: ["bob","alice","eve"]
+';
+
+        $renderer = new RenderTextFormat();
+        $output = $renderer->render($registry->getMetricFamilySamples(), true);
+
+        self::assertSame(trim($expectedOutput), trim($output));
+    }
 }