Fix tracestate implementation (#1267)
* Port tracestate tests * Fix tracestate implementation * Fix W3C test service * Fix style * Move tests into `TraceStateTest` * Assert that warning is triggered when parsing invalid tracestate * Add additional tests for valid/invalid keys/values
This commit is contained in:
Tobias Bachert
GitHub
parent
a1489d3081
commit
6dd255deac
|
|
@ -4,15 +4,22 @@ declare(strict_types=1);
|
|||
|
||||
namespace OpenTelemetry\API\Trace;
|
||||
|
||||
use function array_reverse;
|
||||
use function count;
|
||||
use function end;
|
||||
use function explode;
|
||||
use function key;
|
||||
use OpenTelemetry\API\Behavior\LogsMessagesTrait;
|
||||
use function prev;
|
||||
use function sprintf;
|
||||
use function strlen;
|
||||
use function trim;
|
||||
|
||||
class TraceState implements TraceStateInterface
|
||||
{
|
||||
use LogsMessagesTrait;
|
||||
|
||||
public const MAX_LIST_MEMBERS = 32; //@see https://www.w3.org/TR/trace-context/#tracestate-header-field-values
|
||||
/** @deprecated will be removed */
|
||||
public const MAX_COMBINED_LENGTH = 512; //@see https://www.w3.org/TR/trace-context/#tracestate-limits
|
||||
public const LIST_MEMBERS_SEPARATOR = ',';
|
||||
public const LIST_MEMBER_KEY_VALUE_SPLITTER = '=';
|
||||
|
|
@ -23,139 +30,130 @@ class TraceState implements TraceStateInterface
|
|||
private const VALID_VALUE_BASE_REGEX = '/^[ -~]{0,255}[!-~]$/';
|
||||
private const INVALID_VALUE_COMMA_EQUAL_REGEX = '/,|=/';
|
||||
|
||||
/** @var string[] */
|
||||
private array $traceState = [];
|
||||
/** @var array<string, string> */
|
||||
private array $traceState;
|
||||
|
||||
public function __construct(string $rawTracestate = null)
|
||||
public function __construct(?string $rawTracestate = null)
|
||||
{
|
||||
if ($rawTracestate === null || trim($rawTracestate) === '') {
|
||||
return;
|
||||
}
|
||||
$this->traceState = $this->parse($rawTracestate);
|
||||
$this->traceState = self::parse($rawTracestate ?? '');
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function with(string $key, string $value): TraceStateInterface
|
||||
{
|
||||
$clonedTracestate = clone $this;
|
||||
if (!self::validateMember($this->traceState, $key, $value)) {
|
||||
self::logWarning('Invalid tracestate key/value for: ' . $key);
|
||||
|
||||
if ($this->validateKey($key) && $this->validateValue($value)) {
|
||||
|
||||
/*
|
||||
* Only one entry per key is allowed. In this case we need to overwrite the vendor entry
|
||||
* upon reentry to the tracing system and ensure the updated entry is at the beginning of
|
||||
* the list. This means we place it the back for now and it will be at the beginning once
|
||||
* we reverse the order back during __toString().
|
||||
*/
|
||||
if (array_key_exists($key, $clonedTracestate->traceState)) {
|
||||
unset($clonedTracestate->traceState[$key]);
|
||||
}
|
||||
|
||||
// Add new or updated entry to the back of the list.
|
||||
$clonedTracestate->traceState[$key] = $value;
|
||||
} else {
|
||||
self::logWarning('Invalid tracetrace key/value for: ' . $key);
|
||||
return $this;
|
||||
}
|
||||
|
||||
return $clonedTracestate;
|
||||
$clone = clone $this;
|
||||
$clone->traceState = [$key => $value] + $this->traceState;
|
||||
|
||||
return $clone;
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function without(string $key): TraceStateInterface
|
||||
{
|
||||
$clonedTracestate = clone $this;
|
||||
|
||||
if ($key !== '') {
|
||||
unset($clonedTracestate->traceState[$key]);
|
||||
if (!isset($this->traceState[$key])) {
|
||||
return $this;
|
||||
}
|
||||
|
||||
return $clonedTracestate;
|
||||
$clone = clone $this;
|
||||
unset($clone->traceState[$key]);
|
||||
|
||||
return $clone;
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function get(string $key): ?string
|
||||
{
|
||||
return $this->traceState[$key] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function getListMemberCount(): int
|
||||
{
|
||||
return count($this->traceState);
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function __toString(): string
|
||||
public function toString(?int $limit = null): string
|
||||
{
|
||||
if ($this->traceState === []) {
|
||||
return '';
|
||||
}
|
||||
$traceStateString='';
|
||||
foreach (array_reverse($this->traceState) as $k => $v) {
|
||||
$traceStateString .=$k . self::LIST_MEMBER_KEY_VALUE_SPLITTER . $v . self::LIST_MEMBERS_SEPARATOR;
|
||||
$traceState = $this->traceState;
|
||||
|
||||
if ($limit !== null) {
|
||||
$length = 0;
|
||||
foreach ($traceState as $key => $value) {
|
||||
$length && ($length += 1);
|
||||
$length += strlen($key) + 1 + strlen($value);
|
||||
}
|
||||
if ($length > $limit) {
|
||||
// Entries larger than 128 characters long SHOULD be removed first.
|
||||
foreach ([128, 0] as $threshold) {
|
||||
// Then entries SHOULD be removed starting from the end of tracestate.
|
||||
for ($value = end($traceState); $key = key($traceState);) {
|
||||
$entry = strlen($key) + 1 + strlen($value);
|
||||
$value = prev($traceState);
|
||||
if ($entry <= $threshold) {
|
||||
continue;
|
||||
}
|
||||
|
||||
unset($traceState[$key]);
|
||||
if (($length -= $entry + 1) <= $limit) {
|
||||
break 2;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return rtrim($traceStateString, ',');
|
||||
$s = '';
|
||||
foreach ($traceState as $key => $value) {
|
||||
$s && ($s .= ',');
|
||||
$s .= $key;
|
||||
$s .= '=';
|
||||
$s .= $value;
|
||||
}
|
||||
|
||||
return $s;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the raw tracestate header into the TraceState object. Since new or updated entries must
|
||||
* be added to the beginning of the list, the key-value pairs in the TraceState object will be
|
||||
* stored in reverse order. This ensures new entries added to the TraceState object are at the
|
||||
* beginning when we reverse the order back again while building the final tracestate header.
|
||||
*
|
||||
* Ex:
|
||||
* tracestate = 'vendor1=value1,vendor2=value2'
|
||||
*
|
||||
* ||
|
||||
* \/
|
||||
*
|
||||
* $this->tracestate = ['vendor2' => 'value2' ,'vendor1' => 'value1']
|
||||
*
|
||||
*/
|
||||
private function parse(string $rawTracestate): array
|
||||
public function __toString(): string
|
||||
{
|
||||
if (strlen($rawTracestate) > self::MAX_COMBINED_LENGTH) {
|
||||
self::logWarning('tracestate discarded, exceeds max combined length: ' . self::MAX_COMBINED_LENGTH);
|
||||
return $this->toString();
|
||||
}
|
||||
|
||||
return [];
|
||||
}
|
||||
$parsedTracestate = [];
|
||||
$listMembers = explode(self::LIST_MEMBERS_SEPARATOR, $rawTracestate);
|
||||
private static function parse(string $rawTracestate): array
|
||||
{
|
||||
$traceState = [];
|
||||
$members = explode(',', $rawTracestate);
|
||||
foreach ($members as $member) {
|
||||
if (($member = trim($member, " \t")) === '') {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (count($listMembers) > self::MAX_LIST_MEMBERS) {
|
||||
self::logWarning('tracestate discarded, too many members');
|
||||
|
||||
return [];
|
||||
}
|
||||
|
||||
foreach ($listMembers as $listMember) {
|
||||
$vendor = explode(self::LIST_MEMBER_KEY_VALUE_SPLITTER, trim($listMember));
|
||||
|
||||
// There should only be one list-member per vendor separated by '='
|
||||
if (count($vendor) !== 2 || !$this->validateKey($vendor[0]) || !$this->validateValue($vendor[1])) {
|
||||
self::logWarning('tracestate discarded, invalid member: ' . $listMember);
|
||||
$member = explode('=', $member, 2);
|
||||
if (count($member) !== 2) {
|
||||
self::logWarning(sprintf('Incomplete list member in tracestate "%s"', $rawTracestate));
|
||||
|
||||
return [];
|
||||
}
|
||||
$parsedTracestate[$vendor[0]] = $vendor[1];
|
||||
|
||||
[$key, $value] = $member;
|
||||
if (!self::validateMember($traceState, $key, $value)) {
|
||||
self::logWarning(sprintf('Invalid list member "%s=%s" in tracestate "%s"', $key, $value, $rawTracestate));
|
||||
|
||||
return [];
|
||||
}
|
||||
|
||||
$traceState[$key] ??= $value;
|
||||
}
|
||||
|
||||
/*
|
||||
* Reversing the tracestate ensures the new entries added to the TraceState object are at
|
||||
* the beginning when we reverse it back during __toString().
|
||||
*/
|
||||
return array_reverse($parsedTracestate);
|
||||
return $traceState;
|
||||
}
|
||||
|
||||
private static function validateMember(array $traceState, string $key, string $value): bool
|
||||
{
|
||||
return (isset($traceState[$key]) || self::validateKey($key))
|
||||
&& self::validateValue($value)
|
||||
&& (count($traceState) < self::MAX_LIST_MEMBERS || isset($traceState[$key]));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -168,7 +166,7 @@ class TraceState implements TraceStateInterface
|
|||
*
|
||||
* @see https://www.w3.org/TR/trace-context/#key
|
||||
*/
|
||||
private function validateKey(string $key): bool
|
||||
private static function validateKey(string $key): bool
|
||||
{
|
||||
return preg_match(self::VALID_KEY_REGEX, $key) !== 0;
|
||||
}
|
||||
|
|
@ -180,7 +178,7 @@ class TraceState implements TraceStateInterface
|
|||
*
|
||||
* @see https://www.w3.org/TR/trace-context/#value
|
||||
*/
|
||||
private function validateValue(string $key): bool
|
||||
private static function validateValue(string $key): bool
|
||||
{
|
||||
return (preg_match(self::VALID_VALUE_BASE_REGEX, $key) !== 0)
|
||||
&& (preg_match(self::INVALID_VALUE_COMMA_EQUAL_REGEX, $key) === 0);
|
||||
|
|
|
|||
|
|
@ -50,6 +50,16 @@ interface TraceStateInterface
|
|||
*/
|
||||
public function getListMemberCount(): int;
|
||||
|
||||
/**
|
||||
* Returns the concatenated string representation.
|
||||
*
|
||||
* @param int|null $limit maximum length of the returned representation
|
||||
* @return string the string representation
|
||||
*
|
||||
* @see https://www.w3.org/TR/trace-context/#tracestate-limits
|
||||
*/
|
||||
public function toString(?int $limit = null): string;
|
||||
|
||||
/**
|
||||
* Returns a string representation of this TraceSate
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ function main(): void
|
|||
[
|
||||
new BatchSpanProcessor(
|
||||
new ZipkinExporter(
|
||||
PsrTransportFactory::discover()->create('http://zipkin:9412/api/v2/spans')
|
||||
PsrTransportFactory::discover()->create('http://zipkin:9412/api/v2/spans', 'application/json')
|
||||
),
|
||||
ClockFactory::getDefault()
|
||||
),
|
||||
|
|
|
|||
|
|
@ -22,4 +22,4 @@ rm -rf trace-context
|
|||
git clone https://github.com/w3c/trace-context.git
|
||||
|
||||
# Run the test
|
||||
python3 "trace-context/test/test.py" http://127.0.0.1:8001/test
|
||||
SPEC_LEVEL=1 python3 "trace-context/test/test.py" http://127.0.0.1:8001/test
|
||||
|
|
|
|||
|
|
@ -4,12 +4,14 @@ declare(strict_types=1);
|
|||
|
||||
namespace OpenTelemetry\Tests\API\Unit\Trace;
|
||||
|
||||
use function array_reverse;
|
||||
use OpenTelemetry\API\Behavior\Internal\Logging;
|
||||
use OpenTelemetry\API\LoggerHolder;
|
||||
use OpenTelemetry\API\Trace\TraceState;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use function str_repeat;
|
||||
use function strlen;
|
||||
|
||||
/**
|
||||
* @covers \OpenTelemetry\API\Trace\TraceState
|
||||
|
|
@ -23,6 +25,7 @@ class TraceStateTest extends TestCase
|
|||
{
|
||||
$this->logger = $this->createMock(LoggerInterface::class);
|
||||
LoggerHolder::set($this->logger);
|
||||
Logging::reset();
|
||||
}
|
||||
|
||||
public function test_get_tracestate_value(): void
|
||||
|
|
@ -86,6 +89,16 @@ class TraceStateTest extends TestCase
|
|||
$this->assertSame('value2', $tracestate->get('vendor2'));
|
||||
}
|
||||
|
||||
public function test_with_allows_only32_members(): void
|
||||
{
|
||||
$traceState = new TraceState();
|
||||
for ($i = 0; $i < 33; $i++) {
|
||||
$traceState = $traceState->with('key' . $i, 'test');
|
||||
}
|
||||
|
||||
$this->assertSame(32, $traceState->getListMemberCount());
|
||||
}
|
||||
|
||||
public function test_to_string_tracestate(): void
|
||||
{
|
||||
$tracestate = new TraceState('vendor1=value1');
|
||||
|
|
@ -124,27 +137,18 @@ class TraceStateTest extends TestCase
|
|||
public function test_max_tracestate_length(): void
|
||||
{
|
||||
// Build a vendor key with a length of 256 characters. The max characters allowed.
|
||||
$vendorKey = str_repeat('k', TraceState::MAX_COMBINED_LENGTH / 2);
|
||||
$vendorKey = str_repeat('k', 256);
|
||||
|
||||
// Build a vendor value with a length of 255 characters. One below the max allowed.
|
||||
$vendorValue = str_repeat('v', TraceState::MAX_COMBINED_LENGTH / 2 - 1);
|
||||
// Build a vendor value with a length of 256 characters. The max characters allowed.
|
||||
$vendorValue = str_repeat('v', 256);
|
||||
|
||||
// tracestate length = 513 characters (not accepted).
|
||||
$rawTraceState = $vendorKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $vendorValue . 'v';
|
||||
$this->assertGreaterThan(TraceState::MAX_COMBINED_LENGTH, strlen($rawTraceState));
|
||||
|
||||
$validTracestate = new TraceState($rawTraceState);
|
||||
$this->assertNull($validTracestate->get($vendorKey));
|
||||
|
||||
// tracestate length = 512 characters (accepted).
|
||||
// tracestate length = 513 characters (accepted).
|
||||
$rawTraceState = $vendorKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $vendorValue;
|
||||
$this->assertSame(TraceState::MAX_COMBINED_LENGTH, strlen($rawTraceState));
|
||||
|
||||
$validTracestate = new TraceState($rawTraceState);
|
||||
$this->assertSame($rawTraceState, (string) $validTracestate);
|
||||
}
|
||||
|
||||
public function test_validate_key(): void
|
||||
public function test_parse_validate_key(): void
|
||||
{
|
||||
// Valid keys
|
||||
$validKeys = 'a-b=1,c*d=2,e/f=3,g_h=4,01@i-j=5';
|
||||
|
|
@ -163,25 +167,9 @@ class TraceStateTest extends TestCase
|
|||
|
||||
// Drop all keys on an invalid key
|
||||
$this->assertSame(0, $tracestate->getListMemberCount());
|
||||
|
||||
// Tests a valid key with a length of 256 characters. The max characters allowed.
|
||||
$validKey = str_repeat('k', 256);
|
||||
$validValue = 'v';
|
||||
$tracestate = new TraceState($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue);
|
||||
|
||||
$this->assertSame(256, strlen($validKey));
|
||||
$this->assertSame($validValue, $tracestate->get($validKey));
|
||||
$this->assertSame($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue, (string) $tracestate);
|
||||
|
||||
// Tests an invalid key with a length of 257 characters. One more than the max characters allowed.
|
||||
$invalidKey = str_repeat('k', 257);
|
||||
$tracestate = new TraceState($invalidKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue);
|
||||
|
||||
$this->assertSame(257, strlen($invalidKey));
|
||||
$this->assertNull($tracestate->get($invalidKey));
|
||||
}
|
||||
|
||||
public function test_validate_value(): void
|
||||
public function test_parse_validate_value(): void
|
||||
{
|
||||
// Tests values are within the range of 0x20 to 0x7E characters
|
||||
$tracestate = 'char1=value' . chr(0x19) . '1'
|
||||
|
|
@ -193,21 +181,117 @@ class TraceStateTest extends TestCase
|
|||
|
||||
// Entire tracestate is dropped since the value for char1 is invalid.
|
||||
$this->assertSame(0, $parsedTracestate->getListMemberCount());
|
||||
}
|
||||
|
||||
// Tests a valid value with a length of 256 characters. The max allowed.
|
||||
$validValue = str_repeat('v', 256);
|
||||
$validKey = 'k';
|
||||
$tracestate = new TraceState($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue);
|
||||
#[DataProvider('validKeyValueProvider')]
|
||||
public function test_valid_key_value(string $key, string $value): void
|
||||
{
|
||||
$traceState = new TraceState();
|
||||
$traceState = $traceState->with($key, $value);
|
||||
|
||||
$this->assertSame(256, strlen($validValue));
|
||||
$this->assertSame($validValue, $tracestate->get($validKey));
|
||||
$this->assertSame($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue, (string) $tracestate);
|
||||
$this->assertSame($value, $traceState->get($key));
|
||||
$this->assertSame($key . '=' . $value, (string) $traceState);
|
||||
}
|
||||
|
||||
// Tests an invalid value with a length of 257 characters. One more than the max allowed.
|
||||
$invalidValue = str_repeat('v', 257);
|
||||
$tracestate = new TraceState($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $invalidValue);
|
||||
public static function validKeyValueProvider(): array
|
||||
{
|
||||
return [
|
||||
['valid@key', 'value'],
|
||||
['valid@key', '0'],
|
||||
['abcdefghijklmnopqrstuvwxyz0123456789_-*/', 'value'],
|
||||
['abcdefghijklmnopqrstuvwxyz0123456789_-*/@a1234_-*/', 'value'],
|
||||
['key', strtr(implode(range(chr(0x20), chr(0x7E))), [',' => '', '=' => ''])],
|
||||
[str_repeat('a', 256), 'value'],
|
||||
['key', str_repeat('a', 256)],
|
||||
];
|
||||
}
|
||||
|
||||
$this->assertSame(257, strlen($invalidValue));
|
||||
$this->assertNull($tracestate->get($validKey));
|
||||
#[DataProvider('invalidKeyValueProvider')]
|
||||
public function test_invalid_key_value(string $key, string $value): void
|
||||
{
|
||||
$traceState = new TraceState();
|
||||
$traceState = $traceState->with($key, $value);
|
||||
|
||||
$this->assertNull($traceState->get($key));
|
||||
}
|
||||
|
||||
public static function invalidKeyValueProvider(): array
|
||||
{
|
||||
return [
|
||||
['', 'value'],
|
||||
['invalid.key', 'value'],
|
||||
['invalid-key@', 'value'],
|
||||
['0invalid-key', 'value'],
|
||||
[str_repeat('a', 257), 'value'],
|
||||
['invalid@key_____________', 'value'],
|
||||
['-invalid-key@id', 'value'],
|
||||
['invalid-key@0id', 'value'],
|
||||
['invalid-key@@id', 'value'],
|
||||
|
||||
['key', ''],
|
||||
['key', 'invalid-value '],
|
||||
['key', 'invalid,value'],
|
||||
['key', 'invalid=value'],
|
||||
['key', str_repeat('a', 257)],
|
||||
];
|
||||
}
|
||||
|
||||
#[DataProvider('toStringProvider')]
|
||||
public function test_to_string(array $entries, ?int $limit, string $expected): void
|
||||
{
|
||||
$traceState = new TraceState();
|
||||
foreach (array_reverse($entries) as $key => $value) {
|
||||
$traceState = $traceState->with($key, $value);
|
||||
}
|
||||
|
||||
$this->assertSame($expected, $traceState->toString(limit: $limit));
|
||||
}
|
||||
|
||||
public static function toStringProvider(): array
|
||||
{
|
||||
return [
|
||||
[[], null, ''],
|
||||
[['key' => 'value'], null, 'key=value'],
|
||||
[['key' => 'value', 'key2' => 'value2'], null, 'key=value,key2=value2'],
|
||||
[['key' => 'value', 'key2' => 'value2'], 21, 'key=value,key2=value2'],
|
||||
[['key' => 'value', 'key2' => 'value2'], 14, 'key=value'],
|
||||
[['key' => 'value', 'key2' => 'value2'], 9, 'key=value'],
|
||||
[['key' => 'value', 'key2' => 'value2'], 5, ''],
|
||||
[['key' => 'value', 'a' => 'b'], 5, ''],
|
||||
[[str_repeat('a', 10) => str_repeat('v', 50), 'key' => 'value', 'key2' => 'value2'], 50, ''],
|
||||
[[str_repeat('a', 10) => str_repeat('v', 150), 'key' => 'value', 'key2' => 'value2'], 50, 'key=value,key2=value2'],
|
||||
[[str_repeat('a', 10) => str_repeat('v', 117), 'key' => 'value', 'key2' => 'value2'], 50, ''],
|
||||
[[str_repeat('a', 10) => str_repeat('v', 118), 'key' => 'value', 'key2' => 'value2'], 50, 'key=value,key2=value2'],
|
||||
[[str_repeat('a', 10) => str_repeat('v', 118), 'key' => 'value', 'key2' => 'value2'], 14, 'key=value'],
|
||||
];
|
||||
}
|
||||
|
||||
#[DataProvider('parseProvider')]
|
||||
public function test_parse(string $tracestate, ?string $expected): void
|
||||
{
|
||||
$this->logger
|
||||
->expects($expected === null ? $this->once() : $this->never())
|
||||
->method('log')
|
||||
->with('warning', $this->anything(), $this->anything());
|
||||
|
||||
$traceState = new TraceState($tracestate);
|
||||
$this->assertSame($expected ?? '', (string) $traceState);
|
||||
}
|
||||
|
||||
public static function parseProvider(): array
|
||||
{
|
||||
return [
|
||||
['key=value', 'key=value'],
|
||||
['key=value,key2=value2', 'key=value,key2=value2'],
|
||||
['key=value,key2=value2,key=value3', 'key=value,key2=value2'],
|
||||
|
||||
['', ''],
|
||||
[' ', ''],
|
||||
['key=value,,key2=value2', 'key=value,key2=value2'],
|
||||
['key=value, ,key2=value2', 'key=value,key2=value2'],
|
||||
|
||||
['0', null],
|
||||
['key =value', null],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue