Skip to content

Commit

Permalink
Ensure that label values are strings in APCNg adapter (#159)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* Fix PHPStan issues
Signed-off-by: Garry Filakhtov <[email protected]>

---------

Signed-off-by: Garry Filakhtov <[email protected]>
  • Loading branch information
filakhtov authored Oct 18, 2024
1 parent 600f6df commit 50b70a6
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/Prometheus/Storage/APCng.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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());
}
Expand Down
20 changes: 20 additions & 0 deletions tests/Test/Prometheus/Storage/APCngTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.1

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.1

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

Check failure on line 59 in tests/Test/Prometheus/Storage/APCngTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3

Parameter #2 $labels of method Prometheus\Counter::incBy() expects array<string>, array<int, int> given.

$counter = apcu_fetch("prom:counter:ns_int_label_values:WyIzIl0=:value");
self::assertSame(3000, $counter);
}

/**
* @test
*/
Expand Down

0 comments on commit 50b70a6

Please sign in to comment.