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

@@ -10,7 +10,7 @@
 /**
  * @psalm-internal OpenTelemetry
  */
-final class BaggageContextKey extends ContextKey
+final class BaggageContextKey
 {
     private const KEY_NAME = 'opentelemetry-trace-baggage-key';
 

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';
 

src/Context/Context.php

@@ -7,15 +7,15 @@
 /**
  * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.1/specification/context/context.md#overview
  */
-class Context
+final class Context
 {
 
     /**
      * @var ContextStorageInterface&ExecutionContextAwareInterface
      */
     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;
     }
 }

src/Context/ContextKey.php

@@ -4,7 +4,7 @@
 
 namespace OpenTelemetry\Context;
 
-class ContextKey
+final class ContextKey
 {
     private ?string $name;
 

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;
+    }
 }

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);

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);
         }

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));
     }
 }

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));
     }
 }

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