Jimdo / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
281 stars 213 forks source link

Add support for UTF-8 characters for label values #56

Closed mps-sepetrov closed 6 years ago

mps-sepetrov commented 7 years ago

According to the Prometheus documentation

label_value can be any sequence of UTF-8 characters

Expected behaviour

Label values can be any sequence of UTF-8 characters.

Current behaviour

No measures are taken to ensure that the label values can be persisted without affecting the retrieval process.

Steps to reproduce

  1. Persist a metric (counter, gauge, histogram) using APC or In-Memory storage, which has a label with value : (semicolon).
  2. Retrieve the metrics and observe PHP warnings and metrics with missing label.
<?php

require __DIR__ . '/vendor/autoload.php';

//$adapter = new Prometheus\Storage\APC();
//$adapter->flushAPC();

$adapter = new Prometheus\Storage\InMemory();
$adapter->flushMemory();

$registry = new Prometheus\CollectorRegistry($adapter);

$counter = $registry->registerCounter('test', 'some_counter', 'it increases', ['type']);
$counter->incBy(1, [':']);

$renderer = new Prometheus\RenderTextFormat();
$result = $renderer->render($registry->getMetricFamilySamples());

header('Content-type: ' . Prometheus\RenderTextFormat::MIME_TYPE);
echo $result;
//Warning: array_combine(): Both parameters should have an equal number of elements in ///var/www/html/src/Prometheus/RenderTextFormat.php on line 39
//
//Warning: Invalid argument supplied for foreach() in ///var/www/html/src/Prometheus/RenderTextFormat.php on line 40
//
//Warning: Cannot modify header information - headers already sent by (output started at ///var/www/html/src/Prometheus/RenderTextFormat.php:39) in /var/www/html/test.php on line 19
//# HELP test_some_counter it increases
//# TYPE test_some_counter counter
//test_some_counter{} 1

Proposed implementation

Suitable escaping strategy must be used, depending on the storage implementation. JSON-encoding is not enough to escape colons. Base64-encoding the already JSON-encoded values will make the string safe to persist when using APC and in-memory storage.

bracki commented 6 years ago

Fixed via #57.