stayallive / wp-sentry

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

New feature / filter - Filter events sent to Sentry #179

Closed herewithme closed 9 months ago

herewithme commented 9 months ago

Sentry allows you to filter events in its SDK: https://docs.sentry.io/platforms/php/configuration/filtering/

This would be very useful, because sometimes warnings in the project come from third-party plugins and it's not always possible to patch them.

With the addition of a filter, this gives all the possibilities for advanced users.

herewithme commented 9 months ago

I forgot to thank you for your work on the plugin :)

stayallive commented 9 months ago

You can already do this: https://github.com/stayallive/wp-sentry?tab=readme-ov-file#wp_sentry_options (you can set any option from this hook).

Thanks for taking the time to make a PR but I'd rather not add more options if we can help it 😅

herewithme commented 9 months ago

Hello,

Actually, I find the wp_sentry_options hook quite useless because it executes too late. The vast majority of the code executes before, all the warnings linked to the inclusion of the source code, and all the code that initializes itself, especially in the "plugins_loaded" hook.

I find that there is a certain contradiction in the fact of proposing various means of loading your plugin as soon as possible (config, mu-plugins), and the impossibility of adjusting the Sentry SDK configuration.

With this filter, it's possible to intercept events/errors at an early stage.

Can you reconsider? :)

herewithme commented 9 months ago

ping @stayallive (the PR stay closed)

stayallive commented 9 months ago

Hmm. You make a fair point... I do want to explore another option though... how about:

define( 'WP_SENTRY_PHP_OPTIONS', [
    'before_send' => function (\Sentry\Event $event): ?\Sentry\Event {
        return $event;
    },
    // any other options from: https://docs.sentry.io/platforms/php/configuration/options/
] );

This would cleanup a lot of other constant too (I'll keep them to not break other people's apps ofcourse) but could make the configuration a lot more simple without having a constant for every option possible 😅 I believe this was not possible when I created the plugin because of older PHP version but this seems perfectly legal in PHP 7+ which we only support.

What do you think?

herewithme commented 9 months ago

Yes, it's a good approach, it's more generic, and there's no impact on performance if the constant isn't defined.

The drawback is that in wp-config.php, you can't use apply_filters.

For my purposes, I could do without it, but I find it impractical. This could be a first improvement :)

stayallive commented 9 months ago

I'm not sure why but I seem to had the impression you were adding a new constant config option... I still think WP_SENTRY_PHP_OPTIONS is a good option but that doesn't make you suggestion an invalid one either. So let's merge it!

I can't push to your branch because it was made inside a org. So I've opened #180 with your original commit and my changes on top of that.