Performance tracking #106
Performance tracking #106
Conversation
|
Tweaked it a bit, it now also adds the SQL queries from the Profliler |
|
@barryvdh did you do any testing on a production system of this? |
|
Not that much, database logging didnt really work this way but I was trying a different way before I was distracted by New Relic. It did seem to log the requests itself though, just not much additional data. |
|
@barryvdh I have to say I'm more interested in the CWV tracking (https://docs.sentry.io/product/performance/web-vitals/) than in database logging / PHP request logging. We use Blackfire for that mainly. |
|
This is really exciting, can we get that merged? |
| /** @var \Magento\Framework\App\ResourceConnection */ | ||
| private $resourceConnection; | ||
|
|
||
| public function __construct(\Magento\Framework\App\ResourceConnection $resourceConnection) |
There was a problem hiding this comment.
The $resourceConnection is not used in this file, can it be removed without issues?
|
|
||
| public function execute(\Magento\Framework\Event\Observer $observer) | ||
| { | ||
| $this->sentryPerformance->startTransaction($this->request); |
There was a problem hiding this comment.
Might it be a good idea to add the check wether we're doing performance tracking in these places as well? To ensure no code related to performance tracking is executed when it's disabled
|
Hey @barryvdh / @indykoning , what is still missing from this PR? I would be down to do the last mile to get this PR across the finish line. I would love to have performance tracking integrated :) |
|
For me the main thing missing is that check wether performance tracking is enabled and preventing any performance tracking code from being executed if it is enabled. So just the review left above, #106 (comment) is not the only place this check should be though. |
|
@peterjaap , did you find a way to monitor Core Web Vitals with the module? |
|
Is there any status here? Is it possible to get this merged into master? We're in need of profiling on Magento projects. Would be highly appreciated! |
|
@Seroliser have you tested this PR? |
|
To be honest, I'm not sure if this was ready or not. I think it was somewhat working, but only on dev setups. So not sure how valuable that is. I would have to test some more. I agree that Web Vitals could be more useful, but I was hoping to catch some bad behaviour with production data (eg. queries in loops etc). |
|
After we included the current setup, we were actually able to see results for Core Web Vitals without further configuration. |
|
Also been using this for a few months now and it works great! |
|
I see this PR is still in draft. Did this feature ever get merged into master? I want to start using the package but I need performance monitoring. |
|
@norgeindian as for CoreWebVitals, this might also be interesting for you :-) #137 |
|
@amenk , thanks for the info. Looks good. But why do we already see results in the CoreWebVitals section in Sentry, if that feature was turned off all the time? |
|
@norgeindian my PR is about the new INP metric (and the JS library update) |
|
@amenk , ah sorry, now I get it. Awesome, really cool. Thanks for that. |
This adds initial performance tracking by sending (server side) transactions. Could be used to add long-running queries or other events perhaps. Needs to set a sample rate + enable it in the backend. See #60
(Not very much experience with this, but it does seem to record transactions).