diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b2627904f1..a3e3266579a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Features**: +- Add filter based on transaction names. ([#2118](https://github.com/getsentry/relay/pull/2118)) - Drop profiles without a transaction in the same envelope. ([#2169](https://github.com/getsentry/relay/pull/2169)) - Use GeoIP lookup also in non-processing Relays. Lookup from now on will be also run in light normalization. ([#2229](https://github.com/getsentry/relay/pull/2229)) diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md index 867749789a5..610ea38dbdf 100644 --- a/py/CHANGELOG.md +++ b/py/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Add filter based on transaction names. ([#2118](https://github.com/getsentry/relay/pull/2118)) - Add `lock` attribute to the frame protocol. ([#2171](https://github.com/getsentry/relay/pull/2171)) ## 0.8.25 diff --git a/relay-filter/src/common.rs b/relay-filter/src/common.rs index abfd92bb3b5..093cf142489 100644 --- a/relay-filter/src/common.rs +++ b/relay-filter/src/common.rs @@ -127,6 +127,9 @@ pub enum FilterStatKey { /// Filtered due to invalid CSP policy. InvalidCsp, + + /// Filtered due to the fact that it was a call to a filtered transaction + FilteredTransactions, } // An event grouped to a removed group. @@ -153,6 +156,7 @@ impl FilterStatKey { FilterStatKey::Localhost => "localhost", FilterStatKey::WebCrawlers => "web-crawlers", FilterStatKey::InvalidCsp => "invalid-csp", + FilterStatKey::FilteredTransactions => "filtered-transaction", } } } @@ -176,6 +180,7 @@ impl<'a> TryFrom<&'a str> for FilterStatKey { "localhost" => FilterStatKey::Localhost, "web-crawlers" => FilterStatKey::WebCrawlers, "invalid-csp" => FilterStatKey::InvalidCsp, + "filtered-transaction" => FilterStatKey::FilteredTransactions, other => { return Err(other); } diff --git a/relay-filter/src/config.rs b/relay-filter/src/config.rs index a38b08fcdab..f83b59cffa9 100644 --- a/relay-filter/src/config.rs +++ b/relay-filter/src/config.rs @@ -136,6 +136,20 @@ pub struct ErrorMessagesFilterConfig { pub patterns: GlobPatterns, } +/// Configuration for transaction name filter. +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +pub struct IgnoreTransactionsFilterConfig { + /// List of patterns for ignored transactions that should be filtered. + pub patterns: GlobPatterns, +} + +impl IgnoreTransactionsFilterConfig { + /// Returns true if no configuration for this filter is given. + pub fn is_empty(&self) -> bool { + self.patterns.is_empty() + } +} + impl ErrorMessagesFilterConfig { /// Returns true if no configuration for this filter is given. pub fn is_empty(&self) -> bool { @@ -210,6 +224,13 @@ pub struct FiltersConfig { /// Configuration for the releases filter. #[serde(default, skip_serializing_if = "ReleasesFilterConfig::is_empty")] pub releases: ReleasesFilterConfig, + + /// Configuration for ignore transactions filter. + #[serde( + default, + skip_serializing_if = "IgnoreTransactionsFilterConfig::is_empty" + )] + pub ignore_transactions: IgnoreTransactionsFilterConfig, } impl FiltersConfig { @@ -223,6 +244,7 @@ impl FiltersConfig { && self.legacy_browsers.is_empty() && self.localhost.is_empty() && self.releases.is_empty() + && self.ignore_transactions.is_empty() } } @@ -260,6 +282,9 @@ mod tests { releases: ReleasesFilterConfig { releases: [], }, + ignore_transactions: IgnoreTransactionsFilterConfig { + patterns: [], + }, } "###); Ok(()) @@ -293,6 +318,9 @@ mod tests { releases: ReleasesFilterConfig { releases: GlobPatterns::new(vec!["1.2.3".to_string()]), }, + ignore_transactions: IgnoreTransactionsFilterConfig { + patterns: GlobPatterns::new(vec!["*health*".to_string()]), + }, }; insta::assert_json_snapshot!(filters_config, @r###" @@ -331,6 +359,11 @@ mod tests { "releases": [ "1.2.3" ] + }, + "ignoreTransactions": { + "patterns": [ + "*health*" + ] } } "###); diff --git a/relay-filter/src/lib.rs b/relay-filter/src/lib.rs index 8af78447959..2671cfff2f4 100644 --- a/relay-filter/src/lib.rs +++ b/relay-filter/src/lib.rs @@ -23,6 +23,7 @@ pub mod csp; pub mod error_messages; pub mod legacy_browsers; pub mod localhost; +pub mod transaction_name; pub mod web_crawlers; mod common; @@ -56,6 +57,7 @@ pub fn should_filter( browser_extensions::should_filter(event, &config.browser_extensions)?; legacy_browsers::should_filter(event, &config.legacy_browsers)?; web_crawlers::should_filter(event, &config.web_crawlers)?; + transaction_name::should_filter(event, &config.ignore_transactions)?; Ok(()) } diff --git a/relay-filter/src/transaction_name.rs b/relay-filter/src/transaction_name.rs new file mode 100644 index 00000000000..f8fd7783d93 --- /dev/null +++ b/relay-filter/src/transaction_name.rs @@ -0,0 +1,155 @@ +//! Implements event filtering based on whether the endpoint called is a healthcheck endpoint. +//! +//! If this filter is enabled transactions from healthcheck endpoints will be filtered out. + +use relay_general::protocol::Event; + +use crate::{FilterStatKey, GlobPatterns, IgnoreTransactionsFilterConfig}; + +fn matches(event: &Event, patterns: &GlobPatterns) -> bool { + event + .transaction + .value() + .map_or(false, |transaction| patterns.is_match(transaction)) +} + +/// Filters transaction events for calls to healthcheck endpoints +pub fn should_filter( + event: &Event, + config: &IgnoreTransactionsFilterConfig, +) -> Result<(), FilterStatKey> { + if matches(event, &config.patterns) { + return Err(FilterStatKey::FilteredTransactions); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use relay_general::protocol::Event; + use relay_general::types::Annotated; + + fn _get_patterns() -> GlobPatterns { + let patterns_raw = vec!["*healthcheck*".into(), "*/health".into()]; + GlobPatterns::new(patterns_raw) + } + + /// tests matching for various transactions + #[test] + fn test_matches() { + let patterns = _get_patterns(); + + let transaction_names = [ + "a/b/healthcheck/c", + "a_HEALTHCHECK_b", + "healthcheck", + "/health", + "a/HEALTH", + "/health", + "a/HEALTH", + ]; + + for name in transaction_names { + let event = Event { + transaction: Annotated::new(name.into()), + ..Event::default() + }; + assert!(matches(&event, &patterns), "Did not match `{name}`") + } + } + /// tests non matching transactions transactions + #[test] + fn test_does_not_match() { + let transaction_names = [ + "bad", + "/bad", + "a/b/c", + "health", + "healthz", + "/health/", + "/healthz/", + "/healthx", + "/healthzx", + ]; + let patterns = _get_patterns(); + + for name in transaction_names { + let event = Event { + transaction: Annotated::new(name.into()), + ..Event::default() + }; + assert!( + !matches(&event, &patterns), + "Did match `{name}` but it shouldn't have." + ) + } + } + + // test it doesn't match when the transaction name is missing + #[test] + fn test_does_not_match_missing_transaction() { + let event = Event { ..Event::default() }; + let patterns = _get_patterns(); + assert!( + !matches(&event, &patterns), + "Did match with empty transaction but it shouldn't have." + ) + } + + #[test] + fn test_filters_when_matching() { + let event = Event { + transaction: Annotated::new("/health".into()), + ..Event::default() + }; + let config = IgnoreTransactionsFilterConfig { + patterns: _get_patterns(), + }; + + let filter_result = should_filter(&event, &config); + assert_eq!( + filter_result, + Err(FilterStatKey::FilteredTransactions), + "Event was not filtered " + ) + } + + #[test] + fn test_does_not_filter_when_disabled() { + let event = Event { + transaction: Annotated::new("/health".into()), + ..Event::default() + }; + let filter_result = should_filter( + &event, + &IgnoreTransactionsFilterConfig { + patterns: GlobPatterns::new(vec![]), + }, + ); + assert_eq!( + filter_result, + Ok(()), + "Event filtered although filter should have been disabled" + ) + } + + #[test] + fn test_does_not_filter_when_not_matching() { + let event = Event { + transaction: Annotated::new("/a/b/c".into()), + ..Event::default() + }; + let filter_result = should_filter( + &event, + &IgnoreTransactionsFilterConfig { + patterns: _get_patterns(), + }, + ); + assert_eq!( + filter_result, + Ok(()), + "Event filtered although filter should have not matched" + ) + } +} diff --git a/tests/integration/fixtures/processing.py b/tests/integration/fixtures/processing.py index 3364a0adba0..8aceafe2207 100644 --- a/tests/integration/fixtures/processing.py +++ b/tests/integration/fixtures/processing.py @@ -262,7 +262,9 @@ def events_consumer(kafka_consumer): @pytest.fixture def transactions_consumer(kafka_consumer): - return lambda: EventsConsumer(*kafka_consumer("transactions")) + return lambda timeout=None: EventsConsumer( + timeout=timeout, *kafka_consumer("transactions") + ) @pytest.fixture diff --git a/tests/integration/test_filters.py b/tests/integration/test_filters.py index 6b7172274f4..f050276d53a 100644 --- a/tests/integration/test_filters.py +++ b/tests/integration/test_filters.py @@ -213,3 +213,66 @@ def test_web_crawlers_filter_are_applied( events_consumer.assert_empty() else: events_consumer.get_event() + + +@pytest.mark.parametrize( + "is_enabled, transaction_name, should_filter", + [ + (True, "health-check-1 ", True), + (False, "health-check-2", False), + (True, "some-transaction-3", False), + ], + ids=[ + "when enabled ignore transactions are filtered", + "when disabled ignore transactions are NOT filtered", + "when enabled leaves alone transactions that are not enabled", + ], +) +def test_ignore_transactions_filters_are_applied( + mini_sentry, + relay_with_processing, + transactions_consumer, + is_enabled, + transaction_name, + should_filter, +): + """ + Tests the ignore transactions filter + """ + relay = relay_with_processing() + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + filter_settings = project_config["config"]["filterSettings"] + if is_enabled: + filter_settings["ignoreTransactions"] = {"patterns": ["health*"]} + else: + filter_settings["ignoreTransactions"] = {"patterns": []} + + transactions_consumer = transactions_consumer(timeout=10) + + now = datetime.datetime.utcnow() + start_timestamp = now.timestamp() + timestamp = (now - datetime.timedelta(minutes=-1)).timestamp() + + transaction = { + "event_id": "d2132d31b39445f1938d7e21b6bf0ec4", + "type": "transaction", + "transaction": transaction_name, + "start_timestamp": start_timestamp, + "timestamp": timestamp, + "contexts": { + "trace": { + "trace_id": "1234F60C11214EB38604F4AE0781BFB2", + "span_id": "ABCDFDEAD5F74052", + "type": "trace", + } + }, + } + + relay.send_transaction(project_id, transaction) + + if should_filter: + transactions_consumer.assert_empty() + else: + event, _ = transactions_consumer.get_event() + assert event["transaction"] == transaction_name