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

fix(filter): Added is_enabled field to transactions name filter #2251

Merged
merged 6 commits into from Jun 26, 2023

Conversation

RaduW
Copy link
Contributor

@RaduW RaduW commented Jun 26, 2023

This PR adds the is_enabled field to the TransactionsName configuration.

The flag is added in order to make it easier to work with it in Sentry (since all inbound filters have it).

@RaduW RaduW requested a review from a team June 26, 2023 11:29
relay-filter/src/transaction_name.rs Outdated Show resolved Hide resolved
@@ -141,12 +141,14 @@ pub struct ErrorMessagesFilterConfig {
pub struct IgnoreTransactionsFilterConfig {
/// List of patterns for ignored transactions that should be filtered.
pub patterns: GlobPatterns,
/// True if the filter is enabled
pub is_enabled: bool,
Copy link
Member

Choose a reason for hiding this comment

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

There is a tiny chance that somebody updated their external Relay since this filter was introduced. These old Relays would ignore this flag until they update again. But given the short time frame and the fact that there was no calendar release in between, I think we can ignore this problem.

Copy link
Contributor

@olksdr olksdr left a comment

Choose a reason for hiding this comment

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

What @jjbayer said.
Otherwise lgtm

@RaduW RaduW enabled auto-merge (squash) June 26, 2023 13:04
@RaduW RaduW disabled auto-merge June 26, 2023 13:42
Copy link
Member

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

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

Thanks for getting to this!

@RaduW RaduW enabled auto-merge (squash) June 26, 2023 14:34
@RaduW RaduW merged commit f7e38b7 into master Jun 26, 2023
17 checks passed
@RaduW RaduW deleted the fix/inbound-filters/improve-transactions-filter branch June 26, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants