From f8bf7823ffdefb87d7b659e07f3bc2d07272a2cb Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 27 Jun 2023 13:29:42 +0200 Subject: [PATCH] feat(dynamic_config): Add config for dynamic metrics extraction (#2252) Adds configuration for dynamic metrics extraction to project configuration. This is inactive at this moment, as actual extraction will be implemented in a follow-up. --- CHANGELOG.md | 12 +- Cargo.lock | 1 + py/CHANGELOG.md | 4 + relay-dynamic-config/Cargo.toml | 1 + relay-dynamic-config/src/error_boundary.rs | 9 + relay-dynamic-config/src/metrics.rs | 182 +++++++++++++++++++++ relay-dynamic-config/src/project.rs | 16 +- relay-filter/src/common.rs | 6 + relay-sampling/src/lib.rs | 16 +- 9 files changed, 235 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 934838190c..f05a5fb511 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,19 @@ **Features**: -- Add is_enabled flag on transaction filter. ([#2251](https://github.com/getsentry/relay/pull/2251)) -- Keep stackframes closest to crash when quantity exceeds limit. ([#2236](https://github.com/getsentry/relay/pull/2236)) - Add filter based on transaction names. ([#2118](https://github.com/getsentry/relay/pull/2118)) -- Drop profiles without a transaction in the same envelope. ([#2169](https://github.com/getsentry/relay/pull/2169)) - Use GeoIP lookup also in non-processing Relays. Lookup from now on will be also run in light normalization. ([#2229](https://github.com/getsentry/relay/pull/2229)) - Scrub identifiers from transactions for old SDKs. ([#2250](https://github.com/getsentry/relay/pull/2250)) +**Bug Fixes**: + +- Keep stack frames closest to crash when quantity exceeds limit. ([#2236](https://github.com/getsentry/relay/pull/2236)) +- Drop profiles without a transaction in the same envelope. ([#2169](https://github.com/getsentry/relay/pull/2169)) + +**Internal**: + +- Add the configuration protocol for generic metrics extraction. ([#2252](https://github.com/getsentry/relay/pull/2252)) + ## 23.6.1 - No documented changes. diff --git a/Cargo.lock b/Cargo.lock index e77283022f..eec3f2f888 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3074,6 +3074,7 @@ dependencies = [ "relay-sampling", "serde", "serde_json", + "similar-asserts", "smallvec", ] diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md index 3fdc333064..035417544d 100644 --- a/py/CHANGELOG.md +++ b/py/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Add the configuration protocol for generic metrics extraction. ([#2252](https://github.com/getsentry/relay/pull/2252)) + ## 0.8.27 - Add is_enabled flag on transaction filter. ([#2251](https://github.com/getsentry/relay/pull/2251)) diff --git a/relay-dynamic-config/Cargo.toml b/relay-dynamic-config/Cargo.toml index ac4f8afb7a..bf4569cb13 100644 --- a/relay-dynamic-config/Cargo.toml +++ b/relay-dynamic-config/Cargo.toml @@ -27,3 +27,4 @@ smallvec = "1.10.0" [dev-dependencies] insta = "1.26.0" +similar-asserts = "1.4.2" diff --git a/relay-dynamic-config/src/error_boundary.rs b/relay-dynamic-config/src/error_boundary.rs index b1dc534b98..4c8e213d0a 100644 --- a/relay-dynamic-config/src/error_boundary.rs +++ b/relay-dynamic-config/src/error_boundary.rs @@ -55,6 +55,15 @@ impl ErrorBoundary { } } +impl Default for ErrorBoundary +where + T: Default, +{ + fn default() -> Self { + Self::Ok(T::default()) + } +} + impl<'de, T> Deserialize<'de> for ErrorBoundary where T: Deserialize<'de>, diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs index 567ff841c7..f5921ae9e3 100644 --- a/relay-dynamic-config/src/metrics.rs +++ b/relay-dynamic-config/src/metrics.rs @@ -2,6 +2,7 @@ use std::collections::BTreeSet; +use relay_common::DataCategory; use relay_sampling::RuleCondition; use serde::{Deserialize, Serialize}; @@ -24,6 +25,7 @@ pub struct TaggingRule { /// Current version of metrics extraction. const SESSION_EXTRACT_VERSION: u16 = 3; const EXTRACT_ABNORMAL_MECHANISM_VERSION: u16 = 2; +const METRIC_EXTRACTION_VERSION: u16 = 1; /// Configuration for metric extraction from sessions. #[derive(Debug, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] @@ -124,3 +126,183 @@ impl TransactionMetricsConfig { self.version > 0 && self.version <= TRANSACTION_EXTRACT_VERSION } } + +/// Configuration for generic extraction of metrics from all data categories. +#[derive(Clone, Default, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct MetricExtractionConfig { + /// Versioning of metrics extraction. Relay skips extraction if the version is not supported. + pub version: u16, + + /// A list of metric specifications to extract. + #[serde(default)] + pub metrics: Vec, + + /// A list of tags to add to previously extracted metrics. + /// + /// These tags add further tags to a range of metrics. If some metrics already have a matching + /// tag extracted, the existing tag is left unchanged. + #[serde(default)] + pub tags: Vec, +} + +impl MetricExtractionConfig { + /// Returns `true` if metric extraction is configured. + pub fn is_enabled(&self) -> bool { + self.version > 0 + && self.version <= METRIC_EXTRACTION_VERSION + && !(self.metrics.is_empty() && self.tags.is_empty()) + } +} + +/// Specification for a metric to extract from some data. +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct MetricSpec { + /// Category of data to extract this metric for. + pub category: DataCategory, + + /// The Metric Resource Identifier (MRI) of the metric to extract. + pub mri: String, + + /// A path to the field to extract the metric from. + /// + /// This value contains a fully qualified expression pointing at the data field in the payload + /// to extract the metric from. It follows the + /// [`FieldValueProvider`](relay_sampling::FieldValueProvider) syntax that is also used for + /// dynamic sampling. + /// + /// How the value is treated depends on the metric type: + /// + /// - **Counter** metrics are a special case, since the default product counters do not count + /// any specific field but rather the occurrence of the event. As such, there is no value + /// expression, and the field is set to `None`. Semantics of specifying remain undefined at + /// this point. + /// - **Distribution** metrics require a numeric value. If the value at the specified path is + /// not numeric, metric extraction will be skipped. + /// - **Set** metrics require a string value, which is then emitted into the set as unique + /// value. Insertion of numbers and other types is undefined. + /// + /// If the field does not exist, extraction is skipped. + #[serde(default)] + pub field: Option, + + /// An optional condition to meet before extraction. + /// + /// See [`RuleCondition`] for all available options to specify and combine conditions. If no + /// condition is specified, the metric is extracted unconditionally. + #[serde(default)] + pub condition: Option, + + /// A list of tags to add to the metric. + /// + /// Tags can be conditional, see [`TagSpec`] for configuration options. For this reason, it is + /// possible to list tag keys multiple times, each with different conditions. The first matching + /// condition will be applied. + #[serde(default)] + pub tags: Vec, +} + +/// Mapping between extracted metrics and additional tags to extract. +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TagMapping { + /// A list of Metric Resource Identifiers (MRI) to apply tags to. + /// + /// Entries in this list can contain wildcards to match metrics with dynamic MRIs. + #[serde(default)] + pub metrics: Vec, + + /// A list of tags to add to the metric. + /// + /// Tags can be conditional, see [`TagSpec`] for configuration options. For this reason, it is + /// possible to list tag keys multiple times, each with different conditions. The first matching + /// condition will be applied. + #[serde(default)] + pub tags: Vec, +} + +/// Configuration for a tag to add to a metric. +/// +/// Tags values can be static if defined through `value` or dynamically queried from the payload if +/// defined through `field`. These two options are mutually exclusive, behavior is undefined if both +/// are specified. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TagSpec { + /// The key of the tag to extract. + pub key: String, + + /// Path to a field containing the tag's value. + /// + /// It follows the [`FieldValueProvider`](relay_sampling::FieldValueProvider) syntax to read + /// data from the payload. + /// + /// Mutually exclusive with `value`. + #[serde(default)] + pub field: Option, + + /// Literal value of the tag. + /// + /// Mutually exclusive with `field`. + #[serde(default)] + pub value: Option, + + /// An optional condition to meet before extraction. + /// + /// See [`RuleCondition`] for all available options to specify and combine conditions. If no + /// condition is specified, the tag is added unconditionally, provided it is not already there. + #[serde(default)] + pub condition: Option, +} + +impl TagSpec { + /// Returns the source of tag values, either literal or a field. + pub fn source(&self) -> TagSource<'_> { + if let Some(ref field) = self.field { + TagSource::Field(field) + } else if let Some(ref value) = self.value { + TagSource::Literal(value) + } else { + TagSource::Unknown + } + } +} + +/// Specifies how to obtain the value of a tag in [`TagSpec`]. +#[derive(Clone, Debug, PartialEq)] +pub enum TagSource<'a> { + /// A literal value. + Literal(&'a str), + /// Path to a field to evaluate. + Field(&'a str), + /// An unsupported or unknown source. + Unknown, +} + +#[cfg(test)] +mod tests { + use super::*; + use similar_asserts::assert_eq; + + #[test] + fn parse_tag_spec_value() { + let json = r#"{"key":"foo","value":"bar"}"#; + let spec: TagSpec = serde_json::from_str(json).unwrap(); + assert_eq!(spec.source(), TagSource::Literal("bar")); + } + + #[test] + fn parse_tag_spec_field() { + let json = r#"{"key":"foo","field":"bar"}"#; + let spec: TagSpec = serde_json::from_str(json).unwrap(); + assert_eq!(spec.source(), TagSource::Field("bar")); + } + + #[test] + fn parse_tag_spec_unsupported() { + let json = r#"{"key":"foo","somethingNew":"bar"}"#; + let spec: TagSpec = serde_json::from_str(json).unwrap(); + assert_eq!(spec.source(), TagSource::Unknown); + } +} diff --git a/relay-dynamic-config/src/project.rs b/relay-dynamic-config/src/project.rs index a1243fd6b5..992a65f862 100644 --- a/relay-dynamic-config/src/project.rs +++ b/relay-dynamic-config/src/project.rs @@ -13,7 +13,10 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use crate::feature::Feature; -use crate::{ErrorBoundary, SessionMetricsConfig, TaggingRule, TransactionMetricsConfig}; +use crate::metrics::{ + MetricExtractionConfig, SessionMetricsConfig, TaggingRule, TransactionMetricsConfig, +}; +use crate::ErrorBoundary; /// Dynamic, per-DSN configuration passed down from Sentry. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -55,6 +58,9 @@ pub struct ProjectConfig { /// Configuration for extracting metrics from transaction events. #[serde(skip_serializing_if = "Option::is_none")] pub transaction_metrics: Option>, + /// Configuration for generic metrics extraction from all data categories. + #[serde(default, skip_serializing_if = "skip_metrics_extraction")] + pub metric_extraction: ErrorBoundary, /// The span attributes configuration. #[serde(skip_serializing_if = "BTreeSet::is_empty")] pub span_attributes: BTreeSet, @@ -91,6 +97,7 @@ impl Default for ProjectConfig { breakdowns_v2: None, session_metrics: SessionMetricsConfig::default(), transaction_metrics: None, + metric_extraction: Default::default(), span_attributes: BTreeSet::new(), metric_conditional_tagging: Vec::new(), features: BTreeSet::new(), @@ -101,6 +108,13 @@ impl Default for ProjectConfig { } } +fn skip_metrics_extraction(boundary: &ErrorBoundary) -> bool { + match boundary { + ErrorBoundary::Err(_) => true, + ErrorBoundary::Ok(config) => !config.is_enabled(), + } +} + /// Subset of [`ProjectConfig`] that is passed to external Relays. /// /// For documentation of the fields, see [`ProjectConfig`]. diff --git a/relay-filter/src/common.rs b/relay-filter/src/common.rs index 093cf14248..4d64a30b58 100644 --- a/relay-filter/src/common.rs +++ b/relay-filter/src/common.rs @@ -98,6 +98,12 @@ impl<'de> Deserialize<'de> for GlobPatterns { } } +impl PartialEq for GlobPatterns { + fn eq(&self, other: &Self) -> bool { + self.patterns == other.patterns + } +} + /// Identifies which filter dropped an event for which reason. /// /// Ported from Sentry's same-named "enum". The enum variants are fed into outcomes in kebap-case diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index 329a0a3879..dd9fe318ed 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -113,7 +113,7 @@ pub struct EqCondOptions { } /// A condition that checks for equality -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct EqCondition { pub name: String, @@ -174,7 +174,7 @@ impl EqCondition { macro_rules! impl_cmp_condition { ($struct_name:ident, $operator:tt) => { - #[derive(Debug, Clone, Serialize, Deserialize)] + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct $struct_name { pub name: String, pub value: Number, @@ -211,7 +211,7 @@ impl_cmp_condition!(LtCondition, <); impl_cmp_condition!(GtCondition, >); /// A condition that uses glob matching. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct GlobCondition { pub name: String, pub value: GlobPatterns, @@ -232,7 +232,7 @@ impl GlobCondition { /// Condition that cover custom operators which need /// special handling and have a custom implementation /// for each case. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct CustomCondition { pub name: String, #[serde(default)] @@ -254,7 +254,7 @@ impl CustomCondition { /// /// Creates a condition that is true when any /// of the inner conditions are true -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct OrCondition { inner: Vec, } @@ -276,7 +276,7 @@ impl OrCondition { /// /// Creates a condition that is true when all /// inner conditions are true. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct AndCondition { inner: Vec, } @@ -297,7 +297,7 @@ impl AndCondition { /// /// Creates a condition that is true when the wrapped /// condition si false. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct NotCondition { inner: Box, } @@ -316,7 +316,7 @@ impl NotCondition { } /// A condition from a sampling rule. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase", tag = "op")] pub enum RuleCondition { Eq(EqCondition),