From 92d8f138e9f1a7d2b05bb18ffb4e4343bb2fe99d Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Sat, 30 Dec 2017 22:39:26 -0500 Subject: [PATCH 01/13] Add wrapper function for newrelic_set_appname --- .../NewRelicReporting/Model/NewRelicWrapper.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php b/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php index 845ed0429d2c3..0d7bf630a5a84 100644 --- a/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php +++ b/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php @@ -41,6 +41,19 @@ public function reportError($exception) } } + /** + * Wrapper for 'newrelic_set_appname' + * + * @param string $appName + * @return void + */ + public function setAppName($appName) + { + if (extension_loaded('newrelic')) { + newrelic_set_appname($appName); + } + } + /** * Checks whether newrelic-php5 agent is installed * From e8965a82388dedd94c77ead0bd2789c7e39d25bd Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Sat, 30 Dec 2017 22:40:22 -0500 Subject: [PATCH 02/13] Add setting --- app/code/Magento/NewRelicReporting/etc/adminhtml/system.xml | 5 +++++ app/code/Magento/NewRelicReporting/i18n/en_US.csv | 2 ++ 2 files changed, 7 insertions(+) diff --git a/app/code/Magento/NewRelicReporting/etc/adminhtml/system.xml b/app/code/Magento/NewRelicReporting/etc/adminhtml/system.xml index 582b7c752386a..98f9c55adbdf0 100644 --- a/app/code/Magento/NewRelicReporting/etc/adminhtml/system.xml +++ b/app/code/Magento/NewRelicReporting/etc/adminhtml/system.xml @@ -46,6 +46,11 @@ This is located by navigating to Settings from the New Relic APM website + + + Magento\Config\Model\Config\Source\Yesno + In addition to the main app (which includes all PHP execution), separate apps for adminhtml and frontend will be created. Requires New Relic Application Name to be set. + diff --git a/app/code/Magento/NewRelicReporting/i18n/en_US.csv b/app/code/Magento/NewRelicReporting/i18n/en_US.csv index 433b1b22fcddd..5ea64d3d43439 100644 --- a/app/code/Magento/NewRelicReporting/i18n/en_US.csv +++ b/app/code/Magento/NewRelicReporting/i18n/en_US.csv @@ -21,3 +21,5 @@ General,General "This is located by navigating to Settings from the New Relic APM website","This is located by navigating to Settings from the New Relic APM website" Cron,Cron "Enable Cron","Enable Cron" +"Send Adminhtml and Frontend as Separate Apps","Send Adminhtml and Frontend as Separate Apps" +"In addition to the main app (which includes all PHP execution), separate apps for adminhtml and frontend will be created. Requires New Relic Application Name to be set.","In addition to the main app (which includes all PHP execution), separate apps for adminhtml and frontend will be created. Requires New Relic Application Name to be set." From 0d169ac0952d77459c011ccbb08ca1c5cc9800c9 Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Sat, 30 Dec 2017 22:40:44 -0500 Subject: [PATCH 03/13] Add method to consult setting --- app/code/Magento/NewRelicReporting/Model/Config.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/code/Magento/NewRelicReporting/Model/Config.php b/app/code/Magento/NewRelicReporting/Model/Config.php index 32e1078c01c9d..bcc87ec72d53f 100644 --- a/app/code/Magento/NewRelicReporting/Model/Config.php +++ b/app/code/Magento/NewRelicReporting/Model/Config.php @@ -161,6 +161,16 @@ public function getNewRelicAppName() return (string)$this->scopeConfig->getValue('newrelicreporting/general/app_name'); } + /** + * Returns configured separate apps value + * + * @return bool + */ + public function isSeparateApps() + { + return (bool)$this->scopeConfig->getValue('newrelicreporting/general/separate_apps'); + } + /** * Returns config setting for overall cron to be enabled * From 4d8c70224079604ce56b484c70f90552a85df450 Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Sat, 30 Dec 2017 22:41:26 -0500 Subject: [PATCH 04/13] Add mechanics for separate appnames --- .../NewRelicReporting/Plugin/StatePlugin.php | 83 +++++++++++++++++++ app/code/Magento/NewRelicReporting/etc/di.xml | 3 + 2 files changed, 86 insertions(+) create mode 100644 app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php new file mode 100644 index 0000000000000..149517bc7ce3f --- /dev/null +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -0,0 +1,83 @@ +config = $config; + $this->newRelicWrapper = $newRelicWrapper; + } + + /** + * Set separate appname + * + * @param State $subject + * @param null $result + * @return void + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function afterSetAreaCode(State $state, $result) + { + if (!$this->shouldSetAppName()) { + return; + } + + try { + $this->newRelicWrapper->setAppName($this->appName($state)); + } catch (LocalizedException $e) { + return; + } + } + + private function appName(State $state) + { + $code = $state->getAreaCode(); + $current = $this->config->getNewRelicAppName(); + + return $current . ';' . $current . '_' . $code; + } + + private function shouldSetAppName() + { + if (!$this->config->isNewRelicEnabled()) { + return false; + } + + if (!$this->config->getNewRelicAppName()) { + return false; + } + + if (!$this->config->isSeparateApps()) { + return false; + } + + return true; + } +} diff --git a/app/code/Magento/NewRelicReporting/etc/di.xml b/app/code/Magento/NewRelicReporting/etc/di.xml index 2dccc45c1129b..bab7d6611f14b 100644 --- a/app/code/Magento/NewRelicReporting/etc/di.xml +++ b/app/code/Magento/NewRelicReporting/etc/di.xml @@ -30,6 +30,9 @@ + + + From 630ea11e0f2a2ea1e838cd827363d4696022d533 Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Mon, 26 Feb 2018 21:22:06 -0500 Subject: [PATCH 05/13] Add type hint --- app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php b/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php index 0d7bf630a5a84..ec21e06976b8b 100644 --- a/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php +++ b/app/code/Magento/NewRelicReporting/Model/NewRelicWrapper.php @@ -47,7 +47,7 @@ public function reportError($exception) * @param string $appName * @return void */ - public function setAppName($appName) + public function setAppName(string $appName) { if (extension_loaded('newrelic')) { newrelic_set_appname($appName); From 9fa2765bb96f5fe737d809bf4d6b993a381135fa Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Mon, 26 Feb 2018 21:38:05 -0500 Subject: [PATCH 06/13] Log exceptions This would happen if for some reason the area code wasn't set --- .../Magento/NewRelicReporting/Plugin/StatePlugin.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php index 149517bc7ce3f..b3f3237256be6 100644 --- a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -9,6 +9,7 @@ use Magento\Framework\Exception\LocalizedException; use Magento\NewRelicReporting\Model\Config; use Magento\NewRelicReporting\Model\NewRelicWrapper; +use Psr\Log\LoggerInterface; class StatePlugin { @@ -22,16 +23,23 @@ class StatePlugin */ private $newRelicWrapper; + /** + * @var LoggerInterface + */ + private $logger; + /** * @param Config $config * @param NewRelicWrapper $newRelicWrapper */ public function __construct( Config $config, - NewRelicWrapper $newRelicWrapper + NewRelicWrapper $newRelicWrapper, + LoggerInterface $logger ) { $this->config = $config; $this->newRelicWrapper = $newRelicWrapper; + $this->logger = $logger; } /** @@ -52,6 +60,7 @@ public function afterSetAreaCode(State $state, $result) try { $this->newRelicWrapper->setAppName($this->appName($state)); } catch (LocalizedException $e) { + $this->logger->critical($e); return; } } From 5c607a4f83bd5799f95b886274e2493d012d7d96 Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Mon, 26 Feb 2018 21:40:49 -0500 Subject: [PATCH 07/13] Update returns --- app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php index b3f3237256be6..0be7c72689e7f 100644 --- a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -54,14 +54,14 @@ public function __construct( public function afterSetAreaCode(State $state, $result) { if (!$this->shouldSetAppName()) { - return; + return $result; } try { $this->newRelicWrapper->setAppName($this->appName($state)); } catch (LocalizedException $e) { $this->logger->critical($e); - return; + return $result; } } From 009082b3f229a92c95a883c7f06dcc3d1fe9ad6b Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Thu, 1 Mar 2018 22:39:38 -0500 Subject: [PATCH 08/13] Add a test --- .../Plugin/SeparateAppsTest.php | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php diff --git a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php new file mode 100644 index 0000000000000..3850a8cb3f9ae --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php @@ -0,0 +1,47 @@ +objectManager = Bootstrap::getObjectManager(); + } + + /** + * @magentoConfigFixture default/newrelicreporting/general/enable 1 + * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills + * @magentoConfigFixture default/newrelicreporting/general/separate_apps 1 + */ + public function testAppNameIsSetWhenConfiguredCorrectly() + { + $newRelicWrapper = $this->getMockBuilder(NewRelicWrapper::class) + ->setMethods(['setAppName']) + ->getMock(); + + $this->objectManager->configure([NewRelicWrapper::class => ['shared' => true]]); + $this->objectManager->addSharedInstance($newRelicWrapper, NewRelicWrapper::class); + + $newRelicWrapper->expects($this->once()) + ->method('setAppName') + ->with($this->equalTo('beverly_hills;beverly_hills_90210')); + + $state = $this->objectManager->get(State::class); + + $state->setAreaCode('90210'); + } +} From 061a5a102c100af60e59101b6b33ff1d679f542c Mon Sep 17 00:00:00 2001 From: Max Chadwick Date: Sun, 27 May 2018 20:28:39 -0400 Subject: [PATCH 09/13] Fix indentation --- .../Plugin/SeparateAppsTest.php | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php index 3850a8cb3f9ae..92b0ec0f6a732 100644 --- a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php +++ b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php @@ -12,36 +12,36 @@ class SeparateAppsTest extends \PHPUnit\Framework\TestCase { - /** - * @var ObjectManager - */ - private $objectManager; - - protected function setUp() - { - $this->objectManager = Bootstrap::getObjectManager(); - } - - /** - * @magentoConfigFixture default/newrelicreporting/general/enable 1 - * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills - * @magentoConfigFixture default/newrelicreporting/general/separate_apps 1 - */ - public function testAppNameIsSetWhenConfiguredCorrectly() - { - $newRelicWrapper = $this->getMockBuilder(NewRelicWrapper::class) - ->setMethods(['setAppName']) - ->getMock(); - - $this->objectManager->configure([NewRelicWrapper::class => ['shared' => true]]); - $this->objectManager->addSharedInstance($newRelicWrapper, NewRelicWrapper::class); - - $newRelicWrapper->expects($this->once()) - ->method('setAppName') - ->with($this->equalTo('beverly_hills;beverly_hills_90210')); - - $state = $this->objectManager->get(State::class); - - $state->setAreaCode('90210'); - } + /** + * @var ObjectManager + */ + private $objectManager; + + protected function setUp() + { + $this->objectManager = Bootstrap::getObjectManager(); + } + + /** + * @magentoConfigFixture default/newrelicreporting/general/enable 1 + * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills + * @magentoConfigFixture default/newrelicreporting/general/separate_apps 1 + */ + public function testAppNameIsSetWhenConfiguredCorrectly() + { + $newRelicWrapper = $this->getMockBuilder(NewRelicWrapper::class) + ->setMethods(['setAppName']) + ->getMock(); + + $this->objectManager->configure([NewRelicWrapper::class => ['shared' => true]]); + $this->objectManager->addSharedInstance($newRelicWrapper, NewRelicWrapper::class); + + $newRelicWrapper->expects($this->once()) + ->method('setAppName') + ->with($this->equalTo('beverly_hills;beverly_hills_90210')); + + $state = $this->objectManager->get(State::class); + + $state->setAreaCode('90210'); + } } From 05e2d82ec55cb97448c246f793dcf6461670b273 Mon Sep 17 00:00:00 2001 From: Ihor Sviziev Date: Mon, 11 Jun 2018 08:08:14 +0300 Subject: [PATCH 10/13] Add Ability To Separate Frontend / Adminhtml in New Relic Declare strict types --- app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php index 0be7c72689e7f..92d39d04e0dba 100644 --- a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -3,6 +3,8 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\NewRelicReporting\Plugin; use Magento\Framework\App\State; From 7fde1e786b0a69c73a99c8468f1a538a9006cf54 Mon Sep 17 00:00:00 2001 From: Ihor Sviziev Date: Mon, 11 Jun 2018 08:08:57 +0300 Subject: [PATCH 11/13] Add Ability To Separate Frontend / Adminhtml in New Relic Declare strict types --- .../Magento/NewRelicReporting/Plugin/SeparateAppsTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php index 92b0ec0f6a732..e14bcd4d11a4e 100644 --- a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php +++ b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php @@ -3,6 +3,8 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\NewRelicReporting\Plugin; use Magento\Framework\App\State; From ccc355a63f5c17222466834113fbde599af19290 Mon Sep 17 00:00:00 2001 From: Ievgen Shakhsuvarov Date: Fri, 14 Sep 2018 11:12:31 +0300 Subject: [PATCH 12/13] Add Ability To Separate Frontend / Adminhtml in New Relic - Minor improvements --- .../NewRelicReporting/Plugin/StatePlugin.php | 46 +++++++++++-------- .../Plugin/SeparateAppsTest.php | 27 +++++++++++ 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php index 92d39d04e0dba..e1ed6ef89e555 100644 --- a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -13,6 +13,9 @@ use Magento\NewRelicReporting\Model\NewRelicWrapper; use Psr\Log\LoggerInterface; +/** + * Handles setting which, when enabled, reports frontend and adminhtml as separate apps to New Relic. + */ class StatePlugin { /** @@ -33,6 +36,7 @@ class StatePlugin /** * @param Config $config * @param NewRelicWrapper $newRelicWrapper + * @param LoggerInterface $logger */ public function __construct( Config $config, @@ -49,25 +53,30 @@ public function __construct( * * @param State $subject * @param null $result - * @return void - * - * @SuppressWarnings(PHPMD.UnusedFormalParameter) + * @return mixed */ - public function afterSetAreaCode(State $state, $result) + public function afterSetAreaCode(State $subject, $result) { if (!$this->shouldSetAppName()) { return $result; } try { - $this->newRelicWrapper->setAppName($this->appName($state)); + $this->newRelicWrapper->setAppName($this->appName($subject)); } catch (LocalizedException $e) { $this->logger->critical($e); return $result; } + + return $result; } - private function appName(State $state) + /** + * @param State $state + * @return string + * @throws LocalizedException + */ + private function appName(State $state): string { $code = $state->getAreaCode(); $current = $this->config->getNewRelicAppName(); @@ -75,20 +84,17 @@ private function appName(State $state) return $current . ';' . $current . '_' . $code; } - private function shouldSetAppName() + /** + * Check if app name should be set. + * + * @return bool + */ + private function shouldSetAppName(): bool { - if (!$this->config->isNewRelicEnabled()) { - return false; - } - - if (!$this->config->getNewRelicAppName()) { - return false; - } - - if (!$this->config->isSeparateApps()) { - return false; - } - - return true; + return ( + $this->config->isSeparateApps() && + $this->config->getNewRelicAppName() && + $this->config->isNewRelicEnabled() + ); } } diff --git a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php index e14bcd4d11a4e..9271e08942279 100644 --- a/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php +++ b/dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php @@ -12,6 +12,9 @@ use Magento\TestFramework\ObjectManager; use Magento\TestFramework\Helper\Bootstrap; +/** + * Class SeparateAppsTest + */ class SeparateAppsTest extends \PHPUnit\Framework\TestCase { /** @@ -19,6 +22,9 @@ class SeparateAppsTest extends \PHPUnit\Framework\TestCase */ private $objectManager; + /** + * @inheritdoc + */ protected function setUp() { $this->objectManager = Bootstrap::getObjectManager(); @@ -46,4 +52,25 @@ public function testAppNameIsSetWhenConfiguredCorrectly() $state->setAreaCode('90210'); } + + /** + * @magentoConfigFixture default/newrelicreporting/general/enable 1 + * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills + * @magentoConfigFixture default/newrelicreporting/general/separate_apps 0 + */ + public function testAppNameIsNotSetWhenDisabled() + { + $newRelicWrapper = $this->getMockBuilder(NewRelicWrapper::class) + ->setMethods(['setAppName']) + ->getMock(); + + $this->objectManager->configure([NewRelicWrapper::class => ['shared' => true]]); + $this->objectManager->addSharedInstance($newRelicWrapper, NewRelicWrapper::class); + + $newRelicWrapper->expects($this->never())->method('setAppName'); + + $state = $this->objectManager->get(State::class); + + $state->setAreaCode('90210'); + } } From b9e20c7fc508d0272ca1916a8e89cfd74a038e94 Mon Sep 17 00:00:00 2001 From: Ievgen Shakhsuvarov Date: Fri, 14 Sep 2018 15:18:12 +0300 Subject: [PATCH 13/13] Add Ability To Separate Frontend / Adminhtml in New Relic - Minor improvements --- app/code/Magento/NewRelicReporting/Model/Config.php | 3 +++ app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/NewRelicReporting/Model/Config.php b/app/code/Magento/NewRelicReporting/Model/Config.php index bcc87ec72d53f..4bb381eb2f12d 100644 --- a/app/code/Magento/NewRelicReporting/Model/Config.php +++ b/app/code/Magento/NewRelicReporting/Model/Config.php @@ -5,6 +5,9 @@ */ namespace Magento\NewRelicReporting\Model; +/** + * NewRelic configuration model + */ class Config { /**#@+ diff --git a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php index e1ed6ef89e555..8be29fa6db9d9 100644 --- a/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php +++ b/app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php @@ -52,7 +52,7 @@ public function __construct( * Set separate appname * * @param State $subject - * @param null $result + * @param mixed $result * @return mixed */ public function afterSetAreaCode(State $subject, $result) @@ -72,6 +72,8 @@ public function afterSetAreaCode(State $subject, $result) } /** + * Format appName. + * * @param State $state * @return string * @throws LocalizedException