Finish the transaction at the kernel.terminate event rather than at the kernel.finish_request event (#465)

Co-authored-by: Stefano Arlandini <[email protected]>

Author: rjd22

src/EventListener/TracingRequestListener.php

@@ -7,7 +7,6 @@
 use Sentry\Tracing\Transaction;
 use Sentry\Tracing\TransactionContext;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
 
 /**
  * This event listener acts on the master requests and starts a transaction
@@ -44,16 +43,12 @@ public function handleKernelRequestEvent(RequestListenerRequestEvent $event): vo
 
     /**
      * This method is called for each request handled by the framework and
-     * ends the tracing.
+     * ends the tracing on terminate after the client received the response.
      *
-     * @param FinishRequestEvent $event The event
+     * @param RequestListenerTerminateEvent $event The event
      */
-    public function handleKernelFinishRequestEvent(FinishRequestEvent $event): void
+    public function handleKernelTerminateEvent(RequestListenerTerminateEvent $event): void
     {
-        if (!$event->isMasterRequest()) {
-            return;
-        }
-
         $transaction = $this->hub->getTransaction();
 
         if (null === $transaction) {

src/Resources/config/services.xml

@@ -58,8 +58,8 @@
             <argument type="service" id="Sentry\State\HubInterface" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="handleKernelRequestEvent" priority="4" />
-            <tag name="kernel.event_listener" event="kernel.finish_request" method="handleKernelFinishRequestEvent" priority="5" />
             <tag name="kernel.event_listener" event="kernel.response" method="handleKernelResponseEvent" priority="15" />
+            <tag name="kernel.event_listener" event="kernel.terminate" method="handleKernelTerminateEvent" priority="5" />
         </service>
 
         <service id="Sentry\SentryBundle\EventListener\TracingSubRequestListener" class="Sentry\SentryBundle\EventListener\TracingSubRequestListener">

tests/EventListener/TracingRequestListenerTest.php

@@ -10,6 +10,7 @@
 use Sentry\Options;
 use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent;
 use Sentry\SentryBundle\EventListener\RequestListenerResponseEvent;
+use Sentry\SentryBundle\EventListener\RequestListenerTerminateEvent;
 use Sentry\SentryBundle\EventListener\TracingRequestListener;
 use Sentry\State\HubInterface;
 use Sentry\Tracing\SpanId;
@@ -371,4 +372,37 @@ public function testHandleResponseRequestEventDoesNothingIfNoTransactionIsSetOnH
             new Response()
         ));
     }
+
+    /**
+     * @group time-sensitive
+     */
+    public function testHandleKernelTerminateEvent(): void
+    {
+        $transaction = new Transaction(new TransactionContext());
+
+        $this->hub->expects($this->once())
+            ->method('getTransaction')
+            ->willReturn($transaction);
+
+        $this->listener->handleKernelTerminateEvent(new RequestListenerTerminateEvent(
+            $this->createMock(HttpKernelInterface::class),
+            new Request(),
+            new Response()
+        ));
+
+        $this->assertSame(microtime(true), $transaction->getEndTimestamp());
+    }
+
+    public function testHandleKernelTerminateEventDoesNothingIfNoTransactionIsSetOnHub(): void
+    {
+        $this->hub->expects($this->once())
+            ->method('getTransaction')
+            ->willReturn(null);
+
+        $this->listener->handleKernelTerminateEvent(new RequestListenerTerminateEvent(
+            $this->createMock(HttpKernelInterface::class),
+            new Request(),
+            new Response()
+        ));
+    }
 }