Repository: open-telemetry/opentelemetry-php (php)

Issue:
`Event\Dispatcher::getInstance()` leaks scope
**Steps to reproduce**
```php
$key = Context::createKey('-');
$scope = Context::getCurrent()->with($key, 1)->activate();
Dispatcher::getInstance();
$result = $scope->detach();

assert($result === 0); // currently MISMATCH | 1
assert(Context::getCurrent()->get($key) === null); // currently 1
```

**What is the expected behavior?**
`Event\Dispatcher::getInstance()` does not create a scope.

**What is the actual behavior?**
`Event\Dispatcher::getInstance()` creates a scope that is never detached.

**Additional context**
See also https://github.com/open-telemetry/opentelemetry-php/pull/763#discussion_r923037358.


Pull request:
store root dispatcher in a static var
This seems to be the safest way to store the main dispatcher. Using context to store the main event dispatcher causes a scope to be created: activating the newly-created context which stores dispatcher causes a new scope, and that can break things.
I also looked at storing dispatcher in the root context, but that can't work because contexts are immutable.

Closes #778 

Patch:
diff --git a/src/API/Common/Event/Dispatcher.php b/src/API/Common/Event/Dispatcher.php
index 6d45888a..b34c1a70 100644
--- a/src/API/Common/Event/Dispatcher.php
+++ b/src/API/Common/Event/Dispatcher.php
@@ -5,20 +5,16 @@ declare(strict_types=1);
 namespace OpenTelemetry\API\Common\Event;
 
 use CloudEvents\V1\CloudEventInterface;
-use OpenTelemetry\Context\Context;
-use OpenTelemetry\Context\ContextKey;
 
 class Dispatcher implements DispatcherInterface
 {
-    private static ?ContextKey $key = null;
+    private static ?self $root = null;
     /** @var array<string, array<int, array<callable>>> */
     private array $listeners = [];
 
-    public static function getInstance(): self
+    public static function getRoot(): self
     {
-        $key = self::getConstantKeyInstance();
-
-        return Context::getCurrent()->get($key) ?? self::createInstance();
+        return self::$root ??= new self();
     }
 
     public function dispatch(CloudEventInterface $event): void
@@ -47,17 +43,4 @@ class Dispatcher implements DispatcherInterface
             $listener($event);
         }
     }
-
-    private static function getConstantKeyInstance(): ContextKey
-    {
-        return self::$key ??= new ContextKey(self::class);
-    }
-
-    private static function createInstance(): self
-    {
-        $dispatcher = new self();
-        Context::getCurrent()->with(self::getConstantKeyInstance(), $dispatcher)->activate();
-
-        return $dispatcher;
-    }
 }
diff --git a/src/API/Common/Event/EmitsEventsTrait.php b/src/API/Common/Event/EmitsEventsTrait.php
index f55a2326..e55c6f20 100644
--- a/src/API/Common/Event/EmitsEventsTrait.php
+++ b/src/API/Common/Event/EmitsEventsTrait.php
@@ -10,6 +10,6 @@ trait EmitsEventsTrait
 {
     protected static function emit(CloudEventInterface $event): void
     {
-        Dispatcher::getInstance()->dispatch($event);
+        Dispatcher::getRoot()->dispatch($event);
     }
 }


Test patch:
diff --git a/tests/Benchmark/EventBench.php b/tests/Benchmark/EventBench.php
index cb4e4656..28f8c2a9 100644
--- a/tests/Benchmark/EventBench.php
+++ b/tests/Benchmark/EventBench.php
@@ -17,7 +17,7 @@ class EventBench
 
     public function __construct()
     {
-        $this->dispatcher = Dispatcher::getInstance();
+        $this->dispatcher = Dispatcher::getRoot();
         $this->listener = function () {
         };
         $this->event = new CloudEvent(uniqid(), self::class, 'foo');
diff --git a/tests/Integration/API/Common/EventContextIntegrationTest.php b/tests/Integration/API/Common/EventContextIntegrationTest.php
new file mode 100644
index 00000000..ab757517
--- /dev/null
+++ b/tests/Integration/API/Common/EventContextIntegrationTest.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Tests\Integration\API\Common;
+
+use OpenTelemetry\API\Common\Event\Dispatcher;
+use OpenTelemetry\Context\Context;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @coversNothing
+ */
+class EventContextIntegrationTest extends TestCase
+{
+    public function test_event_dispatcher_does_not_leak_scope(): void
+    {
+        $key = Context::createKey('-');
+        $scope = Context::getCurrent()->with($key, 1)->activate();
+        Dispatcher::getRoot();
+        $result = $scope->detach();
+
+        $this->assertSame(0, $result);
+        $this->assertNull(Context::getCurrent()->get($key));
+    }
+}
diff --git a/tests/Unit/API/Common/Event/DispatcherTest.php b/tests/Unit/API/Common/Event/DispatcherTest.php
index 20708d9e..d7e45a30 100644
--- a/tests/Unit/API/Common/Event/DispatcherTest.php
+++ b/tests/Unit/API/Common/Event/DispatcherTest.php
@@ -6,8 +6,6 @@ namespace OpenTelemetry\Tests\Unit\API\Common\Event;
 
 use CloudEvents\V1\CloudEventInterface;
 use OpenTelemetry\API\Common\Event\Dispatcher;
-use OpenTelemetry\Context\Context;
-use OpenTelemetry\Context\ContextKey;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -31,20 +29,11 @@ class DispatcherTest extends TestCase
         $this->event->method('getType')->willReturn('foo');
     }
 
-    public function test_get_instance(): void
+    public function test_get_root_dispatcher(): void
     {
-        $dispatcher = Dispatcher::getInstance();
+        $dispatcher = Dispatcher::getRoot();
         $this->assertInstanceOf(Dispatcher::class, $dispatcher);
-        $this->assertSame($dispatcher, Dispatcher::getInstance());
-    }
-
-    public function test_get_instance_from_parent_context(): void
-    {
-        $dispatcher = Dispatcher::getInstance();
-        $this->assertInstanceOf(Dispatcher::class, $dispatcher);
-        $parent = Context::getCurrent()->with(new ContextKey('foo'), 'bar');
-        $parent->activate();
-        $this->assertSame($dispatcher, Dispatcher::getInstance());
+        $this->assertSame($dispatcher, Dispatcher::getRoot());
     }
 
     public function test_add_listener(): void
diff --git a/tests/Unit/API/Common/Event/EmitsEventsTraitTest.php b/tests/Unit/API/Common/Event/EmitsEventsTraitTest.php
index 9d06dca5..fee48d2a 100644
--- a/tests/Unit/API/Common/Event/EmitsEventsTraitTest.php
+++ b/tests/Unit/API/Common/Event/EmitsEventsTraitTest.php
@@ -20,7 +20,7 @@ class EmitsEventsTraitTest extends TestCase
         $event->method('getType')->willReturn('bar');
         $called = false;
         $class = $this->createInstance();
-        Dispatcher::getInstance()->listen($event->getType(), function () use (&$called) {
+        Dispatcher::getRoot()->listen($event->getType(), function () use (&$called) {
             $this->assertTrue(true, 'listener was called');
             $called = true;
         });