From 639ec25581a4fd6d1b6a63f6b1a69044ac7c9465 Mon Sep 17 00:00:00 2001 From: Iain Mckay Date: Wed, 7 Jul 2021 08:34:26 +0200 Subject: [PATCH] Added new explicitly_collection_exceptions for dealing with multiple exception reporting in commands --- CHANGELOG.md | 3 +++ DependencyInjection/Configuration.php | 6 ++++++ DependencyInjection/ElasticApmExtension.php | 1 + Interactor/Config.php | 9 ++++++++- Listener/CommandListener.php | 11 ++++++++++- README.md | 10 ++++++++++ 6 files changed, 38 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5337f7..5ddfe1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 1.0.0-rc2 +- Added `explicitly_collect_exceptions` to `commands` configuration to work around multiple collection of the same exception. + ## 1.0.0-rc1 - Added basic memory tracking, disabled by default - Refactored how custom labels and custom are attached diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 9a72e6a..3255b8e 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -79,6 +79,12 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->arrayNode('commands') ->canBeDisabled() + ->children() + ->booleanNode('explicitly_collect_exceptions') + ->info('Should exceptions be explicitly collected? This can conflict with the built-in collection in PHP APM') + ->defaultTrue() + ->end() + ->end() ->end() ->end() ; diff --git a/DependencyInjection/ElasticApmExtension.php b/DependencyInjection/ElasticApmExtension.php index fcf334c..a03d9ab 100644 --- a/DependencyInjection/ElasticApmExtension.php +++ b/DependencyInjection/ElasticApmExtension.php @@ -54,6 +54,7 @@ public function load(array $configs, ContainerBuilder $container): void '$customContext' => $config['custom_context'], '$shouldCollectMemoryUsage' => $config['track_memory_usage'], '$memoryUsageLabelName' => $config['memory_usage_label'], + '$shouldExplicitlyCollectCommandExceptions' => $config['commands']['explicitly_collect_exceptions'], ] ); diff --git a/Interactor/Config.php b/Interactor/Config.php index 2e03fd9..b951267 100644 --- a/Interactor/Config.php +++ b/Interactor/Config.php @@ -24,8 +24,9 @@ class Config private $customContext; private $shouldCollectMemoryUsage; private $memoryUsageLabelName; + private $shouldExplicitlyCollectCommandExceptions; - public function __construct(array $customLabels, array $customContext, bool $shouldCollectMemoryUsage, string $memoryUsageLabelName) + public function __construct(array $customLabels, array $customContext, bool $shouldCollectMemoryUsage, string $memoryUsageLabelName, bool $shouldExplicitlyCollectCommandExceptions) { if (0 === strlen($memoryUsageLabelName)) { throw new ConfigurationException('$memoryUsageLabelName cannot be blank'); @@ -35,6 +36,7 @@ public function __construct(array $customLabels, array $customContext, bool $sho $this->customContext = $customContext; $this->shouldCollectMemoryUsage = $shouldCollectMemoryUsage; $this->memoryUsageLabelName = $memoryUsageLabelName; + $this->shouldExplicitlyCollectCommandExceptions = $shouldExplicitlyCollectCommandExceptions; } public function setCustomLabels(array $customLabels): void @@ -82,4 +84,9 @@ public function getMemoryUsageLabelName(): string { return $this->memoryUsageLabelName; } + + public function shouldExplicitlyCollectCommandExceptions(): bool + { + return $this->shouldExplicitlyCollectCommandExceptions; + } } diff --git a/Listener/CommandListener.php b/Listener/CommandListener.php index 2cb7150..ccc4bc5 100644 --- a/Listener/CommandListener.php +++ b/Listener/CommandListener.php @@ -13,6 +13,7 @@ namespace ElasticApmBundle\Listener; +use ElasticApmBundle\Interactor\Config; use ElasticApmBundle\Interactor\ElasticApmInteractorInterface; use Symfony\Component\Console\ConsoleEvents; use Symfony\Component\Console\Event\ConsoleCommandEvent; @@ -22,10 +23,12 @@ class CommandListener implements EventSubscriberInterface { private $interactor; + private $config; - public function __construct(ElasticApmInteractorInterface $interactor) + public function __construct(ElasticApmInteractorInterface $interactor, Config $config) { $this->interactor = $interactor; + $this->config = $config; } public static function getSubscribedEvents(): array @@ -69,9 +72,15 @@ public function onConsoleCommand(ConsoleCommandEvent $event): void public function onConsoleError(ConsoleErrorEvent $event): void { + if (! $this->config->shouldExplicitlyCollectCommandExceptions()) { + return; + } + + $this->interactor->addContextFromConfig(); $this->interactor->noticeThrowable($event->getError()); if (null !== $event->getError()->getPrevious()) { + $this->interactor->addContextFromConfig(); $this->interactor->noticeThrowable($event->getError()->getPrevious()); } } diff --git a/README.md b/README.md index e77fd6b..3a2f4ce 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,7 @@ elastic_apm: transaction_naming_service: ~ # Transaction naming service (see below) commands: enabled: true # If true, enhances CLI commands with options and arguments (default: true) + explicitly_collect_exceptions: true # Turn this off if you are experiencing multiple reports of exceptions. ``` ## Enhanced RUM instrumentation @@ -130,6 +131,15 @@ monolog: id: 'Monolog\Handler\ElasticsearchHandler' ``` +## Troubleshooting + +### Exceptions from commands are being recorded multiple times + +PHP APM will automatically collect unhandled exceptions. The bundle will also install a listener for command exceptions. Our listener and the default behaviour can conflict which causes this behaviour. + +To fix this you can turn off `explicitly_collect_exceptions` under the `command` configuration node. + + ## Credits This bundle is based largely on the work done by https://github.com/ekino/EkinoNewRelicBundle.