Closed stayallive closed 1 month ago
this PR will support https://docs.sentry.io/product/profiling/ ?
Indirectly yes. This implements: https://docs.sentry.io/product/performance/
Which is a prerequisite for profiling.
Could we break this PR to don't delay it that much?, if we break it with implementing the base of performance, I could create a PR to add profiling.
To avoid this PR as a blocker since I feel is quite big
Maybe I wasn't too clear. With performance implemented we get access to profiling, no extra work. There is nothing specific for the SDK to do except having a transaction (which this PR implements) from which point profiling can be enabled with a single configuration option so no additional PRs needed. Hence why I said this PR indirectly adds support for profiling 🤘
Alright. That point is clear.
I was wondering if you could reduce the scope of this issue to allow the basics of performance being enabled to allow use profiling, before the rest of the others features.
We could. But I feel like the scope is small enough. I want performance to be somewhat usable without profiling (since that requires a php extension to be installed so not an option for many WordPress users) and what is listed in the description is a very small subset of what could be done eventually.
How could I help to speed up this PR? If there is any chance of it
This PR is technically already working as is. So if you are able and have a test environment it would be great if you can give it a whirl to see if there are any bugs that I missed.
Giving this a try @stayallive
@stayallive, excellent work. While I'm still familiarizing myself with the nuances of Sentry's "Performance" feature, I can confirm that the code is tracking activity in the "Performance" tab. I appreciate your efforts in implementing this feature.
However, I did encounter a few fatal errors involving the code here and similar code:
$this->record_transient_operation('put', func_get_args()[2]);
Replacing it with:
$args = func_get_args()[2];
$this->record_transient_operation('put', $args);
resolved my issues. I didn't notice any other problems in the code.
@saas786 sorry for my late reaction. I don't see how you change would fix anything... do you have the exact fatal error message by chance so I can try to reproduce?
Thanks all for helping testing. I am going to release this as it currently stands and iterate on it from there based on feedback. Hope everyone tries it out and please open up issues with feature requests and or to report bugs 🎉
@stayallive
Here is the log of the issue I am referring to:
[03-Jul-2024 16:21:14 UTC] PHP Fatal error: Cannot use result of built-in function in write context in \wp-content\plugins\wp-sentry-integration\src\tracing\features\class-wp-sentry-tracing-feature-transients.php on line 45
That interesting, thanks @saas786. I've released https://github.com/stayallive/wp-sentry/releases/tag/v7.17.2 which should fix that 😄
This is a rough draft of performance tracing, things I like to implement:
shutdown
usage)Code location(defer until next)Code location(defer until next)SAVE_QUERIES
and multi-site URL)trace
helper for custom spans