Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

**Features**:

- Add filter for healthcheck endpoints. ([#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))

Expand Down
1 change: 1 addition & 0 deletions py/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Add filter for healthcheck endpoints. ([#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
Expand Down
5 changes: 5 additions & 0 deletions relay-filter/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -153,6 +156,7 @@ impl FilterStatKey {
FilterStatKey::Localhost => "localhost",
FilterStatKey::WebCrawlers => "web-crawlers",
FilterStatKey::InvalidCsp => "invalid-csp",
FilterStatKey::FilteredTransactions => "filtered-transaction",
}
}
}
Expand All @@ -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);
}
Expand Down
33 changes: 33 additions & 0 deletions relay-filter/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -223,6 +244,7 @@ impl FiltersConfig {
&& self.legacy_browsers.is_empty()
&& self.localhost.is_empty()
&& self.releases.is_empty()
&& self.ignore_transactions.is_empty()
}
}

Expand Down Expand Up @@ -260,6 +282,9 @@ mod tests {
releases: ReleasesFilterConfig {
releases: [],
},
ignore_transactions: IgnoreTransactionsFilterConfig {
patterns: [],
},
}
"###);
Ok(())
Expand Down Expand Up @@ -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###"
Expand Down Expand Up @@ -331,6 +359,11 @@ mod tests {
"releases": [
"1.2.3"
]
},
"ignoreTransactions": {
"patterns": [
"*health*"
]
}
}
"###);
Expand Down
2 changes: 2 additions & 0 deletions relay-filter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}
155 changes: 155 additions & 0 deletions relay-filter/src/transaction_name.rs
Original file line number Diff line number Diff line change
@@ -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"
)
}
}
4 changes: 3 additions & 1 deletion tests/integration/fixtures/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 63 additions & 0 deletions tests/integration/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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