-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Newrelic api change #32648
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
Newrelic api change #32648
Conversation
Hi @nuzil. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento run all tests |
@magento create issue |
@magento run Functional Tests B2B, Functional Tests EE, WebAPI Tests |
@magento run WebAPI Tests |
@@ -15,7 +15,7 @@ class Deployments | |||
/** | |||
* API URL for New Relic deployments | |||
*/ | |||
const API_URL = 'https://api.newrelic.com/deployments.xml'; | |||
const API_URL = 'https://api.newrelic.com/v2/applications/%s/deployments.json'; |
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.
Should we take value from config (app/code/Magento/NewRelicReporting/etc/config.xml) ?
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.
Hey @swnsma
We do,
$apiUrl = $this->config->getNewRelicApiUrl();
if (empty($apiUrl)) {
$this->logger->notice('New Relic API URL is blank, using fallback URL');
$apiUrl = self::API_URL;
}
I see that config.xml is exists in module and cannot be empty in this case, but also may happen that somebody already extended module and for some reason using constant. So I would leave it there
Hi @swnsma, thank you for the review. |
Hi @nuzil Flushed after I hit the command php bin/magento newrelic:create:deploy-marker "manju1_deply1" "System.log". |
Ok, Seems I have to do some missing configurations, will come back soon! thanks! |
Hi @nuzil |
✔️ QA Passed Preconditions:
Manual testing scenario:
Before: ✖️ Were not able to deploy successfully After: ✔️ Able to deploy successfully (below screenshot has the successfully deployed log too) Since this fix is relevant to the latest release version of new relic and its integration, it has no impact on any other functionalities. Hence no additional regression is required. |
Description (*)
New Relic has an possibility to setup deployment markers.
But this feature is not working anymore, cause NewRelic changed API some time ago and current implementation is useless.
I changed Modue to work with a new API.
More info:
https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/record-monitor-deployments/
Fixed Issues (if relevant)
Fixed Deployment functionality for NewRelic module
Manual testing scenarios (*)
Contribution checklist (*)
Resolved issues: