Skip to content

Commit

Permalink
feat(inbound-filters): Implement generic inbound filters (#2595)
Browse files Browse the repository at this point in the history
  • Loading branch information
iambriccardo authored Oct 30, 2023
1 parent 2a86991 commit ee3036c
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
- Add a setting to rollout ingesting all resource spans. ([#2586](https://github.com/getsentry/relay/pull/2586))
- Drop events starting or ending before January 1, 1970 UTC. ([#2613](https://github.com/getsentry/relay/pull/2613))
- Add support for X-Sentry-Forwarded-For header. ([#2572](https://github.com/getsentry/relay/pull/2572))
- Add a generic way of configuring inbound filters via project configs. ([#2595](https://github.com/getsentry/relay/pull/2595))

**Bug Fixes**:

Expand Down
21 changes: 13 additions & 8 deletions relay-filter/src/common.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::convert::TryFrom;
use std::fmt;

Expand All @@ -7,7 +8,7 @@ use serde::Serialize;
///
/// Ported from Sentry's same-named "enum". The enum variants are fed into outcomes in kebap-case
/// (e.g. "browser-extensions")
#[derive(Debug, Copy, Clone, Eq, PartialEq, Serialize, Hash)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Hash)]
pub enum FilterStatKey {
/// Filtered by ip address.
IpAddress,
Expand Down Expand Up @@ -35,6 +36,9 @@ pub enum FilterStatKey {

/// Filtered due to the fact that it was a call to a filtered transaction
FilteredTransactions,

/// Filtered due to a generic filter.
GenericFilter(String),
}

// An event grouped to a removed group.
Expand All @@ -51,8 +55,8 @@ pub enum FilterStatKey {

impl FilterStatKey {
/// Returns the string identifier of the filter stat key.
pub fn name(self) -> &'static str {
match self {
pub fn name(self) -> Cow<'static, str> {
Cow::Borrowed(match self {
FilterStatKey::IpAddress => "ip-address",
FilterStatKey::ReleaseVersion => "release-version",
FilterStatKey::ErrorMessage => "error-message",
Expand All @@ -62,13 +66,16 @@ impl FilterStatKey {
FilterStatKey::WebCrawlers => "web-crawlers",
FilterStatKey::InvalidCsp => "invalid-csp",
FilterStatKey::FilteredTransactions => "filtered-transaction",
}
FilterStatKey::GenericFilter(filter_identifier) => {
return Cow::Owned(filter_identifier);
}
})
}
}

impl fmt::Display for FilterStatKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.name())
write!(f, "{}", self.clone().name())
}
}

Expand All @@ -86,9 +93,7 @@ impl<'a> TryFrom<&'a str> for FilterStatKey {
"web-crawlers" => FilterStatKey::WebCrawlers,
"invalid-csp" => FilterStatKey::InvalidCsp,
"filtered-transaction" => FilterStatKey::FilteredTransactions,
other => {
return Err(other);
}
other => FilterStatKey::GenericFilter(other.to_string()),
})
}
}
119 changes: 118 additions & 1 deletion relay-filter/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::convert::Infallible;
use std::str::FromStr;

use relay_common::glob3::GlobPatterns;
use relay_protocol::RuleCondition;
use serde::{Deserialize, Serialize};

/// Common configuration for event filters.
Expand Down Expand Up @@ -196,6 +197,41 @@ impl LegacyBrowsersFilterConfig {
}
}

/// Configuration for a generic filter.
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct GenericFilterConfig {
/// Unique identifier of the generic filter.
pub id: String,
/// Specifies whether this filter is enabled.
pub is_enabled: bool,
/// The condition for the filter.
pub condition: Option<RuleCondition>,
}

impl GenericFilterConfig {
/// Returns true if the filter is not enabled or no condition was supplied.
pub fn is_empty(&self) -> bool {
!self.is_enabled || self.condition.is_none()
}
}

/// Configuration for generic filters.
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
pub(crate) struct GenericFiltersConfig {
/// Version of the filters configuration.
pub version: u16,
/// List of generic filters.
pub filters: Vec<GenericFilterConfig>,
}

impl GenericFiltersConfig {
/// Returns true if the filters are not declared.
pub fn is_empty(&self) -> bool {
self.filters.is_empty()
}
}

/// Configuration for all event filters.
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -238,10 +274,14 @@ pub struct FiltersConfig {
skip_serializing_if = "IgnoreTransactionsFilterConfig::is_empty"
)]
pub ignore_transactions: IgnoreTransactionsFilterConfig,

/// Configuration for generic filters.
#[serde(default, skip_serializing_if = "GenericFiltersConfig::is_empty")]
pub(crate) generic: GenericFiltersConfig,
}

impl FiltersConfig {
/// Returns true if there are no filter configurations delcared.
/// Returns true if there are no filter configurations declared.
pub fn is_empty(&self) -> bool {
self.browser_extensions.is_empty()
&& self.client_ips.is_empty()
Expand All @@ -252,6 +292,7 @@ impl FiltersConfig {
&& self.localhost.is_empty()
&& self.releases.is_empty()
&& self.ignore_transactions.is_empty()
&& self.generic.is_empty()
}
}

Expand Down Expand Up @@ -293,6 +334,10 @@ mod tests {
patterns: [],
is_enabled: false,
},
generic: GenericFiltersConfig {
version: 0,
filters: [],
},
}
"###);
Ok(())
Expand Down Expand Up @@ -333,6 +378,14 @@ mod tests {
patterns: GlobPatterns::new(vec!["*health*".to_string()]),
is_enabled: true,
},
generic: GenericFiltersConfig {
version: 1,
filters: vec![GenericFilterConfig {
id: "hydrationError".to_string(),
is_enabled: true,
condition: Some(RuleCondition::eq("event.exceptions", "HydrationError")),
}],
},
};

insta::assert_json_snapshot!(filters_config, @r#"
Expand Down Expand Up @@ -378,6 +431,20 @@ mod tests {
"*health*"
],
"isEnabled": true
},
"generic": {
"version": 1,
"filters": [
{
"id": "hydrationError",
"isEnabled": true,
"condition": {
"op": "eq",
"name": "event.exceptions",
"value": "HydrationError"
}
}
]
}
}
"#);
Expand All @@ -394,4 +461,54 @@ mod tests {
}
"###);
}

#[test]
fn test_deserialize_generic_filters() {
let json = r#"{
"version": 1,
"filters": [
{
"id": "hydrationError",
"isEnabled": true,
"condition": {
"op": "eq",
"name": "event.exceptions",
"value": "HydrationError"
}
},
{
"id": "chunkLoadError",
"isEnabled": false
}
]
}"#;
let config = serde_json::from_str::<GenericFiltersConfig>(json).unwrap();
insta::assert_debug_snapshot!(config, @r###"
GenericFiltersConfig {
version: 1,
filters: [
GenericFilterConfig {
id: "hydrationError",
is_enabled: true,
condition: Some(
Eq(
EqCondition {
name: "event.exceptions",
value: String("HydrationError"),
options: EqCondOptions {
ignore_case: false,
},
},
),
),
},
GenericFilterConfig {
id: "chunkLoadError",
is_enabled: false,
condition: None,
},
],
}
"###);
}
}
149 changes: 149 additions & 0 deletions relay-filter/src/generic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
//! Implements generic filtering based on the [`RuleCondition`] DSL.
//!
//! Multiple generic filters can be defined and they are going to be checked in FIFO order. The
//! first one that matches, will result in the event being discarded with a [`FilterStatKey`]
//! identifying the matching filter.

use crate::{FilterStatKey, GenericFiltersConfig};
use relay_event_schema::protocol::Event;
use relay_protocol::RuleCondition;

/// Maximum supported version of the generic filters schema.
///
/// If the version in the project config is higher, no generic filters are applied.
pub const VERSION: u16 = 1;

fn is_enabled(config: &GenericFiltersConfig) -> bool {
config.version > 0 && config.version <= VERSION
}

/// Checks events by patterns in their error messages.
fn matches(event: &Event, condition: Option<&RuleCondition>) -> bool {
// TODO: the condition DSL needs to be extended to support more complex semantics, such as
// collections operations.
condition.map_or(false, |condition| condition.matches(event))
}

/// Filters events by patterns in their error messages.
pub(crate) fn should_filter(
event: &Event,
config: &GenericFiltersConfig,
) -> Result<(), FilterStatKey> {
// We check if the configuration is enabled, since we support only configuration with a version
// <= than the maximum one in this Relay instance.
if !is_enabled(config) {
return Ok(());
}

for filter_config in config.filters.iter() {
if !filter_config.is_empty() && matches(event, filter_config.condition.as_ref()) {
return Err(FilterStatKey::GenericFilter(filter_config.id.clone()));
}
}

Ok(())
}

#[cfg(test)]
mod tests {
use crate::generic::{should_filter, VERSION};
use crate::{FilterStatKey, GenericFilterConfig, GenericFiltersConfig};
use relay_event_schema::protocol::{Event, LenientString};
use relay_protocol::Annotated;
use relay_protocol::RuleCondition;

fn mock_filters() -> Vec<GenericFilterConfig> {
vec![
GenericFilterConfig {
id: "firstReleases".to_string(),
is_enabled: true,
condition: Some(RuleCondition::eq("event.release", "1.0")),
},
GenericFilterConfig {
id: "helloTransactions".to_string(),
is_enabled: true,
condition: Some(RuleCondition::eq("event.transaction", "/hello")),
},
]
}

#[test]
fn test_should_filter_match_rules() {
let config = GenericFiltersConfig {
version: 1,
filters: mock_filters(),
};

// Matching first rule.
let event = Event {
release: Annotated::new(LenientString("1.0".to_string())),
..Default::default()
};
assert_eq!(
should_filter(&event, &config),
Err(FilterStatKey::GenericFilter("firstReleases".to_string()))
);

// Matching second rule.
let event = Event {
transaction: Annotated::new("/hello".to_string()),
..Default::default()
};
assert_eq!(
should_filter(&event, &config),
Err(FilterStatKey::GenericFilter(
"helloTransactions".to_string()
))
);
}

#[test]
fn test_should_filter_fifo_match_rules() {
let config = GenericFiltersConfig {
version: 1,
filters: mock_filters(),
};

// Matching both rules (first is taken).
let event = Event {
release: Annotated::new(LenientString("1.0".to_string())),
transaction: Annotated::new("/hello".to_string()),
..Default::default()
};
assert_eq!(
should_filter(&event, &config),
Err(FilterStatKey::GenericFilter("firstReleases".to_string()))
);
}

#[test]
fn test_should_filter_match_no_rules() {
let config = GenericFiltersConfig {
version: 1,
filters: mock_filters(),
};

// Matching no rule.
let event = Event {
transaction: Annotated::new("/world".to_string()),
..Default::default()
};
assert_eq!(should_filter(&event, &config), Ok(()));
}

#[test]
fn test_should_filter_with_higher_config_version() {
let config = GenericFiltersConfig {
// We simulate receiving a higher configuration version, which we don't support.
version: VERSION + 1,
filters: mock_filters(),
};

let event = Event {
release: Annotated::new(LenientString("1.0".to_string())),
transaction: Annotated::new("/hello".to_string()),
..Default::default()
};
assert_eq!(should_filter(&event, &config), Ok(()));
}
}
Loading

0 comments on commit ee3036c

Please sign in to comment.