From a22f9f10778efb061986a5d604da834fce10c920 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Thu, 16 Jun 2022 14:17:59 +0200 Subject: [PATCH] Cleanup/fix context package (#711) * Remove `Context::withValue()` Creates a hidden `Scope` when called without `$parent`. * Remove `Context::detach(Scope)` * Remove static `Context::attach()` * Add missing typehints * Use loop instead of recursion * Fix `MultiTextMapPropagator::extract()` clearing context * Fix `TextMapPropagator::composite()` for named arguments * Make `Context` constructor non-public * Make `Context` and `ContextKey` final * Fix `ArrayAccessGetterSetter` Resolves inconsistencies in case-insensitive handling. * Fix psalm * Fix cannot mock final class `Context` Revert after introducing ContextInterface? * Apply feedback and remove obsolete parentheses * Use psalm-suppress instead of assert --- src/API/Baggage/BaggageContextKey.php | 2 +- src/API/Trace/SpanContextKey.php | 2 +- src/Context/Context.php | 94 ++++--------------- src/Context/ContextKey.php | 2 +- .../Propagation/ArrayAccessGetterSetter.php | 73 ++++++++------ .../Propagation/KeyedArrayAccessInterface.php | 16 ---- .../Propagation/MultiTextMapPropagator.php | 2 +- src/Context/Propagation/TextMapPropagator.php | 4 +- .../Integration/SDK/AlwaysOffSamplerTest.php | 2 +- tests/Integration/SDK/AlwaysOnSamplerTest.php | 2 +- tests/Integration/SDK/ParentBasedTest.php | 4 +- tests/Integration/SDK/SpanProcessorTest.php | 2 +- .../SDK/TraceIdRatioBasedSamplerTest.php | 10 +- tests/Integration/SDK/TracerTest.php | 2 +- tests/Unit/API/Trace/NoopSpanBuilderTest.php | 15 ++- .../Propagation/B3MultiPropagatorTest.php | 58 +++++------- tests/Unit/Context/ContextStorageTest.php | 6 +- tests/Unit/Context/ContextTest.php | 92 ++++-------------- .../ArrayAccessGetterSetterTest.php | 46 +++++---- .../MultiTextMapPropagatorTest.php | 12 +-- tests/Unit/Context/ScopeTest.php | 20 ++-- .../Trace/Sampler/AlwaysOffSamplerTest.php | 2 +- .../SDK/Trace/Sampler/AlwaysOnSamplerTest.php | 2 +- .../SDK/Trace/Sampler/ParentBasedTest.php | 4 +- .../Sampler/TraceIdRatioBasedSamplerTest.php | 2 +- 25 files changed, 178 insertions(+), 298 deletions(-) delete mode 100644 src/Context/Propagation/KeyedArrayAccessInterface.php diff --git a/src/API/Baggage/BaggageContextKey.php b/src/API/Baggage/BaggageContextKey.php index e59a3de58..c6b3450eb 100644 --- a/src/API/Baggage/BaggageContextKey.php +++ b/src/API/Baggage/BaggageContextKey.php @@ -10,7 +10,7 @@ /** * @psalm-internal OpenTelemetry */ -final class BaggageContextKey extends ContextKey +final class BaggageContextKey { private const KEY_NAME = 'opentelemetry-trace-baggage-key'; diff --git a/src/API/Trace/SpanContextKey.php b/src/API/Trace/SpanContextKey.php index 79e352a70..2732f7ff3 100644 --- a/src/API/Trace/SpanContextKey.php +++ b/src/API/Trace/SpanContextKey.php @@ -10,7 +10,7 @@ /** * @psalm-internal \OpenTelemetry */ -final class SpanContextKey extends ContextKey +final class SpanContextKey { private const KEY_NAME = 'opentelemetry-trace-span-key'; diff --git a/src/Context/Context.php b/src/Context/Context.php index 3d6f82a8b..213e1b2da 100644 --- a/src/Context/Context.php +++ b/src/Context/Context.php @@ -7,7 +7,7 @@ /** * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.1/specification/context/context.md#overview */ -class Context +final class Context { /** @@ -15,7 +15,7 @@ class Context */ private static ContextStorageInterface $storage; - private static ?\OpenTelemetry\Context\Context $root = null; + private static ?Context $root = null; /** * @internal @@ -36,15 +36,6 @@ public static function storage(): ContextStorageInterface return self::$storage ??= new ContextStorage(); } - /** - * This will set the given Context to be the "current" one. We return a token which can be passed to `detach()` to - * reset the Current Context back to the previous one. - */ - public static function attach(Context $ctx): ScopeInterface - { - return self::storage()->attach($ctx); - } - /** * @param non-empty-string $key * @@ -55,16 +46,6 @@ public static function createKey(string $key): ContextKey return new ContextKey($key); } - /** - * Given a token, the current context will be set back to the one prior to the token being generated. - */ - public static function detach(ScopeInterface $token): Context - { - $token->detach(); - - return self::getCurrent(); - } - public static function getCurrent(): Context { return self::storage()->current(); @@ -95,51 +76,21 @@ public static function getRoot(): self * * @return mixed */ - public static function getValue(ContextKey $key, $ctx=null) + public static function getValue(ContextKey $key, ?Context $ctx=null) { - $ctx = $ctx ?? static::getCurrent(); + $ctx = $ctx ?? self::getCurrent(); return $ctx->get($key); } - /** - * This is a static version of set(). - * This is primarily useful when the caller doesn't already have a reference to a Context that they want to mutate. - * - * There are two ways to call this function. - * 1) With a $parent parameter: - * Context::setValue($key, $value, $ctx) is functionally equivalent to $ctx->set($key, $value) - * 2) Without a $parent parameter: - * In this scenario, setValue() will use the `$current_context` reference as supplied by `getCurrent()` - * `getCurrent()` will always return a valid Context. If one does not exist at the global scope, - * an "empty" context will be created. - * - * @param ContextKey $key - * @param mixed $value - * @param Context|null $parent - * - * @return Context a new Context containing the k/v - */ - public static function withValue(ContextKey $key, $value, $parent=null) - { - if (null === $parent) { - // TODO This should not attach, leads to a context that cannot be detached - self::storage()->attach(new self($key, $value, self::getCurrent())); - - return self::getCurrent(); - } - - return new self($key, $value, $parent); - } - - protected ?ContextKey $key; + private ?ContextKey $key; /** - * @var mixed|null + * @var mixed */ - protected $value; + private $value; - protected ?\OpenTelemetry\Context\Context $parent; + private ?Context $parent; /** * This is a general purpose read-only key-value store. Read-only in the sense that adding a new value does not @@ -149,16 +100,11 @@ public static function withValue(ContextKey $key, $value, $parent=null) * to the key object, the value that corresponds to the key, and an optional reference to the parent Context * (i.e. the next link in the linked list chain) * - * If you inherit from this class, you should "shadow" $parent into your subclass so that all operations give - * you back an instance of the same type that you are interacting with and different subclasses should NOT be - * treated as interoperable. i.e. you should NOT have a Context object chain with both Context instances interleaved - * with Baggage instances. - * * @param ContextKey|null $key The key object. Should only be null when creating an "empty" context - * @param mixed|null $value - * @param self|null $parent Reference to the parent object + * @param mixed $value + * @param Context|null $parent Reference to the parent object */ - final public function __construct(?ContextKey $key=null, $value=null, $parent=null) + private function __construct(?ContextKey $key=null, $value=null, ?Context $parent=null) { $this->key = $key; $this->value = $value; @@ -174,7 +120,7 @@ final public function __construct(?ContextKey $key=null, $value=null, $parent=nu * * @return Context a new Context containing the k/v */ - public function with(ContextKey $key, $value) + public function with(ContextKey $key, $value): Context { return new self($key, $value, $this); } @@ -194,24 +140,22 @@ public function withContextValue(ImplicitContextKeyedInterface $value): Context */ public function activate(): ScopeInterface { - return self::attach($this); + return self::storage()->attach($this); } /** * Fetch a value from the Context given a key value. * - * @return mixed|null + * @return mixed */ public function get(ContextKey $key) { - if ($this->key === $key) { - return $this->value; - } - - if (null === $this->parent) { - return null; + for ($context = $this; $context; $context = $context->parent) { + if ($context->key === $key) { + return $context->value; + } } - return $this->parent->get($key); + return null; } } diff --git a/src/Context/ContextKey.php b/src/Context/ContextKey.php index 40dfbd2e2..a6d38720d 100644 --- a/src/Context/ContextKey.php +++ b/src/Context/ContextKey.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Context; -class ContextKey +final class ContextKey { private ?string $name; diff --git a/src/Context/Propagation/ArrayAccessGetterSetter.php b/src/Context/Propagation/ArrayAccessGetterSetter.php index bc1a02e02..f566ccc1d 100644 --- a/src/Context/Propagation/ArrayAccessGetterSetter.php +++ b/src/Context/Propagation/ArrayAccessGetterSetter.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Context\Propagation; -use function array_keys; +use function array_key_first; use ArrayAccess; use function get_class; use function gettype; @@ -13,7 +13,8 @@ use function is_object; use function is_string; use function sprintf; -use function strtolower; +use function strcasecmp; +use Traversable; /** * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.1/specification/context/api-propagators.md#textmap-propagator Getter and Setter. @@ -40,12 +41,13 @@ public static function getInstance(): self /** {@inheritdoc} */ public function keys($carrier): array { - if (is_array($carrier)) { - return array_keys($carrier); - } + if ($this->isSupportedCarrier($carrier)) { + $keys = []; + foreach ($carrier as $key => $_) { + $keys[] = (string) $key; + } - if ($carrier instanceof KeyedArrayAccessInterface) { - return $carrier->keys(); + return $keys; } throw new InvalidArgumentException( @@ -59,30 +61,15 @@ public function keys($carrier): array /** {@inheritdoc} */ public function get($carrier, string $key): ?string { - $lKey = strtolower($key); - if ($carrier instanceof ArrayAccess) { - return $carrier->offsetExists($lKey) ? $carrier->offsetGet($lKey) : null; - } - - if (is_array($carrier)) { - if (empty($carrier)) { - return null; + if ($this->isSupportedCarrier($carrier)) { + $value = $carrier[$this->resolveKey($carrier, $key)] ?? null; + if (is_array($value) && $value) { + $value = $value[array_key_first($value)]; } - foreach ($carrier as $k => $value) { - // Ensure traceparent and tracestate header keys are lowercase - if (is_string($k)) { - if (strtolower($k) === $lKey) { - if (is_array($value)) { - return empty($value) ? null : $value[0]; - } - - return $value; - } - } - } - - return null; + return is_string($value) + ? $value + : null; } throw new InvalidArgumentException( @@ -100,9 +87,12 @@ public function set(&$carrier, string $key, string $value): void if ($key === '') { throw new InvalidArgumentException('Unable to set value with an empty key'); } + if ($this->isSupportedCarrier($carrier)) { + if (($r = $this->resolveKey($carrier, $key)) !== $key) { + unset($carrier[$r]); + } - if ($carrier instanceof ArrayAccess || is_array($carrier)) { - $carrier[strtolower($key)] = $value; + $carrier[$key] = $value; return; } @@ -115,4 +105,25 @@ public function set(&$carrier, string $key, string $value): void ) ); } + + private function isSupportedCarrier($carrier): bool + { + return is_array($carrier) || $carrier instanceof ArrayAccess && $carrier instanceof Traversable; + } + + private function resolveKey($carrier, string $key): string + { + if (isset($carrier[$key])) { + return $key; + } + + foreach ($carrier as $k => $_) { + $k = (string) $k; + if (!strcasecmp($k, $key)) { + return $k; + } + } + + return $key; + } } diff --git a/src/Context/Propagation/KeyedArrayAccessInterface.php b/src/Context/Propagation/KeyedArrayAccessInterface.php deleted file mode 100644 index 0d585ee03..000000000 --- a/src/Context/Propagation/KeyedArrayAccessInterface.php +++ /dev/null @@ -1,16 +0,0 @@ - */ - public function keys(): array; -} diff --git a/src/Context/Propagation/MultiTextMapPropagator.php b/src/Context/Propagation/MultiTextMapPropagator.php index 93e550605..cd8c80fb9 100644 --- a/src/Context/Propagation/MultiTextMapPropagator.php +++ b/src/Context/Propagation/MultiTextMapPropagator.php @@ -51,7 +51,7 @@ public function inject(&$carrier, PropagationSetterInterface $setter = null, Con public function extract($carrier, PropagationGetterInterface $getter = null, Context $context = null): Context { - $context = $context ?? Context::getRoot(); + $context = $context ?? Context::getCurrent(); foreach ($this->propagators as $propagator) { $context = $propagator->extract($carrier, $getter, $context); diff --git a/src/Context/Propagation/TextMapPropagator.php b/src/Context/Propagation/TextMapPropagator.php index 2519bcd61..debf41a22 100644 --- a/src/Context/Propagation/TextMapPropagator.php +++ b/src/Context/Propagation/TextMapPropagator.php @@ -4,6 +4,7 @@ namespace OpenTelemetry\Context\Propagation; +use function array_key_first; use function count; final class TextMapPropagator @@ -14,7 +15,8 @@ public static function composite(TextMapPropagatorInterface ...$propagators): Te case 0: return NoopTextMapPropagator::getInstance(); case 1: - return $propagators[0]; + /** @psalm-suppress PossiblyNullArrayOffset */ + return $propagators[array_key_first($propagators)]; default: return new MultiTextMapPropagator($propagators); } diff --git a/tests/Integration/SDK/AlwaysOffSamplerTest.php b/tests/Integration/SDK/AlwaysOffSamplerTest.php index bc9b9aeb0..67b249ae7 100644 --- a/tests/Integration/SDK/AlwaysOffSamplerTest.php +++ b/tests/Integration/SDK/AlwaysOffSamplerTest.php @@ -58,6 +58,6 @@ private function createParentContext(bool $sampled, bool $isRemote, ?API\TraceSt ); } - return (new Context())->withContextValue(new NonRecordingSpan($spanContext)); + return Context::getRoot()->withContextValue(new NonRecordingSpan($spanContext)); } } diff --git a/tests/Integration/SDK/AlwaysOnSamplerTest.php b/tests/Integration/SDK/AlwaysOnSamplerTest.php index 65159c856..5de7bb062 100644 --- a/tests/Integration/SDK/AlwaysOnSamplerTest.php +++ b/tests/Integration/SDK/AlwaysOnSamplerTest.php @@ -58,6 +58,6 @@ private function createParentContext(bool $sampled, bool $isRemote, ?API\TraceSt ); } - return (new Context())->withContextValue(new NonRecordingSpan($spanContext)); + return Context::getRoot()->withContextValue(new NonRecordingSpan($spanContext)); } } diff --git a/tests/Integration/SDK/ParentBasedTest.php b/tests/Integration/SDK/ParentBasedTest.php index e37110048..fb7fe059a 100644 --- a/tests/Integration/SDK/ParentBasedTest.php +++ b/tests/Integration/SDK/ParentBasedTest.php @@ -24,7 +24,7 @@ public function test_parent_based_root_span(): void $sampler = new ParentBased($rootSampler); $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a3ce929d0e0e4736', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -104,7 +104,7 @@ private function createParentContext(bool $sampled, bool $isRemote, ?API\TraceSt ); } - return (new Context())->withContextValue(new NonRecordingSpan($spanContext)); + return Context::getRoot()->withContextValue(new NonRecordingSpan($spanContext)); } private function createMockSamplerNeverInvoked(): SamplerInterface diff --git a/tests/Integration/SDK/SpanProcessorTest.php b/tests/Integration/SDK/SpanProcessorTest.php index 12dfbb545..ae272f618 100644 --- a/tests/Integration/SDK/SpanProcessorTest.php +++ b/tests/Integration/SDK/SpanProcessorTest.php @@ -17,7 +17,7 @@ class SpanProcessorTest extends TestCase { public function test_parent_context_should_be_passed_to_span_processor(): void { - $parentContext = new Context(); + $parentContext = Context::getRoot(); $spanProcessor = $this->createMock(SpanProcessorInterface::class); $spanProcessor diff --git a/tests/Integration/SDK/TraceIdRatioBasedSamplerTest.php b/tests/Integration/SDK/TraceIdRatioBasedSamplerTest.php index c0b6f5ca2..8514b7698 100644 --- a/tests/Integration/SDK/TraceIdRatioBasedSamplerTest.php +++ b/tests/Integration/SDK/TraceIdRatioBasedSamplerTest.php @@ -21,7 +21,7 @@ public function test_never_trace_id_ratio_based_sampler_decision(): void { $sampler = new TraceIdRatioBasedSampler(0.0); $decision = $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a3ce929d0e0e4736', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -33,7 +33,7 @@ public function test_always_trace_id_ratio_based_sampler_decision(): void { $sampler = new TraceIdRatioBasedSampler(1.0); $decision = $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a3ce929d0e0e4736', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -45,7 +45,7 @@ public function test_failing_trace_id_ratio_based_sampler_decision(): void { $sampler = new TraceIdRatioBasedSampler(0.99); $decision = $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6afffffffffffffff', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -57,7 +57,7 @@ public function test_passing_trace_id_ratio_based_sampler_decision(): void { $sampler = new TraceIdRatioBasedSampler(0.01); $decision = $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a000000000000000', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -100,6 +100,6 @@ private function createParentContext(bool $sampled, bool $isRemote, ?API\TraceSt ); } - return (new Context())->withContextValue(new NonRecordingSpan($spanContext)); + return Context::getRoot()->withContextValue(new NonRecordingSpan($spanContext)); } } diff --git a/tests/Integration/SDK/TracerTest.php b/tests/Integration/SDK/TracerTest.php index 862bed484..9fdec7f8b 100644 --- a/tests/Integration/SDK/TracerTest.php +++ b/tests/Integration/SDK/TracerTest.php @@ -39,7 +39,7 @@ public function test_noop_span_should_be_started_when_sampling_result_is_drop(): public function test_sampler_may_override_parents_trace_state(): void { $parentTraceState = new TraceState('orig-key=orig_value'); - $parentContext = (new Context()) + $parentContext = Context::getRoot() ->withContextValue( new NonRecordingSpan( SpanContext::create( diff --git a/tests/Unit/API/Trace/NoopSpanBuilderTest.php b/tests/Unit/API/Trace/NoopSpanBuilderTest.php index 64446a420..904bb4a3a 100644 --- a/tests/Unit/API/Trace/NoopSpanBuilderTest.php +++ b/tests/Unit/API/Trace/NoopSpanBuilderTest.php @@ -27,7 +27,7 @@ public function test_set_parent(): void NoopSpanBuilder::class, (new NoopSpanBuilder($contextStorage))->setParent( // @todo: Create a interface for Context to allow it to be mocked - new Context() + Context::getRoot() ) ); } @@ -39,8 +39,7 @@ public function test_noop_created_span_uses_provided_context(): void $span = $this->createMock(SpanInterface::class); $span->method('getContext')->willReturn($spanContext); - $context = $this->createMock(Context::class); - $context->method('get')->with(SpanContextKey::instance())->willReturn($span); + $context = Context::getRoot()->with(SpanContextKey::instance(), $span); $contextStorage = $this->createMock(ContextStorageInterface::class); @@ -59,8 +58,7 @@ public function test_noop_created_span_uses_current_context(): void $span = $this->createMock(SpanInterface::class); $span->method('getContext')->willReturn($spanContext); - $context = $this->createMock(Context::class); - $context->method('get')->with(SpanContextKey::instance())->willReturn($span); + $context = Context::getRoot()->with(SpanContextKey::instance(), $span); $contextStorage = $this->createMock(ContextStorageInterface::class); $contextStorage->method('current')->willReturn($context); @@ -79,8 +77,7 @@ public function test_noop_created_span_doesnt_use_current_context_if_no_parent() $span = $this->createMock(SpanInterface::class); $span->method('getContext')->willReturn($spanContext); - $context = $this->createMock(Context::class); - $context->method('get')->with(SpanContextKey::instance())->willReturn($span); + $context = Context::getRoot()->with(SpanContextKey::instance(), $span); $contextStorage = $this->createMock(ContextStorageInterface::class); $contextStorage->method('current')->willReturn($context); @@ -99,8 +96,7 @@ public function test_noop_created_span_removes_is_recording_flag(): void $span = $this->createMock(SpanInterface::class); $span->method('isRecording')->willReturn(true); - $context = $this->createMock(Context::class); - $context->method('get')->with(SpanContextKey::instance())->willReturn($span); + $context = Context::getRoot()->with(SpanContextKey::instance(), $span); $contextStorage = $this->createMock(ContextStorageInterface::class); @@ -179,6 +175,7 @@ public function test_set_span_kind(): void public function test_start_span(): void { $contextStorage = $this->createMock(ContextStorageInterface::class); + $contextStorage->method('current')->willReturn(Context::getRoot()); $this->assertInstanceOf( NonRecordingSpan::class, diff --git a/tests/Unit/API/Trace/Propagation/B3MultiPropagatorTest.php b/tests/Unit/API/Trace/Propagation/B3MultiPropagatorTest.php index d08a70894..54ac2f092 100644 --- a/tests/Unit/API/Trace/Propagation/B3MultiPropagatorTest.php +++ b/tests/Unit/API/Trace/Propagation/B3MultiPropagatorTest.php @@ -77,7 +77,7 @@ public function test_inject_sampled_context(): void ) ); - $this->compareKeyCaseInsensitive( + $this->assertSame( [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, @@ -101,7 +101,7 @@ public function test_inject_non_sampled_context(): void ) ); - $this->compareKeyCaseInsensitive( + $this->assertSame( [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, @@ -124,11 +124,11 @@ public function test_extract_nothing(): void */ public function test_extract_sampled_context($sampledValue): void { - $carrier = $this->getLowerCaseKeys([ + $carrier = [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => $sampledValue, - ]); + ]; $this->assertEquals( SpanContext::createFromRemoteParent(self::TRACE_ID_BASE16, self::SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), @@ -151,11 +151,11 @@ public function sampledValueProvider() */ public function test_extract_non_sampled_context($sampledValue): void { - $carrier = $this->getLowerCaseKeys([ + $carrier = [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => $sampledValue, - ]); + ]; $this->assertEquals( SpanContext::createFromRemoteParent(self::TRACE_ID_BASE16, self::SPAN_ID_BASE16), @@ -178,11 +178,11 @@ public function notSampledValueProvider() */ public function test_extract_invalid_sampled_context($sampledValue): void { - $carrier = $this->getLowerCaseKeys([ + $carrier = [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => $sampledValue, - ]); + ]; $this->assertEquals( SpanContext::createFromRemoteParent(self::TRACE_ID_BASE16, self::SPAN_ID_BASE16), @@ -201,11 +201,11 @@ public function invalidSampledValueProvider() public function test_extract_and_inject(): void { - $extractCarrier = $this->getLowerCaseKeys([ + $extractCarrier = [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]); + ]; $context = $this->b3MultiPropagator->extract($extractCarrier); $injectCarrier = []; $this->b3MultiPropagator->inject($injectCarrier, null, $context); @@ -215,66 +215,66 @@ public function test_extract_and_inject(): void public function test_extract_empty_trace_id(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => '', B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } public function test_extract_empty_span_id(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => '', B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } public function test_invalid_trace_id(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => 'abcdefghijklmnopabcdefghijklmnop', B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } public function test_invalid_trace_id_size(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16 . '00', B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16, B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } public function test_invalid_span_id(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => 'abcdefghijklmnop', B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } public function test_invalid_span_id_size(): void { $this->assertInvalid( - $this->getLowerCaseKeys([ + [ B3MultiPropagator::TRACE_ID => self::TRACE_ID_BASE16, B3MultiPropagator::SPAN_ID => self::SPAN_ID_BASE16 . '00', B3MultiPropagator::SAMPLED => self::IS_SAMPLED, - ]) + ] ); } @@ -295,18 +295,4 @@ private function withSpanContext(SpanContextInterface $spanContext, Context $con { return $context->withContextValue(Span::wrap($spanContext)); } - - private function compareKeyCaseInsensitive(array $expected, array $actual): void - { - $expectedLower = $this->getLowerCaseKeys($expected); - $this->assertSame( - $expectedLower, - $actual - ); - } - - private function getLowerCaseKeys(array $carrier): array - { - return array_change_key_case($carrier, CASE_LOWER); - } } diff --git a/tests/Unit/Context/ContextStorageTest.php b/tests/Unit/Context/ContextStorageTest.php index 87af5989e..8d6843b9b 100644 --- a/tests/Unit/Context/ContextStorageTest.php +++ b/tests/Unit/Context/ContextStorageTest.php @@ -52,8 +52,8 @@ public function test_storage_switch_treats_unknown_id_as_main(): void public function test_storage_switch_switches_context(): void { $storage = new ContextStorage(); - $main = new Context(); - $fork = new Context(); + $main = Context::getRoot(); + $fork = Context::getRoot(); $scopeMain = $storage->attach($main); @@ -85,7 +85,7 @@ public function test_storage_switch_switches_context(): void public function test_storage_fork_keeps_forked_root(): void { $storage = new ContextStorage(); - $main = new Context(); + $main = Context::getRoot(); $scopeMain = $storage->attach($main); $storage->fork(1); diff --git a/tests/Unit/Context/ContextTest.php b/tests/Unit/Context/ContextTest.php index ecc7d0c88..60c814525 100644 --- a/tests/Unit/Context/ContextTest.php +++ b/tests/Unit/Context/ContextTest.php @@ -15,9 +15,8 @@ class ContextTest extends TestCase { public function test_activate(): void { - $context = new Context(); + $context = Context::getRoot(); - $this->assertNotSame($context, Context::getCurrent()); $context->activate(); $this->assertSame($context, Context::getCurrent()); } @@ -27,7 +26,7 @@ public function test_ctx_can_store_values_by_key(): void $key1 = new ContextKey('key1'); $key2 = new ContextKey('key2'); - $ctx = (new Context())->with($key1, 'val1')->with($key2, 'val2'); + $ctx = Context::getRoot()->with($key1, 'val1')->with($key2, 'val2'); $this->assertSame($ctx->get($key1), 'val1'); $this->assertSame($ctx->get($key2), 'val2'); @@ -38,7 +37,7 @@ public function test_set_does_not_mutate_the_original(): void $key1 = new ContextKey(); $key2 = new ContextKey(); - $parent = (new Context())->with($key1, 'foo'); + $parent = Context::getRoot()->with($key1, 'foo'); $child = $parent->with($key2, 'bar'); $this->assertSame($child->get($key1), 'foo'); @@ -55,7 +54,7 @@ public function test_ctx_key_names_are_not_ids(): void $key1 = new ContextKey($key_name); $key2 = new ContextKey($key_name); - $ctx = (new Context())->with($key1, 'val1')->with($key2, 'val2'); + $ctx = Context::getRoot()->with($key1, 'val1')->with($key2, 'val2'); $this->assertSame($ctx->get($key1), 'val1'); $this->assertSame($ctx->get($key2), 'val2'); @@ -66,7 +65,7 @@ public function test_empty_ctx_keys_are_valid(): void $key1 = new ContextKey(); $key2 = new ContextKey(); - $ctx = (new Context())->with($key1, 'val1')->with($key2, 'val2'); + $ctx = Context::getRoot()->with($key1, 'val1')->with($key2, 'val2'); $this->assertSame($ctx->get($key1), 'val1'); $this->assertSame($ctx->get($key2), 'val2'); @@ -84,7 +83,7 @@ public function test_ctx_can_store_scalar_array_null_and_obj(): void $null_key = new ContextKey(); $obj_key = new ContextKey(); - $ctx = (new Context()) + $ctx = Context::getRoot() ->with($scalar_key, $scalar_val) ->with($array_key, $array_val) ->with($null_key, $null_val) @@ -98,67 +97,47 @@ public function test_ctx_can_store_scalar_array_null_and_obj(): void public function test_storage_order_doesnt_matter(): void { + $context = Context::getRoot(); $arr = []; foreach (range(0, 9) as $i) { $r = rand(0, 100); $key = new ContextKey((string) $r); - Context::withValue($key, $r); + $context = $context->with($key, $r); $arr[$r] = $key; } ksort($arr); foreach ($arr as $v => $k) { - $this->assertSame(Context::getValue($k), $v); + $this->assertSame($context->get($k), $v); } } - public function test_static_use_of_current_doesnt_interfere_with_other_calls(): void - { - $key1 = new ContextKey(); - $key2 = new ContextKey(); - $key3 = new ContextKey(); - - Context::withValue($key1, '111'); - Context::withValue($key2, '222'); - - $ctx = Context::withValue($key3, '333', new Context()); - - $this->assertSame(Context::getValue($key1), '111'); - $this->assertSame(Context::getValue($key2), '222'); - - $this->assertSame(Context::getValue($key3, $ctx), '333'); - - $this->assertNull(Context::getValue($key1, $ctx)); - $this->assertNull(Context::getValue($key2, $ctx)); - $this->assertNull(Context::getValue($key3)); - } - public function test_reusing_key_overwrites_value(): void { $key = new ContextKey(); - $ctx = (new Context())->with($key, 'val1'); + $ctx = Context::getRoot()->with($key, 'val1'); $this->assertSame($ctx->get($key), 'val1'); $ctx = $ctx->with($key, 'val2'); $this->assertSame($ctx->get($key), 'val2'); } - public function test_ctx_value_not_found_throws(): void + public function test_ctx_value_not_found_returns_null(): void { - $ctx = (new Context())->with(new ContextKey('foo'), 'bar'); + $ctx = Context::getRoot()->with(new ContextKey('foo'), 'bar'); $this->assertNull($ctx->get(new ContextKey('baz'))); } public function test_attach_and_detach_set_current_ctx(): void { $key = new ContextKey(); - Context::attach((new Context())->with($key, '111')); + Context::getRoot()->with($key, '111')->activate(); - $token = Context::attach((new Context())->with($key, '222')); + $token = Context::getRoot()->with($key, '222')->activate(); $this->assertSame(Context::getValue($key), '222'); - Context::detach($token); + $token->detach(); $this->assertSame(Context::getValue($key), '111'); } @@ -167,47 +146,10 @@ public function test_instance_set_and_static_get_use_same_ctx(): void $key = new ContextKey('ofoba'); $val = 'foobar'; - $ctx = (new Context())->with($key, $val); - Context::attach($ctx); + $ctx = Context::getRoot()->with($key, $val); + $ctx->activate(); $this->assertSame(Context::getValue($key, $ctx), $val); $this->assertSame(Context::getValue($key, null), $val); } - - public function test_static_set_and_instance_get_use_same_ctx(): void - { - $key1 = new ContextKey(); - $key2 = new ContextKey(); - $val1 = '111'; - $val2 = '222'; - - $ctx = Context::withValue($key1, $val1); - $ctx = Context::withValue($key2, $val2, $ctx); - - $this->assertSame($ctx->get($key1), $val1); - $this->assertSame($ctx->get($key2), $val2); - } - - public function test_static_without_passed_ctx_uses_current(): void - { - $ctx = Context::withValue(new ContextKey(), '111'); - $first = Context::getCurrent(); - $this->assertSame($first, $ctx); - - $ctx = Context::withValue(new ContextKey(), '222'); - $second = Context::getCurrent(); - $this->assertSame($second, $ctx); - - $this->assertNotSame($first, $second); - } - - public function test_static_with_passed_ctx_does_not_use_current(): void - { - $key1 = new ContextKey(); - $currentCtx = Context::withValue($key1, '111'); - - $key2 = new ContextKey(); - $otherCtx = Context::withValue($key2, '222', new Context()); - $this->assertSame($currentCtx, Context::getCurrent()); - } } diff --git a/tests/Unit/Context/Propagation/ArrayAccessGetterSetterTest.php b/tests/Unit/Context/Propagation/ArrayAccessGetterSetterTest.php index d1b07485b..d9fba0da1 100644 --- a/tests/Unit/Context/Propagation/ArrayAccessGetterSetterTest.php +++ b/tests/Unit/Context/Propagation/ArrayAccessGetterSetterTest.php @@ -7,7 +7,6 @@ use ArrayObject; use InvalidArgumentException; use OpenTelemetry\Context\Propagation\ArrayAccessGetterSetter; -use OpenTelemetry\Context\Propagation\KeyedArrayAccessInterface; use PHPUnit\Framework\TestCase; use stdClass; @@ -46,9 +45,9 @@ public function test_get_numerical_key_from_carrier(): void 'b' => 'bravo', ]; $map = new ArrayAccessGetterSetter(); - $this->assertNull($map->get($carrier, '1')); + $this->assertSame('alpha', $map->get($carrier, '1')); $this->assertSame('bravo', $map->get($carrier, 'b')); - $this->assertSame([1, 'b'], $map->keys($carrier)); + $this->assertSame(['1', 'b'], $map->keys($carrier)); } public function test_get_from_unsupported_carrier(): void @@ -61,10 +60,10 @@ public function test_get_from_unsupported_carrier(): void public function test_keys_from_map_array_access(): void { - $carrier = new ArrayObject(['a' => 'alpha']); + $carrier = new ArrayObject(['a' => 'alpha', 'b' => 'bravo']); + $map = new ArrayAccessGetterSetter(); - $this->expectException(InvalidArgumentException::class); - $map->keys($carrier); + $this->assertSame(['a', 'b'], $map->keys($carrier)); } public function test_keys_from_unsupported_carrier(): void @@ -75,16 +74,6 @@ public function test_keys_from_unsupported_carrier(): void $map->keys($carrier); } - public function test_keys_keyed_array_access_object(): void - { - $carrier = $this->createMock(KeyedArrayAccessInterface::class); - $carrier->method('keys')->willReturn(['a', 'b']); - - $map = new ArrayAccessGetterSetter(); - - $this->assertSame(['a', 'b'], $map->keys($carrier)); - } - public function test_set_map_array(): void { $carrier = ['a' => 'alpha']; @@ -105,6 +94,31 @@ public function test_set_map_array_access(): void $this->assertSame('bravo', $value); } + public function test_set_map_array_access_case(): void + { + $carrier = new ArrayObject(['A' => 'alpha']); + $map = new ArrayAccessGetterSetter(); + + $map->set($carrier, 'A', 'bravo'); + $this->assertCount(1, $carrier); + $this->assertArrayHasKey('A', $carrier); + $this->assertSame('bravo', $carrier['A']); + + $map->set($carrier, 'a', 'charlie'); + $this->assertCount(1, $carrier); + $this->assertArrayHasKey('a', $carrier); + $this->assertSame('charlie', $carrier['a']); + } + + public function test_get_map_array_access_case(): void + { + $carrier = new ArrayObject(['A' => 'alpha']); + $map = new ArrayAccessGetterSetter(); + + $value = $map->get($carrier, 'A'); + $this->assertSame('alpha', $value); + } + public function test_set_unsupported_carrier(): void { $carrier = new stdClass(); diff --git a/tests/Unit/Context/Propagation/MultiTextMapPropagatorTest.php b/tests/Unit/Context/Propagation/MultiTextMapPropagatorTest.php index 28dc00cab..c38c8ac82 100644 --- a/tests/Unit/Context/Propagation/MultiTextMapPropagatorTest.php +++ b/tests/Unit/Context/Propagation/MultiTextMapPropagatorTest.php @@ -76,7 +76,7 @@ public function test_inject_delegates(): void public function test_extract_no_propagators(): void { $this->assertSame( - Context::getRoot(), + Context::getCurrent(), (new MultiTextMapPropagator([]))->extract([]) ); } @@ -85,10 +85,10 @@ public function test_extract_found_all(): void { $carrier = []; - $context1 = new Context(); - $context2 = new Context(); - $context3 = new Context(); - $expectedContext = new Context(); + $context1 = Context::getRoot(); + $context2 = Context::getRoot(); + $context3 = Context::getRoot(); + $expectedContext = Context::getRoot(); $this->propagator1->expects('extract')->with($carrier, null, $context1)->andReturn($context2); $this->propagator2->expects('extract')->with($carrier, null, $context2)->andReturn($context3); @@ -108,7 +108,7 @@ public function test_extract_not_found(): void { $carrier = []; - $context = new Context(); + $context = Context::getRoot(); $this->propagator1->expects('extract')->with($carrier, null, $context)->andReturn($context); $this->propagator2->expects('extract')->with($carrier, null, $context)->andReturn($context); diff --git a/tests/Unit/Context/ScopeTest.php b/tests/Unit/Context/ScopeTest.php index 93bc1f20f..a21debc01 100644 --- a/tests/Unit/Context/ScopeTest.php +++ b/tests/Unit/Context/ScopeTest.php @@ -18,8 +18,8 @@ class ScopeTest extends TestCase public function test_scope_close_restores_context(): void { $key = new ContextKey(); - $ctx = (new Context())->with($key, 'test'); - $scope = Context::attach($ctx); + $ctx = Context::getRoot()->with($key, 'test'); + $scope = $ctx->activate(); $this->assertSame('test', Context::getValue($key)); @@ -31,12 +31,12 @@ public function test_scope_close_restores_context(): void public function test_nested_scope(): void { $key = new ContextKey(); - $ctx1 = (new Context())->with($key, 'test1'); - $scope1 = Context::attach($ctx1); + $ctx1 = Context::getRoot()->with($key, 'test1'); + $scope1 = $ctx1->activate(); $this->assertSame('test1', Context::getValue($key)); - $ctx2 = (new Context())->with($key, 'test2'); - $scope2 = Context::attach($ctx2); + $ctx2 = Context::getRoot()->with($key, 'test2'); + $scope2 = $ctx2->activate(); $this->assertSame('test2', Context::getValue($key)); $scope2->detach(); @@ -48,7 +48,7 @@ public function test_nested_scope(): void public function test_detached_scope_detach(): void { - $scope1 = Context::attach(Context::getCurrent()); + $scope1 = Context::getCurrent()->activate(); $this->assertSame(0, $scope1->detach()); $this->assertSame(ScopeInterface::DETACHED, $scope1->detach() & ScopeInterface::DETACHED); @@ -56,8 +56,8 @@ public function test_detached_scope_detach(): void public function test_order_mismatch_scope_detach(): void { - $scope1 = Context::attach(Context::getCurrent()); - $scope2 = Context::attach(Context::getCurrent()); + $scope1 = Context::getCurrent()->activate(); + $scope2 = Context::getCurrent()->activate(); $this->assertSame(ScopeInterface::MISMATCH, $scope1->detach() & ScopeInterface::MISMATCH); $this->assertSame(0, $scope2->detach()); @@ -80,7 +80,7 @@ public function test_order_mismatch_scope_detach_depth(): void } public function test_inactive_scope_detach(): void { - $scope1 = Context::attach(Context::getCurrent()); + $scope1 = Context::getCurrent()->activate(); Context::storage()->fork(1); Context::storage()->switch(1); diff --git a/tests/Unit/SDK/Trace/Sampler/AlwaysOffSamplerTest.php b/tests/Unit/SDK/Trace/Sampler/AlwaysOffSamplerTest.php index 42e84de1f..0a0fa0955 100644 --- a/tests/Unit/SDK/Trace/Sampler/AlwaysOffSamplerTest.php +++ b/tests/Unit/SDK/Trace/Sampler/AlwaysOffSamplerTest.php @@ -20,7 +20,7 @@ class AlwaysOffSamplerTest extends TestCase */ public function test_should_sample(): void { - $parentContext = $this->createMock(Context::class); + $parentContext = Context::getRoot(); $sampler = new AlwaysOffSampler(); $decision = $sampler->shouldSample( $parentContext, diff --git a/tests/Unit/SDK/Trace/Sampler/AlwaysOnSamplerTest.php b/tests/Unit/SDK/Trace/Sampler/AlwaysOnSamplerTest.php index 4c24dd9db..a8f5c2235 100644 --- a/tests/Unit/SDK/Trace/Sampler/AlwaysOnSamplerTest.php +++ b/tests/Unit/SDK/Trace/Sampler/AlwaysOnSamplerTest.php @@ -20,7 +20,7 @@ class AlwaysOnSamplerTest extends TestCase */ public function test_should_sample(): void { - $parentContext = $this->createMock(Context::class); + $parentContext = Context::getRoot(); $sampler = new AlwaysOnSampler(); $decision = $sampler->shouldSample( $parentContext, diff --git a/tests/Unit/SDK/Trace/Sampler/ParentBasedTest.php b/tests/Unit/SDK/Trace/Sampler/ParentBasedTest.php index 6646d9e92..543078bf1 100644 --- a/tests/Unit/SDK/Trace/Sampler/ParentBasedTest.php +++ b/tests/Unit/SDK/Trace/Sampler/ParentBasedTest.php @@ -46,7 +46,7 @@ public function test_parent_based_root_span(): void $sampler = new ParentBased($rootSampler); $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a3ce929d0e0e4736', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL @@ -111,7 +111,7 @@ private function createParentContext(bool $sampled, bool $isRemote, ?API\TraceSt ); } - return (new Context())->withContextValue(new NonRecordingSpan($spanContext)); + return Context::getRoot()->withContextValue(new NonRecordingSpan($spanContext)); } private function createMockSamplerNeverInvoked(): SamplerInterface diff --git a/tests/Unit/SDK/Trace/Sampler/TraceIdRatioBasedSamplerTest.php b/tests/Unit/SDK/Trace/Sampler/TraceIdRatioBasedSamplerTest.php index db365825f..fc1cc6595 100644 --- a/tests/Unit/SDK/Trace/Sampler/TraceIdRatioBasedSamplerTest.php +++ b/tests/Unit/SDK/Trace/Sampler/TraceIdRatioBasedSamplerTest.php @@ -24,7 +24,7 @@ public function test_should_sample(): void { $sampler = new TraceIdRatioBasedSampler(1.0); $decision = $sampler->shouldSample( - new Context(), + Context::getRoot(), '4bf92f3577b34da6a3ce929d0e0e4736', 'test.opentelemetry.io', API\SpanKind::KIND_INTERNAL