Skip to content

Commit

Permalink
feat(spans): Add separate feature flags for add-ons span metrics and …
Browse files Browse the repository at this point in the history
…indexed spans (#3633)
  • Loading branch information
phacops authored May 29, 2024
1 parent 8b7612f commit 265366c
Show file tree
Hide file tree
Showing 18 changed files with 14,425 additions and 295 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
**Features**:

- Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552))
- Add separate feature flags for add-ons span metrics and indexed spans. ([#3633](https://github.com/getsentry/relay/pull/3633))

**Internal**:

Expand Down
4 changes: 2 additions & 2 deletions relay-common/src/glob2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ pub struct LazyGlob {

impl LazyGlob {
/// Create a new [`LazyGlob`] from the raw string.
pub fn new(raw: String) -> Self {
pub fn new(raw: impl Into<String>) -> Self {
Self {
raw,
raw: raw.into(),
glob: OnceLock::new(),
}
}
Expand Down
543 changes: 321 additions & 222 deletions relay-dynamic-config/src/defaults.rs

Large diffs are not rendered by default.

25 changes: 19 additions & 6 deletions relay-dynamic-config/src/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ pub enum Feature {
/// Serialized as `organizations:device-class-synthesis`.
#[serde(rename = "organizations:device-class-synthesis")]
DeviceClassSynthesis,
/// Enables metric extraction from spans.
///
/// Serialized as `projects:span-metrics-extraction`.
#[serde(rename = "projects:span-metrics-extraction")]
ExtractSpansAndSpanMetricsFromEvent,
/// Allow ingestion of metrics in the "custom" namespace.
///
/// Serialized as `organizations:custom-metrics`.
Expand Down Expand Up @@ -90,6 +85,24 @@ pub enum Feature {
#[serde(rename = "projects:extract-transaction-from-segment-span")]
ExtractTransactionFromSegmentSpan,

/// Enables metric extraction from spans for common modules.
///
/// Serialized as `projects:span-metrics-extraction`.
#[serde(rename = "projects:span-metrics-extraction")]
ExtractCommonSpanMetricsFromEvent,

/// Enables metric extraction from spans for addon modules.
///
/// Serialized as `projects:span-metrics-extraction-addons`.
#[serde(rename = "projects:span-metrics-extraction-addons")]
ExtractAddonsSpanMetricsFromEvent,

/// When enabled, spans will be extracted from a transaction.
///
/// Serialized as `projects:indexed-spans-extraction`.
#[serde(rename = "organizations:indexed-spans-extraction")]
ExtractSpansFromEvent,

/// Deprecated, still forwarded for older downstream Relays.
#[doc(hidden)]
#[serde(rename = "organizations:transaction-name-mark-scrubbed-as-sanitized")]
Expand All @@ -109,7 +122,7 @@ pub enum Feature {
/// Deprecated, still forwarded for older downstream Relays.
#[doc(hidden)]
#[serde(rename = "projects:span-metrics-extraction-all-modules")]
Deprected6,
Deprecated6,
/// Forward compatibility.
#[doc(hidden)]
#[serde(other)]
Expand Down
6 changes: 3 additions & 3 deletions relay-dynamic-config/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ impl GlobalConfig {
/// - Adds hard-coded groups to metrics extraction configs.
pub fn normalize(&mut self) {
if let ErrorBoundary::Ok(config) = &mut self.metric_extraction {
for (group_name, metrics) in defaults::hardcoded_span_metrics() {
for (group_name, metrics, tags) in defaults::hardcoded_span_metrics() {
// We only define these groups if they haven't been defined by the upstream yet.
// This ensures that the innermost Relay always defines the metrics.
if let Entry::Vacant(entry) = config.groups.entry(group_name) {
entry.insert(MetricExtractionGroup {
is_enabled: false, // must be enabled via project config
metrics,
tags: Default::default(),
tags,
});
}
}
Expand Down Expand Up @@ -201,7 +201,7 @@ pub struct Options {
/// Overall sampling of span extraction.
///
/// This number represents the fraction of transactions for which
/// spans are extracted. It applies on top of [`crate::Feature::ExtractSpansAndSpanMetricsFromEvent`],
/// spans are extracted. It applies on top of [`crate::Feature::ExtractCommonSpanMetricsFromEvent`],
/// so both feature flag and sample rate need to be enabled to get any spans extracted.
///
/// `None` is the default and interpreted as a value of 1.0 (extract everything).
Expand Down
55 changes: 51 additions & 4 deletions relay-dynamic-config/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
//! Dynamic configuration for metrics extraction from sessions and transactions.
use core::fmt;
use std::collections::{BTreeMap, BTreeSet};
use std::convert::Infallible;
use std::str::FromStr;

use relay_base_schema::data_category::DataCategory;
use relay_cardinality::CardinalityLimit;
use relay_common::glob2::LazyGlob;
use relay_common::glob3::GlobPatterns;
use relay_common::impl_str_serde;
use relay_protocol::RuleCondition;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -173,7 +177,7 @@ impl TransactionMetricsConfig {
}

/// Combined view of global and project-specific metrics extraction configs.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub struct CombinedMetricExtractionConfig<'a> {
global: &'a MetricExtractionGroups,
project: &'a MetricExtractionConfig,
Expand All @@ -185,7 +189,7 @@ impl<'a> CombinedMetricExtractionConfig<'a> {
for key in project.global_groups.keys() {
if !global.groups.contains_key(key) {
relay_log::error!(
"Metrics group configured for project missing in global config: {key}"
"Metrics group configured for project missing in global config: {key:?}"
)
}
}
Expand Down Expand Up @@ -238,7 +242,7 @@ impl<'a> From<&'a MetricExtractionConfig> for CombinedMetricExtractionConfig<'a>
pub struct MetricExtractionGroups {
/// Mapping from group name to metrics specs & tags.
#[serde(skip_serializing_if = "BTreeMap::is_empty")]
pub groups: BTreeMap<String, MetricExtractionGroup>,
pub groups: BTreeMap<GroupKey, MetricExtractionGroup>,
}

impl MetricExtractionGroups {
Expand Down Expand Up @@ -286,7 +290,7 @@ pub struct MetricExtractionConfig {
/// The groups themselves are configured in [`crate::GlobalConfig`],
/// but can be enabled or disabled here.
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
pub global_groups: BTreeMap<String, MetricExtractionGroupOverride>,
pub global_groups: BTreeMap<GroupKey, MetricExtractionGroupOverride>,

/// A list of metric specifications to extract.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
Expand Down Expand Up @@ -364,6 +368,49 @@ pub struct MetricExtractionGroupOverride {
pub is_enabled: bool,
}

/// Enumeration of keys in [`MetricExtractionGroups`]. In JSON, this is simply a string.
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum GroupKey {
/// Metric extracted for all plans.
SpanMetricsCommon,
/// "addon" metrics.
SpanMetricsAddons,
/// Metrics extracted from spans in the transaction namespace.
SpanMetricsTx,
/// Any other group defined by the upstream.
Other(String),
}

impl fmt::Display for GroupKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}",
match self {
GroupKey::SpanMetricsCommon => "span_metrics_common",
GroupKey::SpanMetricsAddons => "span_metrics_addons",
GroupKey::SpanMetricsTx => "span_metrics_tx",
GroupKey::Other(s) => &s,
}
)
}
}

impl FromStr for GroupKey {
type Err = Infallible;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s {
"span_metrics_common" => GroupKey::SpanMetricsCommon,
"span_metrics_addons" => GroupKey::SpanMetricsAddons,
"span_metrics_tx" => GroupKey::SpanMetricsTx,
s => GroupKey::Other(s.to_owned()),
})
}
}

impl_str_serde!(GroupKey, "a metrics extraction group key");

/// Specification for a metric to extract from some data.
#[derive(Clone, Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
Expand Down
6 changes: 3 additions & 3 deletions relay-event-normalization/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2152,17 +2152,17 @@ mod tests {
version: 1,
costs: vec![
ModelCost {
model_id: LazyGlob::new("claude-2*".into()),
model_id: LazyGlob::new("claude-2*"),
for_completion: false,
cost_per_1k_tokens: 1.0,
},
ModelCost {
model_id: LazyGlob::new("gpt4-21*".into()),
model_id: LazyGlob::new("gpt4-21*"),
for_completion: false,
cost_per_1k_tokens: 2.0,
},
ModelCost {
model_id: LazyGlob::new("gpt4-21*".into()),
model_id: LazyGlob::new("gpt4-21*"),
for_completion: true,
cost_per_1k_tokens: 20.0,
},
Expand Down
Loading

0 comments on commit 265366c

Please sign in to comment.