Skip to content

Commit

Permalink
fix(outcomes): Do not report metrics-based outcomes for profiles (#4365)
Browse files Browse the repository at this point in the history
Profiles are no longer dropped by dynamic sampling, so we do not need to
track the virtual "total profile" category with a tag on metrics.

This also prevents double-reporting (the rate limiter on the payload
also emits an outcome for the `profile` category).
  • Loading branch information
jjbayer authored Dec 12, 2024
1 parent 21aad1e commit e1524fc
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 290 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Accept incoming requests even if there was an error fetching their project config. ([#4140](https://github.com/getsentry/relay/pull/4140))
- Rate limit profiles when transaction was sampled. ([#4195](https://github.com/getsentry/relay/pull/4195))
- Fix scrubbing user paths in minidump debug module names. ([#4351](https://github.com/getsentry/relay/pull/4351))
- Stop collecting the `has_profile` metrics tag & reporting outcomes for it. ([#4365](https://github.com/getsentry/relay/pull/4365))
- Scrub user fields in span.sentry_tags. ([#4364](https://github.com/getsentry/relay/pull/4364)), ([#4370](https://github.com/getsentry/relay/pull/4370))

**Features**:
Expand Down
26 changes: 7 additions & 19 deletions relay-server/src/metrics/minimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ pub struct MinimalTrackableBucket {
#[serde(flatten)]
value: MinimalValue,
#[serde(default)]
tags: Tags,
#[serde(default)]
metadata: BucketMetadata,
}

Expand All @@ -42,9 +40,7 @@ impl TrackableBucket for MinimalTrackableBucket {
MinimalValue::Counter(c) if mri.name == "usage" => c.to_f64() as usize,
_ => 0,
};
let has_profile = matches!(mri.name.as_ref(), "usage" | "duration")
&& self.tags.has_profile.is_some();
BucketSummary::Transactions { count, has_profile }
BucketSummary::Transactions(count)
}
MetricNamespace::Spans => BucketSummary::Spans(match self.value {
MinimalValue::Counter(c) if mri.name == "usage" => c.to_f64() as usize,
Expand Down Expand Up @@ -83,12 +79,6 @@ impl MinimalValue {
}
}

#[derive(Clone, Copy, Debug, Default, Deserialize)]
#[serde(default)]
struct Tags {
has_profile: Option<IgnoredAny>,
}

#[cfg(test)]
mod tests {
use insta::assert_debug_snapshot;
Expand Down Expand Up @@ -172,14 +162,12 @@ mod tests {
let summary = min_buckets.iter().map(|b| b.summary()).collect::<Vec<_>>();
assert_debug_snapshot!(summary, @r###"
[
Transactions {
count: 0,
has_profile: true,
},
Transactions {
count: 3,
has_profile: false,
},
Transactions(
0,
),
Transactions(
3,
),
Spans(
3,
),
Expand Down
16 changes: 3 additions & 13 deletions relay-server/src/metrics/outcomes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ use crate::services::outcome::{Outcome, TrackOutcome};
#[cfg(feature = "processing")]
use relay_cardinality::{CardinalityLimit, CardinalityReport};

pub const PROFILE_TAG: &str = "has_profile";

/// [`MetricOutcomes`] takes care of creating the right outcomes for metrics at the end of their
/// lifecycle.
///
Expand Down Expand Up @@ -99,10 +97,7 @@ impl MetricOutcomes {
/// Contains the count of total transactions or spans that went into this bucket.
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum BucketSummary {
Transactions {
count: usize,
has_profile: bool,
},
Transactions(usize),
Spans(usize),
#[default]
None,
Expand Down Expand Up @@ -185,9 +180,7 @@ impl TrackableBucket for BucketView<'_> {
BucketViewValue::Counter(c) if mri.name == "usage" => c.to_f64() as usize,
_ => 0,
};
let has_profile = matches!(mri.name.as_ref(), "usage" | "duration")
&& self.tag(PROFILE_TAG) == Some("true");
BucketSummary::Transactions { count, has_profile }
BucketSummary::Transactions(count)
}
MetricNamespace::Spans => BucketSummary::Spans(match self.value() {
BucketViewValue::Counter(c) if mri.name == "usage" => c.to_f64() as usize,
Expand Down Expand Up @@ -217,11 +210,8 @@ where
quantities.buckets += 1;
let summary = bucket.summary();
match summary {
BucketSummary::Transactions { count, has_profile } => {
BucketSummary::Transactions(count) => {
quantities.transactions += count;
if has_profile {
quantities.profiles += count;
}
}
BucketSummary::Spans(count) => quantities.spans += count,
BucketSummary::None => continue,
Expand Down
Loading

0 comments on commit e1524fc

Please sign in to comment.