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: support IgnoreTransactions client option #717

Merged
merged 3 commits into from Sep 18, 2023

Conversation

greywolve
Copy link
Contributor

@greywolve greywolve commented Sep 14, 2023

Adds support for ignoring transactions by using the IgnoreTransactions client option. This requires a slice of regex strings that are matched against a transaction's name. If there's a match the transaction is dropped.

Fixes #628

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
client.go 100.00%
integrations.go 100.00%

📢 Thoughts on this report? Let us know!.

Copy link
Member

@tonyo tonyo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 👍

Could you just add a bit of PR description and link to the issue it's fixing/implementing?

Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

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

We want to make sure this continues to work with the ignoreErrorsIntegration that is currently controlled via the IgnoreErrors option.

I suggest adding an ignoreTransactionsIntegration, as we can't remove the ignoreErrorsIntegration, which would cause issues for people using it today.

@greywolve
Copy link
Contributor Author

We want to make sure this continues to work with the ignoreErrorsIntegration that is currently controlled via the IgnoreErrors option.

I suggest adding an ignoreTransactionsIntegration, as we can't remove the ignoreErrorsIntegration, which would cause issues for people using it today.

Hmm, my bad. I missed that IgnoreErrors was already a working integration. I'll rework this to just add an IgnoreTransaction integration, and not do any of the logic in client.go.

Define another integration rather for IgnoreTransactions. IgnoreErrors is already implemented.
@greywolve greywolve changed the title feat: support IgnoreErrors and IgnoreTransactions client options feat: support IgnoreTransactions client options Sep 15, 2023
@greywolve greywolve changed the title feat: support IgnoreTransactions client options feat: support IgnoreTransactions client option Sep 15, 2023
@greywolve
Copy link
Contributor Author

@cleptric rework done. :)

@cleptric cleptric self-requested a review September 18, 2023 10:51
@cleptric cleptric merged commit 8bf9a15 into getsentry:master Sep 18, 2023
17 checks passed
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.

Add ignoreTransactions and ignoreErrors
3 participants