Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add ignore_exceptions & ignore_transactions #1503

Merged
merged 9 commits into from
Mar 23, 2023
Merged

Conversation

cleptric
Copy link
Member

@cleptric cleptric commented Mar 22, 2023

This adds two new options, namely ignore_exceptions and ignore_transactions.

ignore_exceptions

Make it dead simple to ignore an Exception, without the need to deal with any integrations.
I actually marked IgnoreErrorsIntegration as deprecated.

Additionally, we take $exception->getPrevious() into account, when applying the option.

\Sentry\init([
    'ignore_exceptions' => [BadThingsHappenedException::class],
]);

closes #1426

ignore_transactions

Same story, make it dead simple to ignore a transaction, without the need to fiddle around with the traces_sampler or any before_send_transaction stuff.

\Sentry\init([
    'ignore_transactions' => ['GET /health'],
]);

Got inspired after reading https://stevenwoodson.com/blog/conserving-sentry-transactions-by-ignoring-laravel-routes/.

@cleptric cleptric self-assigned this Mar 22, 2023
@cleptric cleptric changed the title feat: Add ignore_exceptions & ignore_transactions feat: Add ignore_exceptions & ignore_transactions Mar 22, 2023
src/Client.php Outdated Show resolved Hide resolved
}

foreach ($exceptions as $exception) {
if (\in_array($exception->getType(), $this->options->getIgnoreExceptions(), true)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing is that this works slight different from the now deprecated IgnoreErrorsIntegration because that uses is_a, not sure if intentional but it might be nice to say you want to ignore HttpException::class instead of [ClientHttpException::class, ServerHttpException::class, ...] on the other hand this is more "expected" and you can still make that logic work with before_send of course.

Just a little rambling, I have no hard feelings either way so fine with the current impl.

tests/ClientTest.php Outdated Show resolved Hide resolved
src/Options.php Outdated Show resolved Hide resolved
src/Options.php Outdated Show resolved Hide resolved
src/Client.php Outdated Show resolved Hide resolved
src/Client.php Outdated Show resolved Hide resolved
@ste93cry
Copy link
Collaborator

ste93cry commented Mar 22, 2023

While I understand the reasoning behind these changes, I feel that the client is getting too much responsibility imho. The solution involving an integration was taken from the JS and Go SDKs and allows us to decouple the client from all these features, which improves the maintenability of the project. I also remember that we already had a discussion in the past about whether to consider "previous" exceptions, and I think that a real improvement would be to make this configurable: in fact, I may want to ignore all the exceptions, but only if they are the topmost.

cleptric and others added 7 commits March 22, 2023 10:26
Co-authored-by: Alex Bouma <alex@bouma.me>
Co-authored-by: Alex Bouma <alex@bouma.me>
Co-authored-by: Stefano Arlandini <sarlandini@alice.it>
@cleptric
Copy link
Member Author

While I understand the reasoning behind these changes, I feel that the client is getting too much responsibility imho. The solution involving an integration was taken from the JS and Go SDKs and allows us to decouple the client from all these features, which improves the maintenability of the project. I also remember that we already had a discussion in the past about whether to consider "previous" exceptions, and I think that a real improvement would be to make this configurable: in fact, I may want to ignore all the exceptions, but only if they are the topmost.

We tend to move away from event processors, as they are very opaque and confusing for people to use. Once getsentry/rfcs#34 lands, we yet again will move more things into the Client, which is fine.

@marandaneto
Copy link

@cleptric we also have that for some SDKs as well.
https://docs.sentry.io/platforms/java/configuration/options/#ignored-exceptions-for-type
It's a very handy utility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve 'ignore_exceptions' configuration to allow to ignore even 'previous' exceptions
4 participants