Skip to content

Commit

Permalink
Ensure that label values are strings in APCNg adapter
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
filakhtov committed Sep 3, 2024
1 parent 35d5a68 commit 0e837ea
Show file tree
Hide file tree
Showing 2 changed files with 25 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
23 changes: 23 additions & 0 deletions tests/Test/Prometheus/Storage/APCngTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,29 @@ 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'],
)->inc([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.3

Parameter #1 $labels of method Prometheus\Counter::inc() 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 #1 $labels of method Prometheus\Counter::inc() 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 #1 $labels of method Prometheus\Counter::inc() expects array<string>, array<int, int> given.

$jsonLabels = json_encode(["3"]);
$encodedLabels = base64_encode($jsonLabels);

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3

Parameter #1 $string of function base64_encode expects string, string|false given.

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.1

Parameter #1 $string of function base64_encode expects string, string|false given.

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2

Parameter #1 $string of function base64_encode expects string, string|false given.

$counter = apcu_fetch("prom:counter:ns_int_label_values:{$encodedLabels}:value");
$this->assertSame(1000, $counter);

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3

Dynamic call to static method PHPUnit\Framework\Assert::assertSame().

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.1

Dynamic call to static method PHPUnit\Framework\Assert::assertSame().

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

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2

Dynamic call to static method PHPUnit\Framework\Assert::assertSame().
}

/**
* @test
*/
Expand Down

0 comments on commit 0e837ea

Please sign in to comment.