Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 28 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7a73ec3
add browser extensions and localhost to dyn sampling filters
RaduW Feb 2, 2021
6533fcc
changed rule matching to type specific implementations
RaduW Feb 3, 2021
77fe904
added legacy browser dynamic filter
RaduW Feb 3, 2021
1e68a4c
added csp dynamic filter
RaduW Feb 3, 2021
92e95ae
added error messages filter
RaduW Feb 3, 2021
8fe4040
added tests for ported inbound filters into dynamic sampling rules.
RaduW Feb 4, 2021
9309ce2
add PR description to changelog
RaduW Feb 5, 2021
9d05f1c
add client_ip rule
RaduW Feb 5, 2021
3486015
librdkafka use only v4 for producer
beezz Feb 9, 2021
1b0eee5
Merge remote-tracking branch 'origin/integration-tests-kafka-update' …
RaduW Feb 9, 2021
19c888f
small fix - intermittent failure
RaduW Feb 9, 2021
54ecee5
merge master
RaduW Feb 11, 2021
bb66229
Merge branch 'master' into feat/dyn-sampling-filters
RaduW Feb 15, 2021
5eccfbb
added new (uniform) serialization format for dynamic filters
RaduW Feb 23, 2021
f677a1e
Merge remote-tracking branch 'origin/master' into feat/dyn-sampling-f…
RaduW Feb 23, 2021
0a14c76
added missing unit tests for dynamic filters
RaduW Feb 24, 2021
a9dce34
manually reverting "librdkafka use only v4 for producer" 348601522894
RaduW Feb 24, 2021
d07406e
merge master
RaduW Feb 24, 2021
8a85e43
minor, fix naming consistency
RaduW Feb 24, 2021
a122319
fixes from code review
RaduW Feb 25, 2021
339b848
fix rule generation to new format (in tests)
RaduW Feb 25, 2021
7e5ec74
Merge branch 'master' into feat/dyn-sampling-filters
RaduW Feb 25, 2021
93e8b20
clenup lint errors
RaduW Feb 25, 2021
2e3fc31
minor, imports cleanup
RaduW Feb 25, 2021
0c1cc64
changed LegacyBrowser trait from `From<&str>` to `FromStr`
RaduW Feb 25, 2021
82df5db
Update relay-server/src/utils/dynamic_sampling.rs
RaduW Feb 26, 2021
827737d
lint rust code
RaduW Feb 26, 2021
ebf3c94
minor, dependency cleanup
RaduW Feb 26, 2021
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
26 changes: 17 additions & 9 deletions relay-filter/src/browser_extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
39 changes: 26 additions & 13 deletions relay-filter/src/client_ips.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
25 changes: 15 additions & 10 deletions relay-filter/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,9 @@ 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)?;

Ok(match string.as_ref() {
impl From<&str> for LegacyBrowser {
RaduW marked this conversation as resolved.
Show resolved Hide resolved
fn from(val: &str) -> Self {
match val {
"default" => LegacyBrowser::Default,
"ie_pre_9" => LegacyBrowser::IePre9,
"ie9" => LegacyBrowser::Ie9,
Expand All @@ -64,8 +59,18 @@ 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(val.to_owned()),
}
}
}

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)?;
Ok(string.as_ref().into())
}
}

Expand Down
40 changes: 27 additions & 13 deletions relay-filter/src/csp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down
32 changes: 17 additions & 15 deletions relay-filter/src/error_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand All @@ -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(())
Expand Down
Loading