feat(server): Add inbound filters functionality to dynamic sampling (#…

…920)

This PR adds the ability to use all inbound filter functionality in dynamic sampling rules.
All conditions that activate an inbound filter can now be used in dynamic sampling rules so
we can replicate all inbound filter functionality (and more) with dynamic sampling rules.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+
 **Features**:
 
 - Relay now picks up HTTP proxies from environment variables. This is made possible by switching to a different HTTP client library.
@@ -15,6 +16,7 @@
 **Internal**:
 
 - Emit the `category` field for outcomes of events. This field disambiguates error events, security events and transactions. As a side-effect, Relay no longer emits outcomes for broken JSON payloads or network errors. ([#931](https://github.com/getsentry/relay/pull/931))
+- Add inbound filters functionality to dynamic sampling rules. ([#920](https://github.com/getsentry/relay/pull/920))
 - The undocumented `http._client` option has been removed. ([#938](https://github.com/getsentry/relay/pull/938))
 
 ## 21.2.0

relay-filter/src/browser_extensions.rs

@@ -7,24 +7,32 @@ use relay_general::protocol::{Event, Exception};
 
 use crate::{FilterConfig, FilterStatKey};
 
-/// Filters events originating from known problematic browser extensions.
-pub fn should_filter(event: &Event, config: &FilterConfig) -> Result<(), FilterStatKey> {
-    if !config.is_enabled {
-        return Ok(());
-    }
-
+/// Check if the event originates from known problematic browser extensions.
+pub fn matches(event: &Event) -> bool {
     if let Some(ex_val) = get_exception_value(event) {
         if EXTENSION_EXC_VALUES.is_match(ex_val) {
-            return Err(FilterStatKey::BrowserExtensions);
+            return true;
         }
     }
     if let Some(ex_source) = get_exception_source(event) {
         if EXTENSION_EXC_SOURCES.is_match(ex_source) {
-            return Err(FilterStatKey::BrowserExtensions);
+            return true;
         }
     }
+    false
+}
+
+/// Filters events originating from known problematic browser extensions.
+pub fn should_filter(event: &Event, config: &FilterConfig) -> Result<(), FilterStatKey> {
+    if !config.is_enabled {
+        return Ok(());
+    }
 
-    Ok(())
+    if matches(event) {
+        Err(FilterStatKey::BrowserExtensions)
+    } else {
+        Ok(())
+    }
 }
 
 fn get_first_exception(event: &Event) -> Option<&Exception> {

relay-filter/src/client_ips.rs

@@ -10,32 +10,45 @@ use ipnetwork::IpNetwork;
 
 use crate::{ClientIpsFilterConfig, FilterStatKey};
 
-/// Filters events by blacklisted client IP ranges.
+/// Checks if the event is part of the blacklisted client IP ranges.
 ///
 /// The client IP is the address of the originator of the event. If it was forwarded through
 /// multiple proxies, this address should be derived from the `X-Forwarded-For` header. Otherwise,
 /// it is the remote socket address.
-pub fn should_filter(
-    client_ip: Option<IpAddr>,
-    config: &ClientIpsFilterConfig,
-) -> Result<(), FilterStatKey> {
-    let blacklisted_ips = &config.blacklisted_ips;
-    if blacklisted_ips.is_empty() {
-        return Ok(());
-    }
-
+pub fn matches<It, S>(client_ip: Option<IpAddr>, blacklisted_ips: It) -> bool
+where
+    It: IntoIterator<Item = S>,
+    S: AsRef<str>,
+{
     let client_ip = match client_ip {
         Some(client_ip) => client_ip,
-        None => return Ok(()),
+        None => return false,
     };
 
     for blacklisted_ip in blacklisted_ips {
-        if let Ok(blacklisted_network) = blacklisted_ip.parse::<IpNetwork>() {
+        if let Ok(blacklisted_network) = blacklisted_ip.as_ref().parse::<IpNetwork>() {
             if blacklisted_network.contains(client_ip) {
-                return Err(FilterStatKey::IpAddress);
+                return true;
             }
         }
     }
+    false
+}
+
+/// Filters events by blacklisted client IP ranges.
+///
+/// The client IP is the address of the originator of the event. If it was forwarded through
+/// multiple proxies, this address should be derived from the `X-Forwarded-For` header. Otherwise,
+/// it is the remote socket address.
+pub fn should_filter(
+    client_ip: Option<IpAddr>,
+    config: &ClientIpsFilterConfig,
+) -> Result<(), FilterStatKey> {
+    let blacklisted_ips = &config.blacklisted_ips;
+
+    if matches(client_ip, blacklisted_ips) {
+        return Err(FilterStatKey::IpAddress);
+    }
 
     Ok(())
 }

relay-filter/src/config.rs

@@ -2,6 +2,8 @@
 
 use std::borrow::Cow;
 use std::collections::BTreeSet;
+use std::convert::Infallible;
+use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
 
@@ -47,14 +49,11 @@ pub enum LegacyBrowser {
     Unknown(String),
 }
 
-impl<'de> Deserialize<'de> for LegacyBrowser {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::de::Deserializer<'de>,
-    {
-        let string = Cow::<str>::deserialize(deserializer)?;
+impl FromStr for LegacyBrowser {
+    type Err = Infallible;
 
-        Ok(match string.as_ref() {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let v = match s {
             "default" => LegacyBrowser::Default,
             "ie_pre_9" => LegacyBrowser::IePre9,
             "ie9" => LegacyBrowser::Ie9,
@@ -64,8 +63,19 @@ impl<'de> Deserialize<'de> for LegacyBrowser {
             "opera_mini_pre_8" => LegacyBrowser::OperaMiniPre8,
             "android_pre_4" => LegacyBrowser::AndroidPre4,
             "safari_pre_6" => LegacyBrowser::SafariPre6,
-            _ => LegacyBrowser::Unknown(string.into_owned()),
-        })
+            _ => LegacyBrowser::Unknown(s.to_owned()),
+        };
+        Ok(v)
+    }
+}
+
+impl<'de> Deserialize<'de> for LegacyBrowser {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::de::Deserializer<'de>,
+    {
+        let s = Cow::<str>::deserialize(deserializer)?;
+        Ok(LegacyBrowser::from_str(s.as_ref()).unwrap())
     }
 }
 

relay-filter/src/csp.rs

@@ -6,40 +6,54 @@ use relay_general::protocol::{Event, EventType};
 
 use crate::{CspFilterConfig, FilterStatKey};
 
-/// Filters CSP events based on disallowed sources.
-pub fn should_filter(event: &Event, config: &CspFilterConfig) -> Result<(), FilterStatKey> {
-    let disallowed_sources = &config.disallowed_sources;
-    if disallowed_sources.is_empty() || event.ty.value() != Some(&EventType::Csp) {
-        return Ok(());
+/// Checks if the event is a CSP Event from one of the disallowed sources.
+pub fn matches<It, S>(event: &Event, disallowed_sources: It) -> bool
+where
+    It: IntoIterator<Item = S>,
+    S: AsRef<str>,
+{
+    if event.ty.value() != Some(&EventType::Csp) {
+        return false;
     }
 
     // parse the sources for easy processing
     let disallowed_sources: Vec<SchemeDomainPort> = disallowed_sources
-        .iter()
-        .map(|origin| -> SchemeDomainPort { origin.as_str().into() })
+        .into_iter()
+        .map(|origin| -> SchemeDomainPort { origin.as_ref().into() })
         .collect();
 
     if let Some(csp) = event.csp.value() {
         if matches_any_origin(csp.blocked_uri.as_str(), &disallowed_sources) {
-            return Err(FilterStatKey::InvalidCsp);
+            return true;
         }
         if matches_any_origin(csp.source_file.as_str(), &disallowed_sources) {
-            return Err(FilterStatKey::InvalidCsp);
+            return true;
         }
     }
+    false
+}
 
-    Ok(())
+/// Filters CSP events based on disallowed sources.
+pub fn should_filter(event: &Event, config: &CspFilterConfig) -> Result<(), FilterStatKey> {
+    if matches(event, &config.disallowed_sources) {
+        Err(FilterStatKey::InvalidCsp)
+    } else {
+        Ok(())
+    }
 }
 
-/// A pattern used to match allowed paths
+/// A pattern used to match allowed paths.
 ///
-/// scheme, domain and port are extracted from an url
+/// Scheme, domain and port are extracted from an url,
 /// they may be either a string (to be matched exactly, case insensitive)
-/// or None (matches anything in the respective position)
+/// or None (matches anything in the respective position).
 #[derive(Hash, PartialEq, Eq)]
 pub struct SchemeDomainPort {
+    /// The scheme of the url.
     pub scheme: Option<String>,
+    /// The domain of the url.
     pub domain: Option<String>,
+    /// The port of the url.
     pub port: Option<String>,
 }
 

relay-filter/src/error_messages.rs

@@ -7,18 +7,19 @@ use std::borrow::Cow;
 
 use relay_general::protocol::Event;
 
-use crate::{ErrorMessagesFilterConfig, FilterStatKey};
+use crate::{ErrorMessagesFilterConfig, FilterStatKey, GlobPatterns};
 
-/// Filters events by patterns in their error messages.
-pub fn should_filter(
-    event: &Event,
-    config: &ErrorMessagesFilterConfig,
-) -> Result<(), FilterStatKey> {
+/// Checks events by patterns in their error messages.
+pub fn matches(event: &Event, patterns: &GlobPatterns) -> bool {
     if let Some(logentry) = event.logentry.value() {
         if let Some(message) = logentry.formatted.value() {
-            should_filter_impl(message.as_ref(), config)?;
+            if patterns.is_match(message.as_ref()) {
+                return true;
+            }
         } else if let Some(message) = logentry.message.value() {
-            should_filter_impl(message.as_ref(), config)?;
+            if patterns.is_match(message.as_ref()) {
+                return true;
+            }
         }
     }
 
@@ -33,21 +34,22 @@ pub fn should_filter(
                         (ty, "") => Cow::Borrowed(ty),
                         (ty, value) => Cow::Owned(format!("{}: {}", ty, value)),
                     };
-
-                    should_filter_impl(&message, config)?;
+                    if patterns.is_match(message.as_ref()) {
+                        return true;
+                    }
                 }
             }
         }
     }
-
-    Ok(())
+    false
 }
 
-fn should_filter_impl(
-    message: &str,
+/// Filters events by patterns in their error messages.
+pub fn should_filter(
+    event: &Event,
     config: &ErrorMessagesFilterConfig,
 ) -> Result<(), FilterStatKey> {
-    if config.patterns.is_match(message) {
+    if matches(event, &config.patterns) {
         Err(FilterStatKey::ErrorMessage)
     } else {
         Ok(())