Improve fiber bound context storage (#1270)

* Improve fiber bound context storage

Now works properly in fibers once initial context is attached.

* Change `Context::storage()` return type to `ContextStorageInterface`

`ExecutionContextAwareInterface` should not be relevant for end-users / it was mainly exposed for the FFI fiber handler; calling any of its method with enabled fiber handler would have broken the storage.
Swoole context storage README creates a new storage instead of wrapping `Context::storage()`: `Context::setStorage(new SwooleContextStorage(new ContextStorage()));`.

* Add BC layer for execution context aware fiber storage

* Fix BC layer inactive execution context detection for {main}

src/Context/Context.php

@@ -44,7 +44,7 @@ final class Context implements ContextInterface
     public static function storage(): ContextStorageInterface&ExecutionContextAwareInterface
     {
         /** @psalm-suppress RedundantPropertyInitializationCheck */
-        return self::$storage ??= new ContextStorage();
+        return self::$storage ??= new FiberBoundContextStorageExecutionAwareBC();
     }
 
     /**

src/Context/ContextStorage.php

@@ -7,9 +7,9 @@ namespace OpenTelemetry\Context;
 /**
  * @internal
  */
-final class ContextStorage implements ContextStorageInterface, ExecutionContextAwareInterface
+final class ContextStorage implements ContextStorageInterface, ContextStorageHeadAware, ExecutionContextAwareInterface
 {
-    public ContextStorageHead $current;
+    private ContextStorageHead $current;
     private ContextStorageHead $main;
     /** @var array<int|string, ContextStorageHead> */
     private array $forks = [];
@@ -34,6 +34,11 @@ final class ContextStorage implements ContextStorageInterface, ExecutionContextA
         unset($this->forks[$id]);
     }
 
+    public function head(): ContextStorageHead
+    {
+        return $this->current;
+    }
+
     public function scope(): ?ContextStorageScopeInterface
     {
         return ($this->current->node->head ?? null) === $this->current

src/Context/ContextStorageHead.php

@@ -11,7 +11,7 @@ final class ContextStorageHead
 {
     public ?ContextStorageNode $node = null;
 
-    public function __construct(public ContextStorage $storage)
+    public function __construct(public ContextStorageHeadAware $storage)
     {
     }
 }

src/Context/ContextStorageHeadAware.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Context;
+
+/**
+ * @internal
+ */
+interface ContextStorageHeadAware
+{
+    public function head(): ?ContextStorageHead;
+}

src/Context/ContextStorageNode.php

@@ -20,26 +20,22 @@ final class ContextStorageNode implements ScopeInterface, ContextStorageScopeInt
     ) {
     }
 
-    public function offsetExists($offset): bool
+    public function offsetExists(mixed $offset): bool
     {
         return isset($this->localStorage[$offset]);
     }
 
-    /**
-     * @phan-suppress PhanUndeclaredClassAttribute
-     */
-    #[\ReturnTypeWillChange]
-    public function offsetGet($offset)
+    public function offsetGet(mixed $offset): mixed
     {
         return $this->localStorage[$offset];
     }
 
-    public function offsetSet($offset, $value): void
+    public function offsetSet(mixed $offset, mixed $value): void
     {
         $this->localStorage[$offset] = $value;
     }
 
-    public function offsetUnset($offset): void
+    public function offsetUnset(mixed $offset): void
     {
         unset($this->localStorage[$offset]);
     }
@@ -52,7 +48,7 @@ final class ContextStorageNode implements ScopeInterface, ContextStorageScopeInt
     public function detach(): int
     {
         $flags = 0;
-        if ($this->head !== $this->head->storage->current) {
+        if ($this->head !== $this->head->storage->head()) {
             $flags |= ScopeInterface::INACTIVE;
         }
 

src/Context/FiberBoundContextStorage.php

@@ -1,77 +1,79 @@
 <?php
 
-/** @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection */
-
 declare(strict_types=1);
 
 namespace OpenTelemetry\Context;
 
 use function assert;
-use function class_exists;
 use const E_USER_WARNING;
 use Fiber;
+use function spl_object_id;
+use function sprintf;
 use function trigger_error;
+use WeakMap;
 
 /**
  * @internal
- *
- * @phan-file-suppress PhanUndeclaredClassReference
- * @phan-file-suppress PhanUndeclaredClassMethod
  */
-final class FiberBoundContextStorage implements ContextStorageInterface, ExecutionContextAwareInterface
+final class FiberBoundContextStorage implements ContextStorageInterface, ContextStorageHeadAware
 {
-    public function __construct(private readonly ContextStorageInterface&ExecutionContextAwareInterface $storage)
+    /** @var WeakMap<object, ContextStorageHead> */
+    private WeakMap $heads;
+
+    public function __construct()
     {
+        $this->heads = new WeakMap();
+        $this->heads[$this] = new ContextStorageHead($this);
     }
 
-    public function fork(int|string $id): void
+    public function head(): ?ContextStorageHead
     {
-        $this->storage->fork($id);
-    }
-
-    public function switch(int|string $id): void
-    {
-        $this->storage->switch($id);
-    }
-
-    public function destroy(int|string $id): void
-    {
-        $this->storage->destroy($id);
+        return $this->heads[Fiber::getCurrent() ?? $this] ?? null;
     }
 
     public function scope(): ?ContextStorageScopeInterface
     {
-        $this->checkFiberMismatch();
+        $head = $this->heads[Fiber::getCurrent() ?? $this] ?? null;
+
+        if (!$head?->node && Fiber::getCurrent()) {
+            self::triggerNotInitializedFiberContextWarning();
 
-        if (($scope = $this->storage->scope()) === null) {
             return null;
         }
 
-        return new FiberBoundContextStorageScope($scope);
+        // Starts with empty head instead of cloned parent -> no need to check for head mismatch
+        return $head->node;
     }
 
     public function current(): ContextInterface
     {
-        $this->checkFiberMismatch();
+        $head = $this->heads[Fiber::getCurrent() ?? $this] ?? null;
 
-        return $this->storage->current();
+        if (!$head?->node && Fiber::getCurrent()) {
+            self::triggerNotInitializedFiberContextWarning();
+
+            // Fallback to {main} to preserve BC
+            $head = $this->heads[$this];
+        }
+
+        return $head->node->context ?? Context::getRoot();
     }
 
     public function attach(ContextInterface $context): ContextStorageScopeInterface
     {
-        $scope = $this->storage->attach($context);
-        assert(class_exists(Fiber::class, false));
-        $scope[Fiber::class] = Fiber::getCurrent();
+        $head = $this->heads[Fiber::getCurrent() ?? $this] ??= new ContextStorageHead($this);
 
-        return new FiberBoundContextStorageScope($scope);
+        return $head->node = new ContextStorageNode($context, $head, $head->node);
     }
 
-    private function checkFiberMismatch(): void
+    private static function triggerNotInitializedFiberContextWarning(): void
     {
-        $scope = $this->storage->scope();
-        assert(class_exists(Fiber::class, false));
-        if ($scope && $scope[Fiber::class] !== Fiber::getCurrent()) {
-            trigger_error('Fiber context switching not supported', E_USER_WARNING);
-        }
+        $fiber = Fiber::getCurrent();
+        assert($fiber !== null);
+
+        trigger_error(sprintf(
+            'Access to not initialized OpenTelemetry context in fiber (id: %d), automatic forking not supported, must attach initial fiber context manually',
+            spl_object_id($fiber),
+        ), E_USER_WARNING);
     }
 }

src/Context/FiberBoundContextStorageExecutionAwareBC.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Context;
+
+/**
+ * @internal
+ */
+final class FiberBoundContextStorageExecutionAwareBC implements ContextStorageInterface, ExecutionContextAwareInterface
+{
+    private readonly FiberBoundContextStorage $storage;
+    private ?ContextStorage $bc = null;
+
+    public function __construct()
+    {
+        $this->storage = new FiberBoundContextStorage();
+    }
+
+    public function fork(int|string $id): void
+    {
+        $this->bcStorage()->fork($id);
+    }
+
+    public function switch(int|string $id): void
+    {
+        $this->bcStorage()->switch($id);
+    }
+
+    public function destroy(int|string $id): void
+    {
+        $this->bcStorage()->destroy($id);
+    }
+
+    private function bcStorage(): ContextStorage
+    {
+        if ($this->bc === null) {
+            $this->bc = new ContextStorage();
+
+            // Copy head into $this->bc storage to preserve already attached scopes
+            /** @psalm-suppress PossiblyNullFunctionCall */
+            $head = (static fn ($storage) => $storage->heads[$storage])
+                ->bindTo(null, FiberBoundContextStorage::class)($this->storage);
+            $head->storage = $this->bc;
+
+            /** @psalm-suppress PossiblyNullFunctionCall */
+            (static fn ($storage) => $storage->current = $storage->main = $head)
+                ->bindTo(null, ContextStorage::class)($this->bc);
+        }
+
+        return $this->bc;
+    }
+
+    public function scope(): ?ContextStorageScopeInterface
+    {
+        return $this->bc
+            ? $this->bc->scope()
+            : $this->storage->scope();
+    }
+
+    public function current(): ContextInterface
+    {
+        return $this->bc
+            ? $this->bc->current()
+            : $this->storage->current();
+    }
+
+    public function attach(ContextInterface $context): ContextStorageScopeInterface
+    {
+        return $this->bc
+            ? $this->bc->attach($context)
+            : $this->storage->attach($context);
+    }
+}

src/Context/FiberBoundContextStorageScope.php

@@ -1,64 +0,0 @@
-<?php
-
-/** @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection */
-
-declare(strict_types=1);
-
-namespace OpenTelemetry\Context;
-
-use function assert;
-use function class_exists;
-use Fiber;
-
-/**
- * @internal
- *
- * @phan-file-suppress PhanUndeclaredClassReference
- * @phan-file-suppress PhanUndeclaredClassMethod
- */
-final class FiberBoundContextStorageScope implements ScopeInterface, ContextStorageScopeInterface
-{
-    public function __construct(private ContextStorageScopeInterface $scope)
-    {
-    }
-
-    public function offsetExists($offset): bool
-    {
-        return $this->scope->offsetExists($offset);
-    }
-
-    /**
-     * @phan-suppress PhanUndeclaredClassAttribute
-     */
-    #[\ReturnTypeWillChange]
-    public function offsetGet($offset)
-    {
-        return $this->scope->offsetGet($offset);
-    }
-
-    public function offsetSet($offset, $value): void
-    {
-        $this->scope->offsetSet($offset, $value);
-    }
-
-    public function offsetUnset($offset): void
-    {
-        $this->scope->offsetUnset($offset);
-    }
-
-    public function context(): ContextInterface
-    {
-        return $this->scope->context();
-    }
-
-    public function detach(): int
-    {
-        $flags = $this->scope->detach();
-        assert(class_exists(Fiber::class, false));
-        if ($this->scope[Fiber::class] !== Fiber::getCurrent()) {
-            $flags |= ScopeInterface::INACTIVE;
-        }
-
-        return $flags;
-    }
-}

src/Context/README.md

@@ -39,9 +39,9 @@ truthy value. Disabling is only recommended for applications using `exit` / `die
 
 ## Async applications
 
-### Fiber support
+### Fiber support - automatic context propagation to newly created fibers
 
-Requires `PHP >= 8.1`, an NTS build, `ext-ffi`, and setting the environment variable `OTEL_PHP_FIBERS_ENABLED` to a truthy value. Additionally `vendor/autoload.php` has to be preloaded for non-CLI SAPIs if [`ffi.enable`](https://www.php.net/manual/en/ffi.configuration.php#ini.ffi.enable) is set to `preload`.
+Requires an NTS build, `ext-ffi`, and setting the environment variable `OTEL_PHP_FIBERS_ENABLED` to a truthy value. Additionally `vendor/autoload.php` has to be preloaded for non-CLI SAPIs if [`ffi.enable`](https://www.php.net/manual/en/ffi.configuration.php#ini.ffi.enable) is set to `preload`.
 
 ### Event loops
 

src/Context/ZendObserverFiber.php

@@ -48,7 +48,7 @@ final class ZendObserverFiber
 
         try {
             $fibers = FFI::scope('OTEL_ZEND_OBSERVER_FIBER');
-        } catch (FFI\Exception $e) {
+        } catch (FFI\Exception) {
             try {
                 $fibers = FFI::load(__DIR__ . '/fiber/zend_observer_fiber.h');
             } catch (FFI\Exception $e) {
@@ -58,9 +58,12 @@ final class ZendObserverFiber
             }
         }
 
-        $fibers->zend_observer_fiber_init_register(static fn (int $initializing) => Context::storage()->fork($initializing)); //@phpstan-ignore-line
-        $fibers->zend_observer_fiber_switch_register(static fn (int $from, int $to) => Context::storage()->switch($to)); //@phpstan-ignore-line
-        $fibers->zend_observer_fiber_destroy_register(static fn (int $destroying) => Context::storage()->destroy($destroying)); //@phpstan-ignore-line
+        $storage = new ContextStorage();
+        $fibers->zend_observer_fiber_init_register(static fn (int $initializing) => $storage->fork($initializing)); //@phpstan-ignore-line
+        $fibers->zend_observer_fiber_switch_register(static fn (int $from, int $to) => $storage->switch($to)); //@phpstan-ignore-line
+        $fibers->zend_observer_fiber_destroy_register(static fn (int $destroying) => $storage->destroy($destroying)); //@phpstan-ignore-line
+
+        Context::setStorage($storage);
 
         return true;
     }

src/Context/fiber/initialize_fiber_handler.php

@@ -1,20 +1,9 @@
 <?php
 
-/** @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection */
-/** @phan-file-suppress PhanUndeclaredClassReference */
-
 declare(strict_types=1);
 
 namespace OpenTelemetry\Context;
 
-use Fiber;
-
-if (!class_exists(Fiber::class)) {
-    return;
-}
-
-if (ZendObserverFiber::isEnabled() && ZendObserverFiber::init()) {
-    // ffi fiber support enabled
-} else {
-    Context::setStorage(new FiberBoundContextStorage(Context::storage()));
+if (ZendObserverFiber::isEnabled()) {
+    ZendObserverFiber::init();
 }

tests/Integration/Context/Fiber/test_context_switching_not_supported.phpt

@@ -16,6 +16,8 @@ $scope = Context::getCurrent()
     ->activate();
 
 $fiber = new Fiber(function() use ($key) {
+    echo 'fiber(pre):' . Context::getCurrent()->get($key), PHP_EOL;
+
     $scope = Context::getCurrent()
         ->with($key, 'fiber')
         ->activate();
@@ -26,6 +28,8 @@ $fiber = new Fiber(function() use ($key) {
     echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;
 
     $scope->detach();
+
+    echo 'fiber(post):' . Context::getCurrent()->get($key), PHP_EOL;
 });
 
 echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
@@ -42,10 +46,14 @@ $scope->detach();
 --EXPECTF--
 main:main
 
-Warning: Fiber context switching not supported in %s
-fiber:fiber
+Warning: Access to not initialized OpenTelemetry context in fiber (id: %d), automatic forking not supported, must attach initial fiber context manually %s
+fiber(pre):main
 
-Warning: Fiber context switching not supported in %s
-main:fiber
+Warning: Access to not initialized OpenTelemetry context in fiber (id: %d), automatic forking not supported, must attach initial fiber context manually %s
 fiber:fiber
 main:main
+fiber:fiber
+
+Warning: Access to not initialized OpenTelemetry context in fiber (id: %d), automatic forking not supported, must attach initial fiber context manually %s
+fiber(post):main
+main:main

tests/Integration/Context/Fiber/test_context_switching_not_supported_does_not_trigger_if_attached_before_usage.phpt

@@ -0,0 +1,67 @@
+--TEST--
+Context usage in fiber without fiber support does not trigger warning if context is attached before usage.
+--SKIPIF--
+<?php if (!class_exists(Fiber::class)) die('skip requires fibers'); ?>
+--ENV--
+OTEL_PHP_FIBERS_ENABLED=0
+--FILE--
+<?php
+use OpenTelemetry\Context\Context;
+
+require_once 'vendor/autoload.php';
+
+$key = Context::createKey('-');
+$scope = Context::getCurrent()
+    ->with($key, 'main')
+    ->activate();
+
+$fiber = new Fiber(bindContext(function() use ($key) {
+    echo 'fiber(pre):' . Context::getCurrent()->get($key), PHP_EOL;
+
+    $scope = Context::getCurrent()
+        ->with($key, 'fiber')
+        ->activate();
+
+    echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;
+
+    Fiber::suspend();
+    echo 'fiber:' . Context::getCurrent()->get($key), PHP_EOL;
+
+    $scope->detach();
+
+    echo 'fiber(post):' . Context::getCurrent()->get($key), PHP_EOL;
+}));
+
+echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
+
+$fiber->start();
+echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
+
+$fiber->resume();
+echo 'main:' . Context::getCurrent()->get($key), PHP_EOL;
+
+$scope->detach();
+
+
+// see https://github.com/opentelemetry-php/context?tab=readme-ov-file#event-loops
+function bindContext(Closure $closure): Closure {
+    $context = Context::getCurrent();
+    return static function (mixed ...$args) use ($closure, $context): mixed {
+        $scope = $context->activate();
+        try {
+            return $closure(...$args);
+        } finally {
+            $scope->detach();
+        }
+    };
+}
+
+?>
+--EXPECTF--
+main:main
+fiber(pre):main
+fiber:fiber
+main:main
+fiber:fiber
+fiber(post):main
+main:main

tests/Unit/Context/DebugScopeTest.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace OpenTelemetry\Tests\Unit\Context;
 
 use Exception;
+use Fiber;
 use function ini_set;
 use OpenTelemetry\Context\Context;
 use OpenTelemetry\Context\DebugScope;
@@ -104,17 +105,10 @@ final class DebugScopeTest extends TestCase
     {
         $scope1 = Context::getCurrent()->activate();
 
-        Context::storage()->fork(1);
-        Context::storage()->switch(1);
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('different execution context');
 
-        try {
-            $this->expectException(Exception::class);
-            $this->expectExceptionMessage('different execution context');
-            $scope1->detach();
-        } finally {
-            Context::storage()->switch(0);
-            Context::storage()->destroy(1);
-        }
+        (new Fiber($scope1->detach(...)))->start();
     }
 
     public function test_missing_scope_detach(): void

tests/Unit/Context/FiberBoundContextStorageExecutionAwareBCTest.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Context;
+
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(FiberBoundContextStorageExecutionAwareBC::class)]
+final class FiberBoundContextStorageExecutionAwareBCTest extends TestCase
+{
+    public function test_retains_scope_after_bc_switch(): void
+    {
+        $storage = new FiberBoundContextStorageExecutionAwareBC();
+
+        $storage->attach($storage->current());
+
+        $scope = $storage->scope();
+
+        $storage->fork(1);
+
+        $this->assertSame($scope, $storage->scope());
+    }
+
+    public function test_inactive_scope_detach(): void
+    {
+        $storage = new FiberBoundContextStorageExecutionAwareBC();
+        $scope1 = $storage->attach($storage->current());
+
+        $storage->fork(1);
+        $storage->switch(1);
+        $this->assertSame(ScopeInterface::INACTIVE, @$scope1->detach() & ScopeInterface::INACTIVE);
+    }
+
+    public function test_storage_switch_switches_context(): void
+    {
+        $storage = new FiberBoundContextStorageExecutionAwareBC();
+        $main = $storage->current();
+        $fork = $storage->current()->with(Context::createKey('-'), 42);
+
+        $scopeMain = $storage->attach($main);
+
+        // Coroutine start
+        $storage->fork(1);
+        $storage->switch(1);
+        $this->assertSame($main, $storage->current());
+
+        $scopeFork = $storage->attach($fork);
+        $this->assertSame($fork, $storage->current());
+
+        // Coroutine suspend
+        $storage->switch(0);
+        $this->assertSame($main, $storage->current());
+
+        // Coroutine resume
+        $storage->switch(1);
+        $this->assertSame($fork, $storage->current());
+
+        $scopeFork->detach();
+
+        // Coroutine return
+        $storage->switch(0);
+        $storage->destroy(1);
+
+        $scopeMain->detach();
+    }
+}

tests/Unit/Context/ScopeTest.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace OpenTelemetry\Tests\Unit\Context;
 
+use Fiber;
 use OpenTelemetry\Context\Context;
 use OpenTelemetry\Context\ContextStorage;
 use OpenTelemetry\Context\ScopeInterface;
@@ -83,12 +84,10 @@ class ScopeTest extends TestCase
     {
         $scope1 = Context::getCurrent()->activate();
 
-        Context::storage()->fork(1);
-        Context::storage()->switch(1);
-        $this->assertSame(ScopeInterface::INACTIVE, @$scope1->detach() & ScopeInterface::INACTIVE);
+        $fiber = new Fiber(static fn () => @$scope1->detach());
+        $fiber->start();
 
-        Context::storage()->switch(0);
-        Context::storage()->destroy(1);
+        $this->assertSame(ScopeInterface::INACTIVE, $fiber->getReturn() & ScopeInterface::INACTIVE);
     }
 
     public function test_scope_context_returns_context_of_scope(): void