[Forwardport] Add Ability To Separate Frontend / Adminhtml in New Relic #15947

ihor-sviziev · 2018-06-08T03:37:21Z

Original Pull Request

Description

Adds a new setting which, when enabled, reports frontend and adminhtml as separate apps to New Relic.

frontend and adminhtml are in addition to the "main" app which includes both (for backwards compatibility). The user needs to set "New Relic Application Name" to use this feature as the area is appended to the appname setting, separated by and underscore.

Useful as slow transactions in admin activity can cause false positives New Relic alerts / skew average response time metrics. Using this feature allows monitoring and alerting on only frontend traffic by configuring alerts for just the frontend New Relic app.

Manual testing scenarios

Pull in changes to a testing environment with New Relic agent installed
Ensure "Enable New Relic Intergration" is set to yes
Ensure "New Relic Application Name" is set
Turn on "Send Adminhtml and Frontend as Separate Apps Setting"
Navigate the frontend and backend of Magento
Login to New Relic and observe 3 apps ("main" app, "adminhtml" app, and "frontend" app).

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

This would happen if for some reason the area code wasn't set

magento-cicd2 · 2018-06-08T03:37:23Z

All committers have signed the CLA.

mpchadwick · 2018-06-09T19:00:20Z

lol @ihor-sviziev was just about to do this and noticed you already took care of it. thanks! https://github.com/magento/magento2/compare/2.3-develop...mpchadwick:feature/2.3/new-relic-separate-areas?expand=1

Declare strict types

ihor-sviziev · 2018-06-21T14:47:11Z

@magento/open-source-maintainers please review my PR

julian-van-drielen · 2018-06-21T14:50:44Z

@ihor-sviziev Your code is not up to the Magento coding standards. Please resolve these issues before we can review your pull request.

See codacy for more information.

ihor-sviziev · 2018-06-21T14:53:44Z

@cream-julian In tests we can use ObjectManager, it's allowed. Original PR with the same changes was already merged

igortregub · 2018-06-21T19:35:13Z

app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php

+     *
+     * @param State $subject
+     * @param null $result
+     * @return void


your code returns some data, please update doc-block or fix code

Also wouldn't be wrong to auto-format this so that the variable names are aligned under each other. I'm not sure if this is required in the magento coding standards though...

@igortregub we can use "mixed", but actually it doesn't matter. Will update PR
@abbasalis it's not required in magento coding standards

imadphp · 2018-06-21T19:43:24Z

app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php

+
+    /**
+     * @param Config $config
+     * @param NewRelicWrapper $newRelicWrapper


Please add $logger param

Good point. will Update PR

imadphp · 2018-06-21T19:45:31Z

app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php

+            return false;
+        }
+
+        return true;


I'd simply say

return ( $this->config->isSeparateApps() && $this->config->getNewRelicAppName() && $this->config->isNewRelicEnabled() )

What do you think?

Will update it

imadphp · 2018-06-21T19:45:53Z

dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php

+     */
+    private $objectManager;
+
+    protected function setUp()


Add your docblocks here please

igortregub · 2018-06-21T19:46:59Z

app/code/Magento/NewRelicReporting/Plugin/StatePlugin.php

+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);


if you use strict type, would be great to use return types as well

igortregub · 2018-06-21T19:51:00Z

dev/tests/integration/testsuite/Magento/NewRelicReporting/Plugin/SeparateAppsTest.php

+     * @magentoConfigFixture default/newrelicreporting/general/app_name beverly_hills
+     * @magentoConfigFixture default/newrelicreporting/general/separate_apps 1
+     */
+    public function testAppNameIsSetWhenConfiguredCorrectly()


you don't check case if adjusting is disabled and exception

…to2 into 2.3-develop-PR-port-12935

- Minor improvements

ihor-sviziev · 2018-09-14T09:47:14Z

Hi @ishakhsuvarov,
Thank you for fixing these issues in my PR.
Also would be great to add these changes to 2.2.x release line. Should I create PR?

- Minor improvements

ishakhsuvarov · 2018-09-14T12:19:09Z

@ihor-sviziev Happy to help.

I think it would make sense to hold on with a 2.2 PR while this gets delivered

ihor-sviziev · 2018-09-14T13:16:18Z

@ishakhsuvarov sure!

magento-engcom-team · 2018-09-16T23:53:14Z

Hi @ihor-sviziev. Thank you for your contribution.
We will aim to release these changes as part of 2.3.0.
Please check the release notes for final confirmation.

- Merge Pull Request #18113 from ihor-sviziev/magento2:new-relic-15947-fixes - Merged commits: 1. be7e142 2. 2a709c1 3. 63c51f4 4. 3eec546

@dmytro-ch

Accepted Public Pull Requests: - #18126: [Backport] [2.2] Changed intval($val) to (int) $val, since it is faster: (by @dmytro-ch) - #18000: Fix sitemap grid render incorrect base urls for multiple stores (by @nntoan) - #18098: Fix shipping discount failed to apply during place order (by @torreytsui) - #18113: [Backport] Fixes from #15947 (by @ihor-sviziev) - #18097: [Backport] Fix import grouped products #12853 (by @insanityinside) - #17993: fix #17582 ./bin/magento config:show fails with a fatal error (by @keyurshah070) - #17679: Update shipment collection to unserialize `packages` attribute after load (by @dnsv) - #18055: fix: reset search mini-form when we have no data / an empty response (by @DanielRuf) - #14065: Correctly convert config integration api resources (by @therool) Fixed GitHub Issues: - #17999: Sitemap grid display incorrect base URL in the grid if using multiple stores (reported by @nntoan) has been fixed in #18000 by @nntoan in 2.2-develop branch Related commits: 1. 1172aae - #17582: ./bin/magento config:show fails with a fatal error (reported by @simonworkhouse) has been fixed in #17993 by @keyurshah070 in 2.2-develop branch Related commits: 1. ae503f6 - #12095: Update 2.2.1: One or more integrations have been reset because of a change to their xml configs. (reported by @thomvanderboon) has been fixed in #14065 by @therool in 2.2-develop branch Related commits: 1. 89a0a09 2. 5815a20 3. 406935b 4. 3868e81 5. 42eab52

mpchadwick added 9 commits June 8, 2018 03:37

Add wrapper function for newrelic_set_appname

92d8f13

Add setting

e8965a8

Add method to consult setting

0d169ac

Add mechanics for separate appnames

4d8c702

Add type hint

630ea11

Log exceptions

9fa2765

This would happen if for some reason the area code wasn't set

Update returns

5c607a4

Add a test

009082b

Fix indentation

061a5a1

magento-engcom-team added the Component: NewRelicReporting label Jun 8, 2018

ihor-sviziev added 2 commits June 11, 2018 08:08

Add Ability To Separate Frontend / Adminhtml in New Relic

05e2d82

Declare strict types

Add Ability To Separate Frontend / Adminhtml in New Relic

7fde1e7

Declare strict types

ihor-sviziev added Release Line: 2.3 forwardport labels Jun 15, 2018

igortregub reviewed Jun 21, 2018

View reviewed changes

imadphp reviewed Jun 21, 2018

View reviewed changes

igortregub reviewed Jun 21, 2018

View reviewed changes

ihor-sviziev added the Progress: needs update label Jun 22, 2018

ihor-sviziev mentioned this pull request Jul 25, 2018

Add Ability To Separate Frontend / Adminhtml in New Relic #12935

Merged

4 tasks

ihor-sviziev closed this Jul 25, 2018

ishakhsuvarov reopened this Sep 14, 2018

magento#15947: Merge branch '2.3-develop' of github.com:magento/magen…

b8e2558

…to2 into 2.3-develop-PR-port-12935

Add Ability To Separate Frontend / Adminhtml in New Relic

ccc355a

- Minor improvements

magento-engcom-team assigned ishakhsuvarov Sep 14, 2018

magento-engcom-team added this to the Release: 2.3.0 milestone Sep 14, 2018

magento-engcom-team removed the Progress: needs update label Sep 14, 2018

Add Ability To Separate Frontend / Adminhtml in New Relic

b9e20c7

- Minor improvements

magento-engcom-team merged commit b9e20c7 into magento:2.3-develop Sep 16, 2018

This was referenced Sep 18, 2018

[Backport] Fixes from magento/magento2#15947 #18112

Closed

[Backport] Fixes from #15947 #18113

Merged

magento-engcom-team added a commit that referenced this pull request Sep 24, 2018

ENGCOM-3005: [Backport] Fixes from #15947 #18113

88ddcaa

- Merge Pull Request #18113 from ihor-sviziev/magento2:new-relic-15947-fixes - Merged commits: 1. be7e142 2. 2a709c1 3. 63c51f4 4. 3eec546

magento-engcom-team pushed a commit that referenced this pull request Sep 24, 2018

ENGCOM-3005: [Backport] Fixes from #15947 #18113

2dbcaef

[Forwardport] Add Ability To Separate Frontend / Adminhtml in New Relic #15947

[Forwardport] Add Ability To Separate Frontend / Adminhtml in New Relic #15947

Uh oh!

Conversation

ihor-sviziev commented Jun 8, 2018 • edited by sidolov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Original Pull Request

Description

Manual testing scenarios

Contribution checklist

Uh oh!

magento-cicd2 commented Jun 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpchadwick commented Jun 9, 2018

Uh oh!

ihor-sviziev commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julian-van-drielen commented Jun 21, 2018

Uh oh!

ihor-sviziev commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igortregub Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imadphp Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ihor-sviziev commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ishakhsuvarov commented Sep 14, 2018

Uh oh!

ihor-sviziev commented Sep 14, 2018

Uh oh!

magento-engcom-team commented Sep 16, 2018

Uh oh!

Uh oh!

ihor-sviziev commented Jun 8, 2018 •

edited by sidolov

Loading

magento-cicd2 commented Jun 8, 2018 •

edited

Loading

ihor-sviziev commented Jun 21, 2018 •

edited

Loading

ihor-sviziev commented Jun 21, 2018 •

edited

Loading

igortregub Jun 21, 2018 •

edited

Loading

imadphp Jun 21, 2018 •

edited

Loading

ihor-sviziev commented Sep 14, 2018 •

edited

Loading