diff --git a/src/API/Baggage/BaggageInterface.php b/src/API/Baggage/BaggageInterface.php index d1af694d..83f45755 100644 --- a/src/API/Baggage/BaggageInterface.php +++ b/src/API/Baggage/BaggageInterface.php @@ -16,8 +16,6 @@ interface BaggageInterface extends ImplicitContextKeyedInterface /** * Returns the {@see API\BaggageInterface} from the provided *$context*, * falling back on {@see API\BaggageInterface::getEmpty()} if there is no baggage in the provided context. - * - * @todo Implement this in the API layer */ public static function fromContext(ContextInterface $context): API\BaggageInterface; @@ -29,8 +27,6 @@ interface BaggageInterface extends ImplicitContextKeyedInterface /** * Returns the current {@see Baggage} from the current {@see ContextInterface}, * falling back on {@see API\BaggageInterface::getEmpty()} if there is no baggage in the current context. - * - * @todo Implement this in the API layer */ public static function getCurrent(): API\BaggageInterface; diff --git a/src/API/Trace/TraceState.php b/src/API/Trace/TraceState.php index 0f1c2854..6e87767b 100644 --- a/src/API/Trace/TraceState.php +++ b/src/API/Trace/TraceState.php @@ -5,12 +5,15 @@ declare(strict_types=1); namespace OpenTelemetry\API\Trace; use function array_reverse; +use OpenTelemetry\API\Behavior\LogsMessagesTrait; use function strlen; class TraceState implements TraceStateInterface { - public const MAX_TRACESTATE_LIST_MEMBERS = 32; - public const MAX_TRACESTATE_LENGTH = 512; + use LogsMessagesTrait; + + public const MAX_LIST_MEMBERS = 32; //@see https://www.w3.org/TR/trace-context/#tracestate-header-field-values + 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 = '='; private const VALID_KEY_CHAR_RANGE = '[_0-9a-z-*\/]'; @@ -39,7 +42,6 @@ class TraceState implements TraceStateInterface { $clonedTracestate = clone $this; - //TODO: Log if we can't set the value if ($this->validateKey($key) && $this->validateValue($value)) { /* @@ -54,6 +56,8 @@ class TraceState implements TraceStateInterface // 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 $clonedTracestate; @@ -66,7 +70,6 @@ class TraceState implements TraceStateInterface { $clonedTracestate = clone $this; - //TODO: Log if we can't unset the value if ($key !== '') { unset($clonedTracestate->traceState[$key]); } @@ -123,36 +126,30 @@ class TraceState implements TraceStateInterface */ private function parse(string $rawTracestate): array { + 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); - if (strlen($rawTracestate) <= self::MAX_TRACESTATE_LENGTH) { - $listMembers = explode(self::LIST_MEMBERS_SEPARATOR, $rawTracestate); + 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); - // Discard tracestate if entries exceed max length. - if (count($listMembers) > self::MAX_TRACESTATE_LIST_MEMBERS) { - // TODO: Log a message when we discard. - return []; - } - - $invalid = false; - 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 '=' - // TODO: Log if we can't validate the key and value - if (count($vendor) === 2 && ($this->validateKey($vendor[0]) && $this->validateValue($vendor[1]))) { - $parsedTracestate[$vendor[0]] = $vendor[1]; - - continue; - } - $invalid = true; - - break; - } - // Discard tracestate if any member is invalid. - if ($invalid) { return []; } + $parsedTracestate[$vendor[0]] = $vendor[1]; } /* diff --git a/tests/Unit/API/Trace/TraceStateTest.php b/tests/Unit/API/Trace/TraceStateTest.php index 19ff6a87..96ad3d48 100644 --- a/tests/Unit/API/Trace/TraceStateTest.php +++ b/tests/Unit/API/Trace/TraceStateTest.php @@ -75,7 +75,7 @@ class TraceStateTest extends TestCase public function test_max_tracestate_list_members(): void { // Build a tracestate with the max 32 values. Ex '0=0,1=1,...,31=31' - $rawTraceState = range(0, TraceState::MAX_TRACESTATE_LIST_MEMBERS - 1); + $rawTraceState = range(0, TraceState::MAX_LIST_MEMBERS - 1); array_walk($rawTraceState, static function (&$v, $k) { $v = 'k' . $k . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . 'v' . $v; }); @@ -84,14 +84,14 @@ class TraceStateTest extends TestCase * @var array $rawTraceState * @see https://github.com/vimeo/psalm/issues/6394 */ - $this->assertCount(TraceState::MAX_TRACESTATE_LIST_MEMBERS, $rawTraceState); + $this->assertCount(TraceState::MAX_LIST_MEMBERS, $rawTraceState); $validTracestate = new TraceState(implode(TraceState::LIST_MEMBERS_SEPARATOR, $rawTraceState)); - $this->assertSame(TraceState::MAX_TRACESTATE_LIST_MEMBERS, $validTracestate->getListMemberCount()); + $this->assertSame(TraceState::MAX_LIST_MEMBERS, $validTracestate->getListMemberCount()); // Add a list-member to the tracestate that exceeds the max of 32. This will cause the tracestate to be discarded. $rawTraceState[32] = 'k32' . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . 'v32'; - $this->assertCount(TraceState::MAX_TRACESTATE_LIST_MEMBERS + 1, $rawTraceState); + $this->assertCount(TraceState::MAX_LIST_MEMBERS + 1, $rawTraceState); $truncatedTracestate = new TraceState(implode(TraceState::LIST_MEMBERS_SEPARATOR, $rawTraceState)); $this->assertSame(0, $truncatedTracestate->getListMemberCount()); @@ -100,21 +100,21 @@ 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_TRACESTATE_LENGTH / 2); + $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_TRACESTATE_LENGTH / 2 - 1); + $vendorValue = str_repeat('v', TraceState::MAX_COMBINED_LENGTH / 2 - 1); // tracestate length = 513 characters (not accepted). $rawTraceState = $vendorKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $vendorValue . 'v'; - $this->assertGreaterThan(TraceState::MAX_TRACESTATE_LENGTH, strlen($rawTraceState)); + $this->assertGreaterThan(TraceState::MAX_COMBINED_LENGTH, strlen($rawTraceState)); $validTracestate = new TraceState($rawTraceState); $this->assertNull($validTracestate->get($vendorKey)); // tracestate length = 512 characters (accepted). $rawTraceState = $vendorKey . TraceState::LIST_MEMBER_KEY_VALUE_SPLITTER . $vendorValue; - $this->assertSame(TraceState::MAX_TRACESTATE_LENGTH, strlen($rawTraceState)); + $this->assertSame(TraceState::MAX_COMBINED_LENGTH, strlen($rawTraceState)); $validTracestate = new TraceState($rawTraceState); $this->assertSame($rawTraceState, (string) $validTracestate);