Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracestate + baggage todos #894

Merged
merged 3 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/API/Baggage/BaggageInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -29,8 +27,6 @@ public static function getBuilder(): API\BaggageBuilderInterface;
/**
* 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;

Expand Down
49 changes: 23 additions & 26 deletions src/API/Trace/TraceState.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
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-*\/]';
Expand Down Expand Up @@ -39,7 +42,6 @@ public function with(string $key, string $value): TraceStateInterface
{
$clonedTracestate = clone $this;

//TODO: Log if we can't set the value
if ($this->validateKey($key) && $this->validateValue($value)) {

/*
Expand All @@ -54,6 +56,8 @@ public function with(string $key, string $value): 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;
Expand All @@ -66,7 +70,6 @@ public function without(string $key): TraceStateInterface
{
$clonedTracestate = clone $this;

//TODO: Log if we can't unset the value
if ($key !== '') {
unset($clonedTracestate->traceState[$key]);
}
Expand Down Expand Up @@ -123,36 +126,30 @@ public function __toString(): string
*/
private function parse(string $rawTracestate): array
{
$parsedTracestate = [];
if (strlen($rawTracestate) > self::MAX_COMBINED_LENGTH) {
self::logWarning('tracestate discarded, exceeds max combined length: ' . self::MAX_COMBINED_LENGTH);

if (strlen($rawTracestate) <= self::MAX_TRACESTATE_LENGTH) {
$listMembers = explode(self::LIST_MEMBERS_SEPARATOR, $rawTracestate);
return [];
}
$parsedTracestate = [];
$listMembers = explode(self::LIST_MEMBERS_SEPARATOR, $rawTracestate);

// Discard tracestate if entries exceed max length.
if (count($listMembers) > self::MAX_TRACESTATE_LIST_MEMBERS) {
// TODO: Log a message when we discard.
return [];
}
if (count($listMembers) > self::MAX_LIST_MEMBERS) {
self::logWarning('tracestate discarded, too many members');

$invalid = false;
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 '='
// 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];
foreach ($listMembers as $listMember) {
$vendor = explode(self::LIST_MEMBER_KEY_VALUE_SPLITTER, trim($listMember));

continue;
}
$invalid = true;
// 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);

break;
}
// Discard tracestate if any member is invalid.
if ($invalid) {
return [];
}
$parsedTracestate[$vendor[0]] = $vendor[1];
}

/*
Expand Down
16 changes: 8 additions & 8 deletions tests/Unit/API/Trace/TraceStateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function test_to_string_tracestate(): void
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;
});
Expand All @@ -84,14 +84,14 @@ public function test_max_tracestate_list_members(): void
* @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());
Expand All @@ -100,21 +100,21 @@ 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_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);
Expand Down