-
Notifications
You must be signed in to change notification settings - Fork 9.4k
[Forwardport] Add Ability To Separate Frontend / Adminhtml in New Relic #15947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
92d8f13
e8965a8
0d169ac
4d8c702
630ea11
9fa2765
5c607a4
009082b
061a5a1
05e2d82
7fde1e7
b8e2558
ccc355a
b9e20c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| <?php | ||
| /** | ||
| * 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; | ||
| use Magento\Framework\Exception\LocalizedException; | ||
| use Magento\NewRelicReporting\Model\Config; | ||
| 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 | ||
| { | ||
| /** | ||
| * @var Config | ||
| */ | ||
| private $config; | ||
|
|
||
| /** | ||
| * @var NewRelicWrapper | ||
| */ | ||
| private $newRelicWrapper; | ||
|
|
||
| /** | ||
| * @var LoggerInterface | ||
| */ | ||
| private $logger; | ||
|
|
||
| /** | ||
| * @param Config $config | ||
| * @param NewRelicWrapper $newRelicWrapper | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. will Update PR |
||
| * @param LoggerInterface $logger | ||
| */ | ||
| public function __construct( | ||
| Config $config, | ||
| NewRelicWrapper $newRelicWrapper, | ||
| LoggerInterface $logger | ||
| ) { | ||
| $this->config = $config; | ||
| $this->newRelicWrapper = $newRelicWrapper; | ||
| $this->logger = $logger; | ||
| } | ||
|
|
||
| /** | ||
| * Set separate appname | ||
| * | ||
| * @param State $subject | ||
| * @param mixed $result | ||
| * @return mixed | ||
| */ | ||
| public function afterSetAreaCode(State $subject, $result) | ||
| { | ||
| if (!$this->shouldSetAppName()) { | ||
| return $result; | ||
| } | ||
|
|
||
| try { | ||
| $this->newRelicWrapper->setAppName($this->appName($subject)); | ||
| } catch (LocalizedException $e) { | ||
| $this->logger->critical($e); | ||
| return $result; | ||
| } | ||
|
|
||
| return $result; | ||
| } | ||
|
|
||
| /** | ||
| * Format appName. | ||
| * | ||
| * @param State $state | ||
| * @return string | ||
| * @throws LocalizedException | ||
| */ | ||
| private function appName(State $state): string | ||
| { | ||
| $code = $state->getAreaCode(); | ||
| $current = $this->config->getNewRelicAppName(); | ||
|
|
||
| return $current . ';' . $current . '_' . $code; | ||
| } | ||
|
|
||
| /** | ||
| * Check if app name should be set. | ||
| * | ||
| * @return bool | ||
| */ | ||
| private function shouldSetAppName(): bool | ||
| { | ||
| return ( | ||
| $this->config->isSeparateApps() && | ||
| $this->config->getNewRelicAppName() && | ||
| $this->config->isNewRelicEnabled() | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| <?php | ||
| /** | ||
| * 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; | ||
| use Magento\NewRelicReporting\Model\NewRelicWrapper; | ||
| use Magento\TestFramework\ObjectManager; | ||
| use Magento\TestFramework\Helper\Bootstrap; | ||
|
|
||
| /** | ||
| * Class SeparateAppsTest | ||
| */ | ||
| class SeparateAppsTest extends \PHPUnit\Framework\TestCase | ||
| { | ||
| /** | ||
| * @var ObjectManager | ||
| */ | ||
| private $objectManager; | ||
|
|
||
| /** | ||
| * @inheritdoc | ||
| */ | ||
| 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() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you don't check case if adjusting is disabled and exception |
||
| { | ||
| $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'); | ||
| } | ||
|
|
||
| /** | ||
| * @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'); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you use strict type, would be great to use return types as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍