sentry-php icon indicating copy to clipboard operation
sentry-php copied to clipboard

Easier way to add Sentry to libraries and plugins

Open davidbarratt opened this issue 6 months ago • 2 comments

Problem Statement

We want to be aware of when our customers run into problems with our plugin. The most straightforward way to do that is to add Sentry to our plugin: https://github.com/JoinFlexHealth/woocommerce/blob/a0a66e8ebad2f8368a7f23ea7cd5a79985a104a3/pay-with-flex.php#L52-L222

However, this is a lot of code and it only actually works for exceptions/errors that are handled. We could register the global Sentry client, but we didn't want to takeover an existing client in the code base. Nor do we want to receive events for things that are outside of our control.

Solution Brainstorm

I had two different ideas here:

  1. It would be helpful if the Sentry global could accept a "stack" of clients. It looks like this is almost supported because Hub supports an array of Frame instances, each can have their own client. However, upon further inspection it doesn't look like that would actually work without adding/removing our client from the stack with each function call. I wish the Hub would just loop through all the clients in the stack and report the error to each unique client / DSN. That would also allow our client to be able to filter through all errors.
  2. If you don't want to do that, another thing that would have been super helpful is allowing the default integrations to accept a custom hub/scope rather than the global one. I basically had to copy / re-implement a lot of the default integration code because the only way to use them is with the global instance.

davidbarratt avatar May 22 '25 23:05 davidbarratt

The general consensus is that there should never be two instances of the same SDK running in the same application. While on PHP it looks much simpler, on other platforms we can't guarantee any clear separation of telemetry from your library and the application it runs in.

I might be ok looking into your second point, but in general this is very low priority for us.

cleptric avatar May 27 '25 11:05 cleptric

That makes sense, I basically wrote some code to filter out anything that wasn't in the stack trace: https://github.com/JoinFlexHealth/woocommerce/blob/bd5f72a146f6c0bf25209142ff5dd479889e634a/pay-with-flex.php#L72-L105

I imagine that would be near impossible for integrations that don't provide a stack trace though, and as you can see I'm not sure what do with events that don't have one, in my case, since the default_integraitons are disabled I'm just going report it, but if that didn't work I would create a custom Event class and basically only report things if one of these conditions is met:

  1. The Event is instanceof the custom event type.
  2. The stacktrace has an isInApp that returns true.

I guess I'm mostly asking to keep all this in mind for the next major release. The least effort / most helpful thing for me would be to not assume the global instance everywhere.

davidbarratt avatar May 27 '25 15:05 davidbarratt