From 50b70a6df4017081917e004f177a3c01cc8115db Mon Sep 17 00:00:00 2001 From: Garry Filakhtov Date: Fri, 18 Oct 2024 18:33:22 +1100 Subject: [PATCH] Ensure that label values are strings in APCNg adapter (#159) * Ensure that label values are strings in APCNg adapter APCNg is crashing if the label value provided wasn't a string, but something that can be coerced to string (such as int). The problem occurs in two different places: - when a metric is emitted, the `storeLabelKeys()` calls into `addItemToKey()` which has its second parameter type hinted as a `string` and throws a type error if anything else is passed. This results in partially stored state; - when trying to scrape metrics with partially stored state, the `APCng::collect()` will try to build all the permutations and expect all the key-value pairs for labels to exist, but numeric label values aren't persisted and so it will cause the `Undefined array key` error as reported in #154; This change ensures that label values are cast to the string type before encoding them and using as APC keys. Signed-off-by: Garry Filakhtov * Fix PHPStan issues Signed-off-by: Garry Filakhtov --------- Signed-off-by: Garry Filakhtov --- src/Prometheus/Storage/APCng.php | 4 ++-- tests/Test/Prometheus/Storage/APCngTest.php | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/Prometheus/Storage/APCng.php b/src/Prometheus/Storage/APCng.php index 5448fc95..416a46f9 100644 --- a/src/Prometheus/Storage/APCng.php +++ b/src/Prometheus/Storage/APCng.php @@ -254,7 +254,7 @@ private function storeLabelKeys(array $data): void $data['name'], $label, 'label' - ]), isset($data['labelValues']) ? $data['labelValues'][$seq] : ''); // may not need the isset check + ]), isset($data['labelValues']) ? (string)$data['labelValues'][$seq] : ''); // may not need the isset check } } @@ -860,7 +860,7 @@ private function sortSamples(array &$samples): void */ private function encodeLabelValues(array $values): string { - $json = json_encode($values); + $json = json_encode(array_map("strval", $values)); if (false === $json) { throw new RuntimeException(json_last_error_msg()); } diff --git a/tests/Test/Prometheus/Storage/APCngTest.php b/tests/Test/Prometheus/Storage/APCngTest.php index 8bf2ea26..02be614c 100644 --- a/tests/Test/Prometheus/Storage/APCngTest.php +++ b/tests/Test/Prometheus/Storage/APCngTest.php @@ -42,6 +42,26 @@ public function itShouldNotClearWholeAPCacheOnFlush(): void ); } + /** + * @test + */ + public function nonStringLabelValuesAreCastToStrings(): void + { + apcu_clear_cache(); + + $adapter = new APCng(); + $registry = new CollectorRegistry($adapter); + $registry->getOrRegisterCounter( + 'ns', + 'int_label_values', + 'test int label values', + ['int_label'], + )->incBy(3, [3]); + + $counter = apcu_fetch("prom:counter:ns_int_label_values:WyIzIl0=:value"); + self::assertSame(3000, $counter); + } + /** * @test */