Skip to content

Commit

Permalink
feat(cardinality): Add cardinality limits config to project config (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Dav1dde authored Jan 22, 2024
1 parent 009504e commit faad766
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 26 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions relay-cardinality/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ redis = ["relay-redis/impl"]

[dependencies]
hashbrown = { workspace = true }
relay-base-schema = { path = "../relay-base-schema" }
relay-log = { path = "../relay-log" }
relay-redis = { path = "../relay-redis" }
relay-statsd = { path = "../relay-statsd" }
serde = { workspace = true }
thiserror = { workspace = true }

[dev-dependencies]
Expand Down
35 changes: 35 additions & 0 deletions relay-cardinality/src/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use relay_base_schema::metrics::MetricNamespace;
use serde::{Deserialize, Serialize};

use crate::SlidingWindow;

/// A cardinality limit.
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct CardinalityLimit {
/// Unique identifier of the cardinality limit.
pub id: String,
/// The sliding window to enforce the cardinality limits in.
pub window: SlidingWindow,
/// The cardinality limit.
pub limit: u64,

/// Scope which the limit applies to.
pub scope: CardinalityScope,
/// Metric namespace the limit applies to.
///
/// No namespace means this specific limit is enforced across all namespaces.
pub namespace: Option<MetricNamespace>,
}

/// A scope to restrict the [`CardinalityLimit`] to.
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub enum CardinalityScope {
/// The organization that this project belongs to.
///
/// This is the top-level scope.
Organization,

/// Any other scope that is not known by this Relay.
#[serde(other)]
Unknown,
}
2 changes: 2 additions & 0 deletions relay-cardinality/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#[cfg(feature = "redis")]
mod cache;
mod config;
mod error;
pub mod limiter;
#[cfg(feature = "redis")]
Expand All @@ -17,6 +18,7 @@ mod statsd;
mod utils;
mod window;

pub use self::config::*;
pub use self::error::*;
pub use self::limiter::{
CardinalityItem, CardinalityLimits, CardinalityScope, Config as CardinalityLimiterConfig,
Expand Down
4 changes: 3 additions & 1 deletion relay-cardinality/src/window.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use serde::{Deserialize, Serialize};

/// A sliding window.
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct SlidingWindow {
/// The number of seconds to apply the limit to.
pub window_seconds: u64,
Expand Down
1 change: 1 addition & 0 deletions relay-dynamic-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ anyhow = { workspace = true }
assert-json-diff = "2.0.2"
relay-auth = { path = "../relay-auth" }
relay-base-schema = { path = "../relay-base-schema" }
relay-cardinality = { path = "../relay-cardinality" }
relay-common = { path = "../relay-common" }
relay-filter = { path = "../relay-filter" }
relay-event-normalization = { path = "../relay-event-normalization" }
Expand Down
27 changes: 16 additions & 11 deletions relay-dynamic-config/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::collections::BTreeSet;

use relay_base_schema::data_category::DataCategory;
use relay_cardinality::CardinalityLimit;
use relay_common::glob2::LazyGlob;
use relay_common::glob3::GlobPatterns;
use relay_protocol::RuleCondition;
Expand All @@ -11,24 +12,21 @@ use serde::{Deserialize, Serialize};
use crate::project::ProjectConfig;

/// Configuration for metrics filtering.
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq)]
#[serde(default, rename_all = "camelCase")]
pub struct Metrics {
/// List of cardinality limits to enforce for this project.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub cardinality_limits: Vec<CardinalityLimit>,
/// List of patterns for blocking metrics based on their name.
#[serde(skip_serializing_if = "GlobPatterns::is_empty")]
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.
/// Returns `true` if there are no changes to the metrics config.
pub fn is_empty(&self) -> bool {
self.denied_names.is_empty()
self.cardinality_limits.is_empty() && self.denied_names.is_empty()
}
}

Expand All @@ -53,7 +51,7 @@ const SESSION_EXTRACT_VERSION: u16 = 3;
const EXTRACT_ABNORMAL_MECHANISM_VERSION: u16 = 2;

/// Configuration for metric extraction from sessions.
#[derive(Debug, Clone, Copy, Default, serde::Deserialize, serde::Serialize)]
#[derive(Debug, Clone, Copy, Default, Deserialize, Serialize)]
#[serde(default, rename_all = "camelCase")]
pub struct SessionMetricsConfig {
/// The revision of the extraction algorithm.
Expand Down Expand Up @@ -505,6 +503,13 @@ mod tests {
use super::*;
use similar_asserts::assert_eq;

#[test]
fn test_empty_metrics_deserialize() {
let m: Metrics = serde_json::from_str("{}").unwrap();
assert!(m.is_empty());
assert_eq!(m, Metrics::default());
}

#[test]
fn parse_tag_spec_value() {
let json = r#"{"key":"foo","value":"bar"}"#;
Expand Down
28 changes: 14 additions & 14 deletions relay-server/src/services/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,7 @@ impl Project {
mod tests {
use std::sync::{Arc, Mutex};

use relay_common::glob3::GlobPatterns;
use relay_common::time::UnixTimestamp;
use relay_metrics::BucketValue;
use relay_test::mock_service;
Expand Down Expand Up @@ -1474,23 +1475,22 @@ mod tests {
.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);
fn apply_pattern_to_names(names: Vec<&str>, patterns: &[&str]) -> Vec<String> {
let metrics = Metrics {
denied_names: GlobPatterns::new(patterns.iter().map(|&s| s.to_owned()).collect()),
..Default::default()
};

Project::apply_metrics_deny_list(&ErrorBoundary::Ok(deny_list), &mut buckets);
let mut buckets = get_test_buckets(&names);
Project::apply_metrics_deny_list(&ErrorBoundary::Ok(metrics), &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"]);
let remaining_names = apply_pattern_to_names(names, &["endpoint.parallel_requests"]);

// There's 1 bucket with that exact name.
let buckets_to_remove = 1;
Expand All @@ -1502,7 +1502,7 @@ mod tests {
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"]);
let remaining_names = apply_pattern_to_names(names, &["*foo"]);

// There's 1 bucket name with 'foo' in the end.
let buckets_to_remove = 1;
Expand All @@ -1514,7 +1514,7 @@ mod tests {
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*"]);
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;
Expand All @@ -1526,7 +1526,7 @@ mod tests {
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*"]);
let remaining_names = apply_pattern_to_names(names, &["endpoint*"]);

// There's 4 buckets starting with "endpoint".
let buckets_to_remove = 4;
Expand All @@ -1537,7 +1537,7 @@ mod tests {
#[test]
fn test_metric_deny_list_everything() {
let names = get_test_bucket_names();
let remaining_names = apply_pattern_to_names(names, ["*"]);
let remaining_names = apply_pattern_to_names(names, &["*"]);

assert_eq!(remaining_names.len(), 0);
}
Expand All @@ -1546,7 +1546,7 @@ mod tests {
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 remaining_names = apply_pattern_to_names(names, &["endpoint*", "*transactions*"]);

let endpoint_buckets = 4;
let transaction_buckets = 3;
Expand Down

0 comments on commit faad766

Please sign in to comment.