diff --git a/src/sentry/src/Listener/EventHandleListener.php b/src/sentry/src/Listener/EventHandleListener.php index 387450537..21bb23ca7 100644 --- a/src/sentry/src/Listener/EventHandleListener.php +++ b/src/sentry/src/Listener/EventHandleListener.php @@ -35,6 +35,8 @@ use Symfony\Component\Console\Input\InputInterface; use Throwable; +use function Hyperf\Coroutine\defer; + /** * @property InputInterface $input * @property int $exitCode @@ -262,6 +264,13 @@ protected function handleRequestReceived(object $event): void if (! $this->feature->isEnabled('request')) { return; } + + // Requests run in coroutines created by the engine, which are not wrapped by the + // CoroutineAspect, so start an isolated runtime context explicitly. It is a no-op + // when the tracing listener already started one, and is ended via defer once the + // request coroutine exits. + SentrySdk::startContext(); + defer(fn () => SentrySdk::endContext()); } /** diff --git a/src/sentry/src/Tracing/Listener/EventHandleListener.php b/src/sentry/src/Tracing/Listener/EventHandleListener.php index fa4a2b2b6..17a6d3896 100644 --- a/src/sentry/src/Tracing/Listener/EventHandleListener.php +++ b/src/sentry/src/Tracing/Listener/EventHandleListener.php @@ -268,6 +268,15 @@ protected function handleRequestReceived(HttpEvent\RequestReceived|RpcEvent\Requ return; } + // HTTP/RPC request coroutines are created by the engine (Hyperf\Engine\Coroutine::create) + // and never go through Hyperf\Coroutine\Coroutine::create, so the CoroutineAspect cannot + // start a runtime context for them. Start one explicitly and end it when the request + // coroutine exits. The defer must be registered before startTransaction(): defers run LIFO, + // so the transaction-finish defer (registered later) runs first, and the Transaction keeps + // the hub it was created with, so finishing it is unaffected by endContext. + SentrySdk::startContext(); + defer(fn () => SentrySdk::endContext()); + $request = $event->request; /** @var Dispatched $dispatched */ $dispatched = $request->getAttribute(Dispatched::class); @@ -311,6 +320,11 @@ protected function handleRequestReceived(HttpEvent\RequestReceived|RpcEvent\Requ ->setData($data) ); + // Capture the request hub while the runtime context is still active, so that + // finishing the transaction does not depend on the context lifetime and never + // touches the shared global hub after endContext() has run. + $hub = SentrySdk::getCurrentHub(); + if (! $transaction->getSampled()) { return; } @@ -324,14 +338,14 @@ protected function handleRequestReceived(HttpEvent\RequestReceived|RpcEvent\Requ ->setStartTimestamp(microtime(true)) ); - SentrySdk::getCurrentHub()->setSpan($span); + $hub->setSpan($span); - defer(function () use ($transaction, $span) { + defer(function () use ($hub, $transaction, $span) { // Make sure the span is finished after the request is handled $span->finish(); // Make sure the transaction is finished after the request is handled - SentrySdk::getCurrentHub()->setSpan($transaction); + $hub->setSpan($transaction); // Finish transaction $transaction->finish(); diff --git a/tests/Sentry/RequestRuntimeContextTest.php b/tests/Sentry/RequestRuntimeContextTest.php new file mode 100644 index 000000000..09faf4df0 --- /dev/null +++ b/tests/Sentry/RequestRuntimeContextTest.php @@ -0,0 +1,158 @@ +container = m::mock(ContainerInterface::class); + $this->container->shouldReceive('has')->with(RpcContext::class)->andReturn(false); + + // The startTransaction() and Carrier helpers resolve from the ApplicationContext + // container, so make Tracer resolvable there deterministically. + $this->container->shouldReceive('get')->with(Tracer::class)->andReturn(new Tracer()); + ApplicationContext::setContainer($this->container); + + $this->makeRequestReceived = function (): HttpRequestReceived { + $handler = new Handler('App\Controller\IndexController::index', '/test'); + $dispatched = new Dispatched([Dispatcher::FOUND, $handler, []], 'http'); + + $uri = m::mock(UriInterface::class); + $uri->shouldReceive('getPath')->andReturn('/test'); + $uri->shouldReceive('getScheme')->andReturn('http'); + + $request = m::mock(ServerRequestInterface::class); + $request->shouldReceive('getAttribute')->with(Dispatched::class)->andReturn($dispatched); + $request->shouldReceive('getUri')->andReturn($uri); + $request->shouldReceive('getMethod')->andReturn('GET'); + $request->shouldReceive('getHeaders')->andReturn([]); + $request->shouldReceive('hasHeader')->andReturn(false); + $request->shouldReceive('getHeaderLine')->andReturn(''); + + $response = m::mock(ResponseInterface::class); + + return new HttpRequestReceived($request, $response); + }; +}); + +afterEach(function () { + SentrySdk::endContext(); + m::close(); +}); + +test('tracing listener starts an isolated runtime context per request and restores on endContext', function () { + $config = new Config([ + 'sentry' => [ + 'enable' => ['request' => true], + 'tracing' => ['request' => true, 'missing_routes' => true], + ], + ]); + $feature = new Feature($config); + $listener = new TracingEventHandleListener($this->container, $config, $feature); + + $before = SentrySdk::getCurrentHub(); + $listener->process(($this->makeRequestReceived)()); + $after = SentrySdk::getCurrentHub(); + + // The request got its own hub instead of the shared global one. + expect($after)->not->toBe($before); + + // Ending the context manually restores the global hub instance. + SentrySdk::endContext(); + expect(SentrySdk::getCurrentHub())->toBe($before); +}); + +test('the deferred endContext restores the global hub when the request coroutine exits', function () { + $config = new Config([ + 'sentry' => [ + 'enable' => ['request' => true], + 'tracing' => ['request' => true, 'missing_routes' => true], + ], + ]); + $feature = new Feature($config); + $listener = new TracingEventHandleListener($this->container, $config, $feature); + $event = ($this->makeRequestReceived)(); + + $before = SentrySdk::getCurrentHub(); + $innerHub = null; + + $cid = Coroutine::create(function () use ($listener, $event, &$innerHub): void { + $listener->process($event); + $innerHub = SentrySdk::getCurrentHub(); + }); + Coroutine::join([$cid]); + + // While the request coroutine is alive it has an isolated hub... + expect($innerHub)->not->toBe($before); + // ...and once it exits, the defer ends the context automatically. + expect(SentrySdk::getCurrentHub())->toBe($before); +}); + +test('base listener starts an isolated runtime context when tracing is disabled', function () { + $config = new Config([ + 'sentry' => [ + 'enable' => ['request' => true], + 'tracing' => ['request' => false], + ], + ]); + $feature = new Feature($config); + $listener = new BaseEventHandleListener($this->container, $feature, $config, m::mock(StdoutLoggerInterface::class)); + + $before = SentrySdk::getCurrentHub(); + $listener->process(($this->makeRequestReceived)()); + + expect(SentrySdk::getCurrentHub())->not->toBe($before); + + SentrySdk::endContext(); + expect(SentrySdk::getCurrentHub())->toBe($before); +}); + +test('no runtime context is started when request features are disabled', function () { + $config = new Config([ + 'sentry' => [ + 'enable' => ['request' => false], + 'tracing' => ['request' => false], + ], + ]); + $feature = new Feature($config); + $tracingListener = new TracingEventHandleListener($this->container, $config, $feature); + $baseListener = new BaseEventHandleListener($this->container, $feature, $config, m::mock(StdoutLoggerInterface::class)); + + $before = SentrySdk::getCurrentHub(); + $tracingListener->process(($this->makeRequestReceived)()); + $baseListener->process(($this->makeRequestReceived)()); + + expect(SentrySdk::getCurrentHub())->toBe($before); +});