stayallive / wp-sentry

A (unofficial) WordPress plugin reporting PHP and JavaScript errors to Sentry.
https://wordpress.org/plugins/wp-sentry-integration/
MIT License
305 stars 48 forks source link

Allow configuration of `\Sentry\Options` before initialization #99

Closed giilby closed 2 years ago

giilby commented 2 years ago

Per 94460cb, a change was made a few years ago that modified the behavior of wp_sentry_options to be called after Sentry client initialization instead of before. I'm assuming there's a reason for the change, but it's not clear to me at the moment.

I bring this up because I'd like to modify the default integrations that are set up when the client is initialized in order to avoid this issue. At first, I thought I could do this with the wp_sentry_options filter, but quickly learned that it's not currently possible.

My issue could be remedied with a small change to sentry-php, but I'm wondering: would it also be useful to provide a new filter that allows modification of the default options passed to ClientBuilder::create()? Happy to submit a PR for this!

stayallive commented 2 years ago

The problem here is that I doubt a filter is very effective since you want to initialise the client way before your own code is even executed.

I did think of a workaround but it felt a little hack-ish, but let me know what you think.


In your wp-config.php:

function wp_sentry_clientbuilder_cb( ClientBuilder $builder ): void {
    // Do anything you want with the ClientBuilder instance.
}
define( 'WP_SENTRY_CLIENTBUILDER_CALLBACK', 'wp_sentry_clientbuilder_cb' );

I'm not very excited about it but I also cannot think of another solution where your code runs before Sentry initializes especially if you are using the must-use plugin wrapper.

giilby commented 2 years ago

@stayallive Yeah, I had something similar in mind. I considered waiting to set WP_SENTRY_PHP_DSN until after a wp_sentry_modify_default_options filter is registered, but I think the constant-based callback registration approach is better, even though both feel hacky. I think it's worth providing because the only proper way to configure integrations is before the client is initialized. Do you want to add it, or should I?

stayallive commented 2 years ago

Sorry for the radio silence on this one, what do you think of:

https://github.com/stayallive/wp-sentry#modifying-the-php-sdk-clientbuilder-or-options-before-initialization

stayallive commented 2 years ago

Actually think it should work and already released it :)

Thanks for bringing this up and brainstorming on it!