Skip to content

Commit

Permalink
feat(server): Denylist for metrics (#2954)
Browse files Browse the repository at this point in the history
New addition to projectconfig that allow users to filter out certain
metrics based on their name using glob patterns.
  • Loading branch information
TBS1996 authored Jan 22, 2024
1 parent f3ff94a commit 009504e
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**:

Expand Down
23 changes: 23 additions & 0 deletions relay-dynamic-config/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>) -> 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")]
Expand Down
14 changes: 13 additions & 1 deletion relay-dynamic-config/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<Vec<SpanDescriptionRule>>,
/// Configuration for metrics.
#[serde(default, skip_serializing_if = "skip_metrics")]
pub metrics: ErrorBoundary<Metrics>,
}

impl ProjectConfig {
Expand Down Expand Up @@ -128,6 +132,7 @@ impl Default for ProjectConfig {
tx_name_rules: Vec::new(),
tx_name_ready: false,
span_description_rules: None,
metrics: Default::default(),
}
}
}
Expand All @@ -139,6 +144,13 @@ fn skip_metrics_extraction(boundary: &ErrorBoundary<MetricExtractionConfig>) ->
}
}

fn skip_metrics(boundary: &ErrorBoundary<Metrics>) -> 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`].
Expand Down
149 changes: 148 additions & 1 deletion relay-server/src/services/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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<Metrics>, buckets: &mut Vec<Bucket>) {
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<Bucket>) {
metrics.retain(|metric| {
let Ok(mri) = MetricResourceIdentifier::parse(&metric.name) else {
relay_log::trace!(mri = metric.name, "dropping metrics with invalid MRI");
Expand Down Expand Up @@ -1420,4 +1440,131 @@ mod tests {

assert!(metrics.is_empty());
}

fn get_test_buckets(names: &[&str]) -> Vec<Bucket> {
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<String> {
let mut buckets = get_test_buckets(&names);
let patterns: Vec<String> = 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);
}
}

0 comments on commit 009504e

Please sign in to comment.