-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
#22047: Magento CRON Job Names are missing in NewRelic: Transactions FIX #22061
#22047: Magento CRON Job Names are missing in NewRelic: Transactions FIX #22061
Conversation
Hi @PiotrSiejczuk. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
cf24497
to
d5970c2
Compare
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.
Hi @PiotrSiejczuk , thanks for the pull request!
Magento_Cron
module should not be aware of New Relic, can you please move the implementation to Magento_NewRelicReporting
module.
Also, please avoid adding static functions and ensure method/constant/properties visibility is set to private
where possible
@sivaschenko thank you for your feedback. I have discussed this topic together with @lbajsarowicz. The idea is that he is going to extend his changes within: PR: #22059 to incorporate mine by doing that within NewRelic_Reporting Module If for some reason that is not going to happen I will refactor my PR to improve logic and quality of the proposed changes |
Hi @PiotrSiejczuk , I'm closing this PR since #22059 was merged recently. |
Hi @PiotrSiejczuk, thank you for your contribution! |
Hi @sidolov, thank you for your information. Although commits from @lbajsarowicz are not addressing the whole scope of the reported issue. It is perfect that CLI CRON commands are being correctly Tracked @ NewRelic now. Although it is not tracking correctly ATOMIC Transactions executed within standard CRON execution. That being said: this ticket should not be CLOSED as the FIX is not addressing the whole scope of the ISSUE described initially |
Description (*)
Magento CRON Job Names are missing in NewRelic: Transactions Names - This Pull Request Fixes it as per Detailed Description below
Fixed Issues (if relevant)
Manual testing scenarios (*)
Contribution checklist (*)