feat(breadcrumbs): add flag to reset breadcrumbs between messages (#946)

Litarnus · web-flow · commit 872879049846 · 2025-09-11T15:20:06.000+02:00
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -187,6 +187,7 @@ private function addMessengerSection(ArrayNodeDefinition $rootNode): void
                     ->{interface_exists(MessageBusInterface::class) ? 'canBeDisabled' : 'canBeEnabled'}()
                     ->children()
                         ->booleanNode('capture_soft_fails')->defaultTrue()->end()
+                        ->booleanNode('isolate_breadcrumbs_by_message')->defaultFalse()->end()
                     ->end()
                 ->end()
             ->end();
diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php
@@ -208,7 +208,7 @@ private function registerMessengerListenerConfiguration(ContainerBuilder $contai
             return;
         }
 
-        $container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails']);
+        $container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails'])->setArgument(2, $config['isolate_breadcrumbs_by_message']);
     }
 
     /**
diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php
@@ -11,6 +11,7 @@
 use Sentry\State\Scope;
 use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
+use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent;
 use Symfony\Component\Messenger\Exception\DelayedMessageHandlingException;
 use Symfony\Component\Messenger\Exception\HandlerFailedException;
 use Symfony\Component\Messenger\Exception\WrappedExceptionsInterface;
@@ -30,15 +31,25 @@ final class MessengerListener
     private $captureSoftFails;
 
     /**
-     * @param HubInterface $hub              The current hub
-     * @param bool         $captureSoftFails Whether to capture errors thrown
-     *                                       while processing a message that
-     *                                       will be retried
+     * @var bool When this is enabled, a new scope is pushed on the stack when a message
+     *           is received and will pop it again after the message was finished (either success or fail).
+     *           This allows us to have breadcrumbs only for one message and no breadcrumb is leaked into
+     *           future messages.
      */
-    public function __construct(HubInterface $hub, bool $captureSoftFails = true)
+    private $isolateBreadcrumbsByMessage;
+
+    /**
+     * @param HubInterface $hub                         The current hub
+     * @param bool         $captureSoftFails            Whether to capture errors thrown
+     *                                                  while processing a message that
+     *                                                  will be retried
+     * @param bool         $isolateBreadcrumbsByMessage Whether messages should have isolated breadcrumbs
+     */
+    public function __construct(HubInterface $hub, bool $captureSoftFails = true, bool $isolateBreadcrumbsByMessage = false)
     {
         $this->hub = $hub;
         $this->captureSoftFails = $captureSoftFails;
+        $this->isolateBreadcrumbsByMessage = $isolateBreadcrumbsByMessage;
     }
 
     /**
@@ -48,28 +59,36 @@ public function __construct(HubInterface $hub, bool $captureSoftFails = true)
      */
     public function handleWorkerMessageFailedEvent(WorkerMessageFailedEvent $event): void
     {
-        if (!$this->captureSoftFails && $event->willRetry()) {
-            return;
-        }
+        try {
+            if (!$this->captureSoftFails && $event->willRetry()) {
+                return;
+            }
 
-        $this->hub->withScope(function (Scope $scope) use ($event): void {
-            $envelope = $event->getEnvelope();
-            $exception = $event->getThrowable();
+            $this->hub->withScope(function (Scope $scope) use ($event): void {
+                $envelope = $event->getEnvelope();
+                $exception = $event->getThrowable();
 
-            $scope->setTag('messenger.receiver_name', $event->getReceiverName());
-            $scope->setTag('messenger.message_class', \get_class($envelope->getMessage()));
+                $scope->setTag('messenger.receiver_name', $event->getReceiverName());
+                $scope->setTag('messenger.message_class', \get_class($envelope->getMessage()));
 
-            /** @var BusNameStamp|null $messageBusStamp */
-            $messageBusStamp = $envelope->last(BusNameStamp::class);
+                /** @var BusNameStamp|null $messageBusStamp */
+                $messageBusStamp = $envelope->last(BusNameStamp::class);
 
-            if (null !== $messageBusStamp) {
-                $scope->setTag('messenger.message_bus', $messageBusStamp->getBusName());
-            }
+                if (null !== $messageBusStamp) {
+                    $scope->setTag('messenger.message_bus', $messageBusStamp->getBusName());
+                }
 
-            $this->captureException($exception, $event->willRetry());
-        });
+                $this->captureException($exception, $event->willRetry());
+            });
 
-        $this->flushClient();
+            $this->flushClient();
+        } finally {
+            // We always want to pop the scope at the end of this method to add the breadcrumbs
+            // to any potential event that is produced.
+            if ($this->isolateBreadcrumbsByMessage) {
+                $this->hub->popScope();
+            }
+        }
     }
 
     /**
@@ -82,6 +101,24 @@ public function handleWorkerMessageHandledEvent(WorkerMessageHandledEvent $event
         // Flush normally happens at shutdown... which only happens in the worker if it is run with a lifecycle limit
         // such as --time=X or --limit=Y. Flush immediately in a background worker.
         $this->flushClient();
+        if ($this->isolateBreadcrumbsByMessage) {
+            $this->hub->popScope();
+        }
+    }
+
+    /**
+     * Method that will push a new scope on the hub to create message local breadcrumbs that will not
+     * "leak" into future messages.
+     *
+     * @param WorkerMessageReceivedEvent $event
+     *
+     * @return void
+     */
+    public function handleWorkerMessageReceivedEvent(WorkerMessageReceivedEvent $event): void
+    {
+        if ($this->isolateBreadcrumbsByMessage) {
+            $this->hub->pushScope();
+        }
     }
 
     /**
diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd
@@ -101,6 +101,7 @@
     <xsd:complexType name="messenger">
         <xsd:attribute name="enabled" type="xsd:boolean" />
         <xsd:attribute name="capture-soft-fails" type="xsd:boolean" />
+        <xsd:attribute name="isolate-breadcrumbs-by-message" type="xsd:boolean" />
     </xsd:complexType>
 
     <xsd:complexType name="tracing">
diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml
@@ -76,6 +76,7 @@
 
             <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageFailedEvent" method="handleWorkerMessageFailedEvent" priority="50" />
             <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageHandledEvent" method="handleWorkerMessageHandledEvent" priority="50" />
+            <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent" method="handleWorkerMessageReceivedEvent" priority="50" />
         </service>
 
         <service id="Sentry\SentryBundle\EventListener\LoginListener" class="Sentry\SentryBundle\EventListener\LoginListener">
diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php
@@ -42,6 +42,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void
             'messenger' => [
                 'enabled' => interface_exists(MessageBusInterface::class),
                 'capture_soft_fails' => true,
+                'isolate_breadcrumbs_by_message' => false,
             ],
             'tracing' => [
                 'enabled' => true,
diff --git a/tests/DependencyInjection/Fixtures/php/full.php b/tests/DependencyInjection/Fixtures/php/full.php
@@ -60,6 +60,7 @@
     'messenger' => [
         'enabled' => true,
         'capture_soft_fails' => false,
+        'isolate_breadcrumbs_by_message' => true,
     ],
     'tracing' => [
         'dbal' => [
diff --git a/tests/DependencyInjection/Fixtures/xml/full.xml b/tests/DependencyInjection/Fixtures/xml/full.xml
@@ -57,7 +57,7 @@
             <sentry:ignore-exception>Symfony\Component\HttpKernel\Exception\BadRequestHttpException</sentry:ignore-exception>
             <sentry:ignore-transaction>GET tracing_ignored_transaction</sentry:ignore-transaction>
         </sentry:options>
-        <sentry:messenger enabled="true" capture-soft-fails="false" />
+        <sentry:messenger enabled="true" capture-soft-fails="false" isolate-breadcrumbs-by-message="true"/>
         <sentry:tracing>
             <sentry:dbal enabled="false">
                 <sentry:connection>default</sentry:connection>
diff --git a/tests/DependencyInjection/Fixtures/yml/full.yml b/tests/DependencyInjection/Fixtures/yml/full.yml
@@ -59,6 +59,7 @@ sentry:
     messenger:
         enabled: true
         capture_soft_fails: false
+        isolate_breadcrumbs_by_message: true
     tracing:
         dbal:
             enabled: false
diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php
@@ -41,6 +41,7 @@
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
+use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent;
 use Symfony\Component\Messenger\MessageBusInterface;
 
 abstract class SentryExtensionTest extends TestCase
@@ -138,10 +139,16 @@ public function testMessengerListener(): void
                     'method' => 'handleWorkerMessageHandledEvent',
                     'priority' => 50,
                 ],
+                [
+                    'event' => WorkerMessageReceivedEvent::class,
+                    'method' => 'handleWorkerMessageReceivedEvent',
+                    'priority' => 50,
+                ],
             ],
         ], $definition->getTags());
 
         $this->assertFalse($definition->getArgument(1));
+        $this->assertTrue($definition->getArgument(2));
     }
 
     public function testMessengerListenerIsRemovedWhenDisabled(): void
diff --git a/tests/End2End/App/Controller/MessengerController.php b/tests/End2End/App/Controller/MessengerController.php
@@ -4,6 +4,7 @@
 
 namespace Sentry\SentryBundle\Tests\End2End\App\Controller;
 
+use Psr\Log\LoggerInterface;
 use Sentry\SentryBundle\Tests\End2End\App\Messenger\FooMessage;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Messenger\MessageBusInterface;
@@ -15,9 +16,15 @@ class MessengerController
      */
     private $messenger;
 
-    public function __construct(MessageBusInterface $messenger)
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(MessageBusInterface $messenger, LoggerInterface $logger)
     {
         $this->messenger = $messenger;
+        $this->logger = $logger;
     }
 
     public function dispatchMessage(): Response
@@ -29,6 +36,7 @@ public function dispatchMessage(): Response
 
     public function dispatchUnrecoverableMessage(): Response
     {
+        $this->logger->warning('Dispatch FooMessage');
         $this->messenger->dispatch(new FooMessage(false));
 
         return new Response('Success');
diff --git a/tests/End2End/App/Messenger/FooMessageHandler.php b/tests/End2End/App/Messenger/FooMessageHandler.php
@@ -4,14 +4,27 @@
 
 namespace Sentry\SentryBundle\Tests\End2End\App\Messenger;
 
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Messenger\Exception\UnrecoverableExceptionInterface;
 
 class FooMessageHandler
 {
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
     public function __invoke(FooMessage $message): void
     {
+        $this->logger->warning('Handle FooMessage');
         if (!$message->shouldRetry()) {
-            throw new class extends \Exception implements UnrecoverableExceptionInterface { };
+            throw new class extends \Exception implements UnrecoverableExceptionInterface {
+            };
         }
 
         throw new \Exception('This is an intentional failure while handling a message of class ' . \get_class($message));
diff --git a/tests/End2End/App/messenger.yml b/tests/End2End/App/messenger.yml
@@ -6,6 +6,7 @@ services:
     class: \Sentry\SentryBundle\Tests\End2End\App\Messenger\StaticInMemoryTransportFactory
 
   Sentry\SentryBundle\Tests\End2End\App\Messenger\FooMessageHandler:
+    autowire: true
     class: \Sentry\SentryBundle\Tests\End2End\App\Messenger\FooMessageHandler
     tags:
       - { name: messenger.message_handler }
@@ -15,6 +16,18 @@ services:
     tags:
       - controller.service_arguments
 
+  Sentry\Monolog\BreadcrumbHandler:
+    arguments:
+      - '@Sentry\State\HubInterface'
+      - !php/const Monolog\Logger::WARNING
+
+monolog:
+  handlers:
+    sentry_breadcrumbs:
+      type: service
+      name: sentry_breadcrumbs
+      id: Sentry\Monolog\BreadcrumbHandler
+
 framework:
   messenger:
     transports:
@@ -28,3 +41,4 @@ framework:
 sentry:
   messenger:
     capture_soft_fails: false
+    isolate_breadcrumbs_by_message: true
diff --git a/tests/End2End/End2EndTest.php b/tests/End2End/End2EndTest.php
@@ -38,6 +38,8 @@ protected function setUp(): void
     {
         parent::setUp();
 
+        StubTransport::$events = [];
+
         file_put_contents(self::SENT_EVENTS_LOG, '');
     }
 
@@ -250,6 +252,30 @@ public function testMessengerCaptureSoftFailCanBeDisabled(): void
         $this->assertLastEventIdIsNull($client);
     }
 
+    public function testIsolateBreadcrumbsByMessage(): void
+    {
+        $this->skipIfMessengerIsMissing();
+
+        $client = static::createClient();
+
+        // Create two messages
+        $client->request('GET', '/dispatch-unrecoverable-message');
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+
+        $client->request('GET', '/dispatch-unrecoverable-message');
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+
+        $this->consumeOneMessage($client->getKernel());
+        $this->consumeOneMessage($client->getKernel());
+
+        $events = StubTransport::$events;
+        $this->assertCount(2, $events);
+
+        // Asser that both have the same number of breadcrumbs meaning that each event has isolated breadcrumbs
+        $this->assertCount(4, $events[0]->getBreadcrumbs());
+        $this->assertCount(4, $events[1]->getBreadcrumbs());
+    }
+
     private function consumeOneMessage(KernelInterface $kernel): void
     {
         $application = new Application($kernel);
diff --git a/tests/EventListener/MessengerListenerTest.php b/tests/EventListener/MessengerListenerTest.php

Original file line number	Diff line number	Diff line change
`@@ -208,7 +208,7 @@ private function registerMessengerListenerConfiguration(ContainerBuilder $contai`
`208`	`208`	`return;`
`209`	`209`	`}`
`210`	`210`
`211`		`- $container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails']);`
	`211`	`+ $container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails'])->setArgument(2, $config['isolate_breadcrumbs_by_message']);`
`212`	`212`	`}`
`213`	`213`
`214`	`214`	`/**`