Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 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 health-check endpoint
HealthCheck,
}

// 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::HealthCheck => "health-check",
}
}
}
Expand All @@ -176,6 +180,7 @@ impl<'a> TryFrom<&'a str> for FilterStatKey {
"localhost" => FilterStatKey::Localhost,
"web-crawlers" => FilterStatKey::WebCrawlers,
"invalid-csp" => FilterStatKey::InvalidCsp,
"health-check" => FilterStatKey::HealthCheck,
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 the health check filter.
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
pub struct HealthCheckEndpointsFilterConfig {
/// List of healthcheck patterns that will be filtered.
pub patterns: GlobPatterns,
}

impl HealthCheckEndpointsFilterConfig {
/// 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 the healthcheck filter.
#[serde(
default,
skip_serializing_if = "HealthCheckEndpointsFilterConfig::is_empty"
)]
pub health_check: HealthCheckEndpointsFilterConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

This filter works for any pattern, not just health checks. Could we rename it to reflect it (e.g. transaction_names)? It's ok if we just use it for health checks, but once we introduce health_check we should keep it for backward compatibility.

}

impl FiltersConfig {
Expand All @@ -223,6 +244,7 @@ impl FiltersConfig {
&& self.legacy_browsers.is_empty()
&& self.localhost.is_empty()
&& self.releases.is_empty()
&& self.health_check.is_empty()
}
}

Expand Down Expand Up @@ -260,6 +282,9 @@ mod tests {
releases: ReleasesFilterConfig {
releases: [],
},
health_check: HealthCheckEndpointsFilterConfig {
patterns: [],
},
}
"###);
Ok(())
Expand Down Expand Up @@ -293,6 +318,9 @@ mod tests {
releases: ReleasesFilterConfig {
releases: GlobPatterns::new(vec!["1.2.3".to_string()]),
},
health_check: HealthCheckEndpointsFilterConfig {
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"
]
},
"healthCheck": {
"patterns": [
"*health*"
]
}
}
"###);
Expand Down
155 changes: 155 additions & 0 deletions relay-filter/src/health_check.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, HealthCheckEndpointsFilterConfig};

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: &HealthCheckEndpointsFilterConfig,
) -> Result<(), FilterStatKey> {
if matches(event, &config.patterns) {
return Err(FilterStatKey::HealthCheck);
}
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 = HealthCheckEndpointsFilterConfig {
patterns: _get_patterns(),
};

let filter_result = should_filter(&event, &config);
assert_eq!(
filter_result,
Err(FilterStatKey::HealthCheck),
"Event did not filter health check event"
)
}

#[test]
fn test_does_not_filter_when_disabled() {
let event = Event {
transaction: Annotated::new("/health".into()),
..Event::default()
};
let filter_result = should_filter(
&event,
&HealthCheckEndpointsFilterConfig {
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,
&HealthCheckEndpointsFilterConfig {
patterns: _get_patterns(),
},
);
assert_eq!(
filter_result,
Ok(()),
"Event filtered although filter should have not matched"
)
}
}
2 changes: 2 additions & 0 deletions relay-filter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub mod browser_extensions;
pub mod client_ips;
pub mod csp;
pub mod error_messages;
pub mod health_check;
pub mod legacy_browsers;
pub mod localhost;
pub mod web_crawlers;
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)?;
health_check::should_filter(event, &config.health_check)?;

Ok(())
}