diff --git a/src/API/Trace/TraceState.php b/src/API/Trace/TraceState.php index b398f2dba..91eac6339 100644 --- a/src/API/Trace/TraceState.php +++ b/src/API/Trace/TraceState.php @@ -4,15 +4,22 @@ 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 */ + 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 ($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]); - } + if (!self::validateMember($this->traceState, $key, $value)) { + self::logWarning('Invalid tracestate key/value for: ' . $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 ''; + $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; + } + } + } + } } - $traceStateString=''; - foreach (array_reverse($this->traceState) as $k => $v) { - $traceStateString .=$k . self::LIST_MEMBER_KEY_VALUE_SPLITTER . $v . self::LIST_MEMBERS_SEPARATOR; + + $s = ''; + foreach ($traceState as $key => $value) { + $s && ($s .= ','); + $s .= $key; + $s .= '='; + $s .= $value; } - return rtrim($traceStateString, ','); + 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 []; - } - $parsedTracestate = []; - $listMembers = explode(self::LIST_MEMBERS_SEPARATOR, $rawTracestate); + return $this->toString(); + } - if (count($listMembers) > self::MAX_LIST_MEMBERS) { - self::logWarning('tracestate discarded, too many members'); + private static function parse(string $rawTracestate): array + { + $traceState = []; + $members = explode(',', $rawTracestate); + foreach ($members as $member) { + if (($member = trim($member, " \t")) === '') { + continue; + } - return []; - } + $member = explode('=', $member, 2); + if (count($member) !== 2) { + self::logWarning(sprintf('Incomplete list member in tracestate "%s"', $rawTracestate)); - foreach ($listMembers as $listMember) { - $vendor = explode(self::LIST_MEMBER_KEY_VALUE_SPLITTER, trim($listMember)); + return []; + } - // 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); + [$key, $value] = $member; + if (!self::validateMember($traceState, $key, $value)) { + self::logWarning(sprintf('Invalid list member "%s=%s" in tracestate "%s"', $key, $value, $rawTracestate)); return []; } - $parsedTracestate[$vendor[0]] = $vendor[1]; + + $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 @@ private function parse(string $rawTracestate): array * * @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 @@ private function validateKey(string $key): bool * * @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); diff --git a/src/API/Trace/TraceStateInterface.php b/src/API/Trace/TraceStateInterface.php index 394ea1b95..d76bd437e 100644 --- a/src/API/Trace/TraceStateInterface.php +++ b/src/API/Trace/TraceStateInterface.php @@ -50,6 +50,16 @@ public function get(string $key): ?string; */ 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 */ diff --git a/tests/TraceContext/W3CTestService/index.php b/tests/TraceContext/W3CTestService/index.php index 891d45e9d..b2787964e 100644 --- a/tests/TraceContext/W3CTestService/index.php +++ b/tests/TraceContext/W3CTestService/index.php @@ -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() ), diff --git a/tests/TraceContext/W3CTestService/trace-context-test.sh b/tests/TraceContext/W3CTestService/trace-context-test.sh index 7a6c8077b..c012c0cd7 100755 --- a/tests/TraceContext/W3CTestService/trace-context-test.sh +++ b/tests/TraceContext/W3CTestService/trace-context-test.sh @@ -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 diff --git a/tests/Unit/API/Trace/TraceStateTest.php b/tests/Unit/API/Trace/TraceStateTest.php index 890bd31aa..3d04febde 100644 --- a/tests/Unit/API/Trace/TraceStateTest.php +++ b/tests/Unit/API/Trace/TraceStateTest.php @@ -4,12 +4,14 @@ 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 @@ public function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); LoggerHolder::set($this->logger); + Logging::reset(); } public function test_get_tracestate_value(): void @@ -86,6 +89,16 @@ public function test_without_tracestate_value(): void $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 @@ public function test_max_tracestate_list_members(): void 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); - - // 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); + $vendorKey = str_repeat('k', 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)); + // Build a vendor value with a length of 256 characters. The max characters allowed. + $vendorValue = str_repeat('v', 256); - $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 @@ public function test_validate_key(): void // 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 @@ public function test_validate_value(): void // Entire tracestate is dropped since the value for char1 is invalid. $this->assertSame(0, $parsedTracestate->getListMemberCount()); + } + + #[DataProvider('validKeyValueProvider')] + public function test_valid_key_value(string $key, string $value): void + { + $traceState = new TraceState(); + $traceState = $traceState->with($key, $value); + + $this->assertSame($value, $traceState->get($key)); + $this->assertSame($key . '=' . $value, (string) $traceState); + } + + 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)], + ]; + } + + #[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)], + ]; + } - // 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('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(256, strlen($validValue)); - $this->assertSame($validValue, $tracestate->get($validKey)); - $this->assertSame($validKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $validValue, (string) $tracestate); + $this->assertSame($expected, $traceState->toString(limit: $limit)); + } - // 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 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'], + ]; + } - $this->assertSame(257, strlen($invalidValue)); - $this->assertNull($tracestate->get($validKey)); + #[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], + ]; } }