From 7f69f4e8f858d4a2f84682ab6493f5ed7bec45ff Mon Sep 17 00:00:00 2001 From: revoltekdaniel Date: Sun, 6 Apr 2025 00:26:42 +0200 Subject: [PATCH 1/4] Add attribute/tag to monitor cron commands --- src/Attribute/SentryMonitorCommand.php | 22 ++++ .../Compiler/CronMonitorPass.php | 34 ++++++ src/DependencyInjection/Configuration.php | 1 + src/DependencyInjection/SentryExtension.php | 30 +++++ src/EventListener/CronMonitorListener.php | 97 +++++++++++++++ src/Resources/config/schema/sentry-1.0.xsd | 1 + src/Resources/config/services.xml | 8 ++ src/SentryBundle.php | 2 + .../Compiler/CronMonitorPassTest.php | 59 +++++++++ .../DependencyInjection/ConfigurationTest.php | 1 + .../Fixtures/php/cron_monitor_disabled.php | 10 ++ .../Fixtures/php/cron_monitor_enabled.php | 10 ++ .../Fixtures/xml/cron_monitor_disabled.xml | 10 ++ .../Fixtures/xml/cron_monitor_enabled.xml | 10 ++ .../Fixtures/yml/cron_monitor_disabled.yml | 2 + .../Fixtures/yml/cron_monitor_enabled.yml | 2 + .../SentryExtensionTest.php | 17 +++ .../EventListener/CronMonitorListenerTest.php | 115 ++++++++++++++++++ 18 files changed, 431 insertions(+) create mode 100644 src/Attribute/SentryMonitorCommand.php create mode 100644 src/DependencyInjection/Compiler/CronMonitorPass.php create mode 100644 src/EventListener/CronMonitorListener.php create mode 100644 tests/DependencyInjection/Compiler/CronMonitorPassTest.php create mode 100644 tests/DependencyInjection/Fixtures/php/cron_monitor_disabled.php create mode 100644 tests/DependencyInjection/Fixtures/php/cron_monitor_enabled.php create mode 100644 tests/DependencyInjection/Fixtures/xml/cron_monitor_disabled.xml create mode 100644 tests/DependencyInjection/Fixtures/xml/cron_monitor_enabled.xml create mode 100644 tests/DependencyInjection/Fixtures/yml/cron_monitor_disabled.yml create mode 100644 tests/DependencyInjection/Fixtures/yml/cron_monitor_enabled.yml create mode 100644 tests/EventListener/CronMonitorListenerTest.php diff --git a/src/Attribute/SentryMonitorCommand.php b/src/Attribute/SentryMonitorCommand.php new file mode 100644 index 00000000..d4570e81 --- /dev/null +++ b/src/Attribute/SentryMonitorCommand.php @@ -0,0 +1,22 @@ +slug = $slug; + } + + public function getSlug(): string + { + return $this->slug; + } +} diff --git a/src/DependencyInjection/Compiler/CronMonitorPass.php b/src/DependencyInjection/Compiler/CronMonitorPass.php new file mode 100644 index 00000000..80485382 --- /dev/null +++ b/src/DependencyInjection/Compiler/CronMonitorPass.php @@ -0,0 +1,34 @@ +getParameter('sentry.cron.enabled')) { + return; + } + + $commands = $this->findAndSortTaggedServices('sentry.monitor_command', $container); + + $commandSlugMapping = []; + foreach ($commands as $reference) { + $id = $reference->__toString(); + foreach ($container->getDefinition($id)->getTag('sentry.monitor_command') as $attributes) { + $commandSlugMapping[$id] = $attributes['slug']; + } + } + + $container->getDefinition(CronMonitorListener::class)->setArgument(1, $commandSlugMapping); + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index ca095895..4d2844c8 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -46,6 +46,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->booleanNode('register_error_listener')->defaultTrue()->end() ->booleanNode('register_error_handler')->defaultTrue()->end() + ->booleanNode('register_cron_monitor')->defaultTrue()->end() ->scalarNode('logger') ->info('The service ID of the PSR-3 logger used to log messages coming from the SDK client. Be aware that setting the same logger of the application may create a circular loop when an event fails to be sent.') ->defaultNull() diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index a05a62e8..52df761f 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -13,7 +13,9 @@ use Sentry\Integration\RequestFetcherInterface; use Sentry\Integration\RequestIntegration; use Sentry\Options; +use Sentry\SentryBundle\Attribute\SentryMonitorCommand; use Sentry\SentryBundle\EventListener\ConsoleListener; +use Sentry\SentryBundle\EventListener\CronMonitorListener; use Sentry\SentryBundle\EventListener\ErrorListener; use Sentry\SentryBundle\EventListener\LoginListener; use Sentry\SentryBundle\EventListener\MessengerListener; @@ -29,6 +31,7 @@ use Symfony\Bundle\TwigBundle\TwigBundle; use Symfony\Component\Cache\CacheItem; use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Loader; @@ -77,6 +80,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container $this->registerTwigTracingConfiguration($container, $mergedConfig['tracing']); $this->registerCacheTracingConfiguration($container, $mergedConfig['tracing']); $this->registerHttpClientTracingConfiguration($container, $mergedConfig['tracing']); + $this->registerCronMonitoringConfiguration($container, $mergedConfig); if (!interface_exists(TokenStorageInterface::class)) { $container->removeDefinition(LoginListener::class); @@ -285,6 +289,32 @@ private function registerHttpClientTracingConfiguration(ContainerBuilder $contai $container->setParameter('sentry.tracing.http_client.enabled', $isConfigEnabled); } + /** + * @param array $config + */ + private function registerCronMonitoringConfiguration(ContainerBuilder $container, array $config): void + { + $container->setParameter('sentry.cron.enabled', (bool) $config['register_cron_monitor']); + + if (!$config['register_cron_monitor']) { + $container->removeDefinition(CronMonitorListener::class); + + return; + } + + if (\PHP_VERSION > 8.1) { + $container->registerAttributeForAutoconfiguration( + SentryMonitorCommand::class, + static function ( + ChildDefinition $definition, + SentryMonitorCommand $attribute + ) { + $definition->addTag('sentry.monitor_command', ['slug' => $attribute->getSlug()]); + } + ); + } + } + /** * @param string[] $integrations * @param array $config diff --git a/src/EventListener/CronMonitorListener.php b/src/EventListener/CronMonitorListener.php new file mode 100644 index 00000000..a26e9714 --- /dev/null +++ b/src/EventListener/CronMonitorListener.php @@ -0,0 +1,97 @@ + + */ + private $registeredCommands; + + /** + * @param HubInterface $hub + * @param array $registeredCommands + */ + public function __construct(HubInterface $hub, array $registeredCommands = []) + { + $this->hub = $hub; + $this->registeredCommands = $registeredCommands; + } + + public function handleConsoleCommandEvent(ConsoleCommandEvent $event): void + { + $command = $event->getCommand(); + + if (false === $this->isValid($command)) { + return; + } + + $checkinId = $this->hub->captureCheckIn( + $this->registeredCommands[$this->getCommandIndex($command)], + CheckInStatus::inProgress() + ); + } + + public function handleConsoleTerminateEvent(ConsoleTerminateEvent $event): void + { + $command = $event->getCommand(); + + if (false === $this->isValid($command)) { + return; + } + + $this->hub->captureCheckIn( + $this->registeredCommands[$this->getCommandIndex($command)], + Command::SUCCESS === $event->getExitCode() + ? CheckInStatus::ok() + : CheckInStatus::error() + ); + } + + public function handleConsoleErrorEvent(ConsoleErrorEvent $event): void + { + $command = $event->getCommand(); + + if (false === $this->isValid($command)) { + return; + } + + $this->hub->captureCheckIn( + $this->registeredCommands[$this->getCommandIndex($command)], + CheckInStatus::error() + ); + } + + private function isValid(?Command $command): bool + { + return $command instanceof Command && isset($this->registeredCommands[$this->getCommandIndex($command)]); + } + + private function getCommandIndex(?Command $command): string + { + if (null === $command) { + return ''; + } + + if (\PHP_VERSION > 8.0) { + return $command::class; + } + + return \get_class($command); + } +} diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index 3bad5d57..d0e81ad0 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -16,6 +16,7 @@ + diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index ff0407e0..939df340 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -26,6 +26,14 @@ + + + + + + + + diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 596b5b94..919411e7 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -6,6 +6,7 @@ use Sentry\SentryBundle\DependencyInjection\Compiler\AddLoginListenerTagPass; use Sentry\SentryBundle\DependencyInjection\Compiler\CacheTracingPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\CronMonitorPass; use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; use Sentry\SentryBundle\DependencyInjection\Compiler\HttpClientTracingPass; use Symfony\Component\DependencyInjection\Compiler\PassConfig; @@ -26,5 +27,6 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new CacheTracingPass()); $container->addCompilerPass(new HttpClientTracingPass()); $container->addCompilerPass(new AddLoginListenerTagPass()); + $container->addCompilerPass(new CronMonitorPass()); } } diff --git a/tests/DependencyInjection/Compiler/CronMonitorPassTest.php b/tests/DependencyInjection/Compiler/CronMonitorPassTest.php new file mode 100644 index 00000000..c08c1233 --- /dev/null +++ b/tests/DependencyInjection/Compiler/CronMonitorPassTest.php @@ -0,0 +1,59 @@ +createContainerBuilder(true); + + $container->setDefinition( + SentryTestCommand::class, + (new Definition(SentryTestCommand::class)) + ->setPublic(false) + ->addTag('console.command') + ->addTag('sentry.monitor_command', ['slug' => 'test-command']) + ); + + $container->compile(); + + $insertedCronMonitorListenerArgument = $container->getDefinition(CronMonitorListener::class)->getArgument(1); + + $this->assertIsArray($insertedCronMonitorListenerArgument); + $this->assertArrayHasKey(SentryTestCommand::class, $insertedCronMonitorListenerArgument); + $this->assertEquals('test-command', $insertedCronMonitorListenerArgument[SentryTestCommand::class]); + } + + public function testProcessDoesNothingIfConditionsForEnablingCronIsFalse(): void + { + $container = $this->createContainerBuilder(false); + $container->compile(); + + $this->assertFalse($container->getDefinition(CronMonitorListener::class)->getArgument(1)); + } + + private function createContainerBuilder(bool $isCronActive): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->addCompilerPass(new CronMonitorPass()); + $container->setParameter('sentry.cron.enabled', $isCronActive); + + $cronMonitorListenerMock = $this->createMock(CronMonitorListener::class); + + $container->setDefinition(CronMonitorListener::class, (new Definition(\get_class($cronMonitorListenerMock))) + ->setPublic(true)) + ->setArgument(0, HubInterface::class) + ->setArgument(1, false); + + return $container; + } +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index c993f6f2..c3f62825 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -22,6 +22,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void $expectedBundleDefaultConfig = [ 'register_error_listener' => true, 'register_error_handler' => true, + 'register_cron_monitor' => true, 'logger' => null, 'options' => [ 'integrations' => [], diff --git a/tests/DependencyInjection/Fixtures/php/cron_monitor_disabled.php b/tests/DependencyInjection/Fixtures/php/cron_monitor_disabled.php new file mode 100644 index 00000000..e9aa4218 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/cron_monitor_disabled.php @@ -0,0 +1,10 @@ +loadFromExtension('sentry', [ + 'register_cron_monitor' => false, +]); diff --git a/tests/DependencyInjection/Fixtures/php/cron_monitor_enabled.php b/tests/DependencyInjection/Fixtures/php/cron_monitor_enabled.php new file mode 100644 index 00000000..2d6e08a6 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/cron_monitor_enabled.php @@ -0,0 +1,10 @@ +loadFromExtension('sentry', [ + 'register_cron_monitor' => true, +]); diff --git a/tests/DependencyInjection/Fixtures/xml/cron_monitor_disabled.xml b/tests/DependencyInjection/Fixtures/xml/cron_monitor_disabled.xml new file mode 100644 index 00000000..12311cd4 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/cron_monitor_disabled.xml @@ -0,0 +1,10 @@ + + + + + + diff --git a/tests/DependencyInjection/Fixtures/xml/cron_monitor_enabled.xml b/tests/DependencyInjection/Fixtures/xml/cron_monitor_enabled.xml new file mode 100644 index 00000000..ebc242ea --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/cron_monitor_enabled.xml @@ -0,0 +1,10 @@ + + + + + + diff --git a/tests/DependencyInjection/Fixtures/yml/cron_monitor_disabled.yml b/tests/DependencyInjection/Fixtures/yml/cron_monitor_disabled.yml new file mode 100644 index 00000000..52427e7f --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/cron_monitor_disabled.yml @@ -0,0 +1,2 @@ +sentry: + register_cron_monitor: false diff --git a/tests/DependencyInjection/Fixtures/yml/cron_monitor_enabled.yml b/tests/DependencyInjection/Fixtures/yml/cron_monitor_enabled.yml new file mode 100644 index 00000000..00882599 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/cron_monitor_enabled.yml @@ -0,0 +1,2 @@ +sentry: + register_cron_monitor: true diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index db949243..5a18967e 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -13,6 +13,7 @@ use Sentry\Options; use Sentry\SentryBundle\DependencyInjection\SentryExtension; use Sentry\SentryBundle\EventListener\ConsoleListener; +use Sentry\SentryBundle\EventListener\CronMonitorListener; use Sentry\SentryBundle\EventListener\ErrorListener; use Sentry\SentryBundle\EventListener\LoginListener; use Sentry\SentryBundle\EventListener\MessengerListener; @@ -460,6 +461,22 @@ public function releaseOptionDataProvider(): \Generator ]; } + public function testRegisterCronMonitoringConfigurationDisabled(): void + { + $container = $this->createContainerFromFixture('cron_monitor_disabled'); + + $this->assertFalse($container->getParameter('sentry.cron.enabled')); + $this->assertFalse($container->hasDefinition(CronMonitorListener::class)); + } + + public function testRegisterCronMonitoringConfiguration(): void + { + $container = $this->createContainerFromFixture('cron_monitor_enabled'); + + $this->assertTrue($container->getParameter('sentry.cron.enabled')); + $this->assertTrue($container->hasDefinition(CronMonitorListener::class)); + } + private function createContainerFromFixture(string $fixtureFile): ContainerBuilder { $container = new ContainerBuilder(new EnvPlaceholderParameterBag([ diff --git a/tests/EventListener/CronMonitorListenerTest.php b/tests/EventListener/CronMonitorListenerTest.php new file mode 100644 index 00000000..91edfad3 --- /dev/null +++ b/tests/EventListener/CronMonitorListenerTest.php @@ -0,0 +1,115 @@ +hub = $this->createMock(HubInterface::class); + } + + public function testHandleConsoleCommandEvent(): void + { + $listener = new CronMonitorListener($this->hub, [Command::class => 'bar']); + + $this->hub->expects($this->once()) + ->method('captureCheckIn') + ->with('bar', CheckInStatus::inProgress()) + ; + + $consoleEvent = new ConsoleCommandEvent(new Command('foo:bar'), new ArrayInput([]), new NullOutput()); + $listener->handleConsoleCommandEvent($consoleEvent); + } + + public function testHandleConsoleCommandEventSkipped(): void + { + $listener = new CronMonitorListener($this->hub, []); + + $this->hub->expects($this->never()) + ->method('captureCheckIn'); + + $consoleEvent = new ConsoleCommandEvent(new Command('foo:bar'), new ArrayInput([]), new NullOutput()); + $listener->handleConsoleCommandEvent($consoleEvent); + } + + public function testHandleConsoleCommandEventSkippedWithEmptyCommand(): void + { + $listener = new CronMonitorListener($this->hub, [Command::class => 'bar']); + + $this->hub->expects($this->never()) + ->method('captureCheckIn'); + + $consoleEvent = new ConsoleCommandEvent(null, new ArrayInput([]), new NullOutput()); + $listener->handleConsoleCommandEvent($consoleEvent); + } + + /** + * @dataProvider provideTerminateEvents + */ + public function testHandleConsoleTerminateEvent(ConsoleTerminateEvent $consoleEvent, CheckInStatus $expectedState): void + { + $listener = new CronMonitorListener($this->hub, [Command::class => 'bar']); + + $this->hub->expects($this->once()) + ->method('captureCheckIn') + ->with('bar', $expectedState) + ; + + $listener->handleConsoleTerminateEvent($consoleEvent); + } + + /** + * @return \Generator + */ + public function provideTerminateEvents(): \Generator + { + yield [ + new ConsoleTerminateEvent(new Command('foo:bar'), new ArrayInput([]), new NullOutput(), Command::SUCCESS), + CheckInStatus::ok(), + ]; + + yield [ + new ConsoleTerminateEvent(new Command('foo:bar'), new ArrayInput([]), new NullOutput(), Command::FAILURE), + CheckInStatus::error(), + ]; + } + + public function testHandleConsoleErrorEvent(): void + { + $listener = new CronMonitorListener($this->hub, [Command::class => 'bar']); + + $this->hub->expects($this->once()) + ->method('captureCheckIn') + ->with('bar', CheckInStatus::error()) + ; + + $consoleEvent = new ConsoleErrorEvent( + new ArrayInput([]), + new NullOutput(), + new \Exception(), + new Command('foo:bar') + ); + + $listener->handleConsoleErrorEvent($consoleEvent); + } +} From 7cd461d2dffe4948020e0088c63ad5167c4cba61 Mon Sep 17 00:00:00 2001 From: revoltekdaniel Date: Fri, 2 May 2025 21:39:12 +0200 Subject: [PATCH 2/4] Update CronMonitorListener to track and reuse check-in IDs --- src/EventListener/CronMonitorListener.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/EventListener/CronMonitorListener.php b/src/EventListener/CronMonitorListener.php index a26e9714..e4b1a325 100644 --- a/src/EventListener/CronMonitorListener.php +++ b/src/EventListener/CronMonitorListener.php @@ -23,6 +23,11 @@ class CronMonitorListener */ private $registeredCommands; + /** + * @var string|null + */ + private $checkinId; + /** * @param HubInterface $hub * @param array $registeredCommands @@ -41,7 +46,7 @@ public function handleConsoleCommandEvent(ConsoleCommandEvent $event): void return; } - $checkinId = $this->hub->captureCheckIn( + $this->checkinId = $this->hub->captureCheckIn( $this->registeredCommands[$this->getCommandIndex($command)], CheckInStatus::inProgress() ); @@ -59,7 +64,10 @@ public function handleConsoleTerminateEvent(ConsoleTerminateEvent $event): void $this->registeredCommands[$this->getCommandIndex($command)], Command::SUCCESS === $event->getExitCode() ? CheckInStatus::ok() - : CheckInStatus::error() + : CheckInStatus::error(), + null, + null, + $this->checkinId ); } @@ -73,7 +81,10 @@ public function handleConsoleErrorEvent(ConsoleErrorEvent $event): void $this->hub->captureCheckIn( $this->registeredCommands[$this->getCommandIndex($command)], - CheckInStatus::error() + CheckInStatus::error(), + null, + null, + $this->checkinId ); } From f45719dc5ac69eaa2376bcba79158480d9a4bcfb Mon Sep 17 00:00:00 2001 From: revoltekdaniel Date: Sun, 4 May 2025 18:30:47 +0200 Subject: [PATCH 3/4] add declare strict and fix import ordering --- src/Attribute/SentryMonitorCommand.php | 2 ++ src/DependencyInjection/Compiler/CronMonitorPass.php | 2 +- tests/DependencyInjection/Compiler/CronMonitorPassTest.php | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Attribute/SentryMonitorCommand.php b/src/Attribute/SentryMonitorCommand.php index d4570e81..f5d22ae4 100644 --- a/src/Attribute/SentryMonitorCommand.php +++ b/src/Attribute/SentryMonitorCommand.php @@ -1,5 +1,7 @@ Date: Sun, 4 May 2025 18:58:23 +0200 Subject: [PATCH 4/4] always use get_class instead of ::class because of support for old php versions. --- src/EventListener/CronMonitorListener.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/EventListener/CronMonitorListener.php b/src/EventListener/CronMonitorListener.php index e4b1a325..0d73cd82 100644 --- a/src/EventListener/CronMonitorListener.php +++ b/src/EventListener/CronMonitorListener.php @@ -99,10 +99,6 @@ private function getCommandIndex(?Command $command): string return ''; } - if (\PHP_VERSION > 8.0) { - return $command::class; - } - return \get_class($command); } }