From dc0e6ce2b85f211a839e304aa25eb20825e6c1e4 Mon Sep 17 00:00:00 2001
From: Joe Cai <joe.cai@bigcommerce.com>
Date: Wed, 20 Dec 2023 16:10:50 +1100
Subject: [PATCH] [RenderTextFormat] Allow value errors to be rendered as
 comments.

Redis and RedisNg allow samples with mismatching labels to be stored, which could cause ValueError to be thrown when
rendering. Rendering would fail as a result, which is not ideal.

- Change render() to accept an optional $silent parameter. When set to true, render the errors as comments instead of
  throwing them and failing the whole operation.

Signed-off-by: Joe Cai <joe.cai@bigcommerce.com>
---
 src/Prometheus/RenderTextFormat.php           | 19 ++++++-
 .../Test/Prometheus/RenderTextFormatTest.php  | 56 +++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/src/Prometheus/RenderTextFormat.php b/src/Prometheus/RenderTextFormat.php
index b160865c..edfa3873 100644
--- a/src/Prometheus/RenderTextFormat.php
+++ b/src/Prometheus/RenderTextFormat.php
@@ -5,6 +5,7 @@
 namespace Prometheus;
 
 use RuntimeException;
+use Throwable;
 
 class RenderTextFormat implements RendererInterface
 {
@@ -12,9 +13,10 @@ class RenderTextFormat implements RendererInterface
 
     /**
      * @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";
diff --git a/tests/Test/Prometheus/RenderTextFormatTest.php b/tests/Test/Prometheus/RenderTextFormatTest.php
index fc13cccc..df1bc0c4 100644
--- a/tests/Test/Prometheus/RenderTextFormatTest.php
+++ b/tests/Test/Prometheus/RenderTextFormatTest.php
@@ -10,6 +10,8 @@
 use Prometheus\RenderTextFormat;
 use PHPUnit\Framework\TestCase;
 use Prometheus\Storage\InMemory;
+use Prometheus\Storage\Redis;
+use ValueError;
 
 class RenderTextFormatTest extends TestCase
 {
@@ -70,4 +72,58 @@ 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));
+    }
 }