From 009504ebb20d8033a8d36107109e121c3fd0a2d6 Mon Sep 17 00:00:00 2001 From: Tor Date: Mon, 22 Jan 2024 13:23:36 +0100 Subject: [PATCH] feat(server): Denylist for metrics (#2954) New addition to projectconfig that allow users to filter out certain metrics based on their name using glob patterns. --- CHANGELOG.md | 1 + relay-dynamic-config/src/metrics.rs | 23 +++++ relay-dynamic-config/src/project.rs | 14 ++- relay-server/src/services/project.rs | 149 ++++++++++++++++++++++++++- 4 files changed, 185 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06265f4e50..0a0358ffcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Proactively move on-disk spool to memory. ([#2949](https://github.com/getsentry/relay/pull/2949)) - Default missing `Event.platform` and `Event.level` fields during light normalization. ([#2961](https://github.com/getsentry/relay/pull/2961)) +- Add possiblity to block metrics with glob-patterns. ([#2954](https://github.com/getsentry/relay/pull/2954)) **Bug Fixes**: diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs index f2882240d0..fd9927a7f3 100644 --- a/relay-dynamic-config/src/metrics.rs +++ b/relay-dynamic-config/src/metrics.rs @@ -4,11 +4,34 @@ use std::collections::BTreeSet; use relay_base_schema::data_category::DataCategory; use relay_common::glob2::LazyGlob; +use relay_common::glob3::GlobPatterns; use relay_protocol::RuleCondition; use serde::{Deserialize, Serialize}; use crate::project::ProjectConfig; +/// Configuration for metrics filtering. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[serde(default, rename_all = "camelCase")] +pub struct Metrics { + /// List of patterns for blocking metrics based on their name. + pub denied_names: GlobPatterns, +} + +impl Metrics { + /// Returns a new instance of [`Metrics`]. + pub fn new(patterns: Vec) -> Self { + Self { + denied_names: GlobPatterns::new(patterns), + } + } + + /// Returns `true` if there are no blocked metrics. + pub fn is_empty(&self) -> bool { + self.denied_names.is_empty() + } +} + /// Rule defining when a target tag should be set on a metric. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/relay-dynamic-config/src/project.rs b/relay-dynamic-config/src/project.rs index 4cbfc8e0cb..7ab32379cc 100644 --- a/relay-dynamic-config/src/project.rs +++ b/relay-dynamic-config/src/project.rs @@ -17,7 +17,8 @@ use crate::defaults; use crate::error_boundary::ErrorBoundary; use crate::feature::FeatureSet; use crate::metrics::{ - self, MetricExtractionConfig, SessionMetricsConfig, TaggingRule, TransactionMetricsConfig, + self, MetricExtractionConfig, Metrics, SessionMetricsConfig, TaggingRule, + TransactionMetricsConfig, }; /// Dynamic, per-DSN configuration passed down from Sentry. @@ -88,6 +89,9 @@ pub struct ProjectConfig { /// relays that might still need them. #[serde(skip_serializing_if = "Option::is_none")] pub span_description_rules: Option>, + /// Configuration for metrics. + #[serde(default, skip_serializing_if = "skip_metrics")] + pub metrics: ErrorBoundary, } impl ProjectConfig { @@ -128,6 +132,7 @@ impl Default for ProjectConfig { tx_name_rules: Vec::new(), tx_name_ready: false, span_description_rules: None, + metrics: Default::default(), } } } @@ -139,6 +144,13 @@ fn skip_metrics_extraction(boundary: &ErrorBoundary) -> } } +fn skip_metrics(boundary: &ErrorBoundary) -> bool { + match boundary { + ErrorBoundary::Err(_) => true, + ErrorBoundary::Ok(metrics) => metrics.is_empty(), + } +} + /// Subset of [`ProjectConfig`] that is passed to external Relays. /// /// For documentation of the fields, see [`ProjectConfig`]. diff --git a/relay-server/src/services/project.rs b/relay-server/src/services/project.rs index 37d958e5d0..c976f39d3e 100644 --- a/relay-server/src/services/project.rs +++ b/relay-server/src/services/project.rs @@ -4,7 +4,7 @@ use std::time::Duration; use chrono::{DateTime, Utc}; use relay_base_schema::project::{ProjectId, ProjectKey}; use relay_config::Config; -use relay_dynamic_config::{ErrorBoundary, Feature, LimitedProjectConfig, ProjectConfig}; +use relay_dynamic_config::{ErrorBoundary, Feature, LimitedProjectConfig, Metrics, ProjectConfig}; use relay_filter::matches_any_origin; use relay_metrics::aggregator::AggregatorConfig; use relay_metrics::{ @@ -585,6 +585,26 @@ impl Project { return; }; + Self::apply_metrics_deny_list(&state.config.metrics, metrics); + Self::filter_disabled_namespace(state, metrics); + } + + fn apply_metrics_deny_list(deny_list: &ErrorBoundary, buckets: &mut Vec) { + let ErrorBoundary::Ok(metrics) = deny_list else { + return; + }; + + buckets.retain(|bucket| { + if metrics.denied_names.is_match(&bucket.name) { + relay_log::trace!(mri = bucket.name, "dropping metrics due to block list"); + false + } else { + true + } + }); + } + + fn filter_disabled_namespace(state: &ProjectState, metrics: &mut Vec) { metrics.retain(|metric| { let Ok(mri) = MetricResourceIdentifier::parse(&metric.name) else { relay_log::trace!(mri = metric.name, "dropping metrics with invalid MRI"); @@ -1420,4 +1440,131 @@ mod tests { assert!(metrics.is_empty()); } + + fn get_test_buckets(names: &[&str]) -> Vec { + let create_bucket = |name: &&str| -> Bucket { + let json = json!({ + "timestamp": 1615889440, + "width": 10, + "name": name, + "type": "c", + "value": 4.0, + "tags": { + "route": "user_index" + }}); + + serde_json::from_value(json).unwrap() + }; + + names.iter().map(create_bucket).collect() + } + + fn get_test_bucket_names() -> Vec<&'static str> { + [ + "g:transactions/foo@none", + "c:custom/foo@none", + "transactions/foo@second", + "transactions/foo", + "c:custom/foo_bar@none", + "endpoint.response_time", + "endpoint.hits", + "endpoint.parallel_requests", + "endpoint.users", + ] + .into() + } + + fn apply_pattern_to_names<'a>( + names: Vec<&str>, + patterns: impl AsRef<[&'a str]>, + ) -> Vec { + let mut buckets = get_test_buckets(&names); + let patterns: Vec = patterns.as_ref().iter().map(|s| (*s).to_owned()).collect(); + let deny_list = Metrics::new(patterns); + + Project::apply_metrics_deny_list(&ErrorBoundary::Ok(deny_list), &mut buckets); + buckets.into_iter().map(|bucket| bucket.name).collect() + } + + #[test] + fn test_metric_deny_list_exact() { + let names = get_test_bucket_names(); + let input_qty = names.len(); + let remaining_names = apply_pattern_to_names(names, ["endpoint.parallel_requests"]); + + // There's 1 bucket with that exact name. + let buckets_to_remove = 1; + + assert_eq!(remaining_names.len(), input_qty - buckets_to_remove); + } + + #[test] + fn test_metric_deny_list_end_glob() { + let names = get_test_bucket_names(); + let input_qty = names.len(); + let remaining_names = apply_pattern_to_names(names, ["*foo"]); + + // There's 1 bucket name with 'foo' in the end. + let buckets_to_remove = 1; + + assert_eq!(remaining_names.len(), input_qty - buckets_to_remove); + } + + #[test] + fn test_metric_deny_list_middle_glob() { + let names = get_test_bucket_names(); + let input_qty = names.len(); + let remaining_names = apply_pattern_to_names(names, ["*foo*"]); + + // There's 4 bucket names with 'foo' in the middle, and one with foo in the end. + let buckets_to_remove = 5; + + assert_eq!(remaining_names.len(), input_qty - buckets_to_remove); + } + + #[test] + fn test_metric_deny_list_beginning_glob() { + let names = get_test_bucket_names(); + let input_qty = names.len(); + let remaining_names = apply_pattern_to_names(names, ["endpoint*"]); + + // There's 4 buckets starting with "endpoint". + let buckets_to_remove = 4; + + assert_eq!(remaining_names.len(), input_qty - buckets_to_remove); + } + + #[test] + fn test_metric_deny_list_everything() { + let names = get_test_bucket_names(); + let remaining_names = apply_pattern_to_names(names, ["*"]); + + assert_eq!(remaining_names.len(), 0); + } + + #[test] + fn test_metric_deny_list_multiple() { + let names = get_test_bucket_names(); + let input_qty = names.len(); + let remaining_names = apply_pattern_to_names(names, ["endpoint*", "*transactions*"]); + + let endpoint_buckets = 4; + let transaction_buckets = 3; + + assert_eq!( + remaining_names.len(), + input_qty - endpoint_buckets - transaction_buckets + ); + } + + #[test] + fn test_serialize_metrics_config() { + let input_str = r#"{"deniedNames":["foo","bar"]}"#; + + let deny_list: Metrics = serde_json::from_str(input_str).unwrap(); + + let back_to_str = serde_json::to_string(&deny_list).unwrap(); + + assert_eq!(input_str, back_to_str); + } }