Skip to content

Commit

Permalink
Revert "feat(metrics): Prefix all metric names (#1147)"
Browse files Browse the repository at this point in the history 
This reverts commit 22afc3e.
  • Loading branch information
@jjbayer
jjbayer committed Dec 15, 2021
1 parent 22afc3e commit 03de9ca
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 97 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
- Fold processing vs non-processing into single actor. ([#1133](https://github.com/getsentry/relay/pull/1133))
- Aggregate outcomes for dynamic sampling, invalid project ID, and rate limits. ([#1134](https://github.com/getsentry/relay/pull/1134))
- Extract session metrics from aggregate sessions. ([#1140](https://github.com/getsentry/relay/pull/1140))
- Prefix names of extracted metrics by `sentry.sessions.` or `sentry.transactions.`. ([#1147](https://github.com/getsentry/relay/pull/1147))

## 21.11.0

Expand Down
58 changes: 26 additions & 32 deletions relay-server/src/metrics_extraction/sessions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ fn nil_to_none(distinct_id: Option<&String>) -> Option<&String> {
Some(distinct_id)
}

/// Generate a sessions-related metric name
/// Would be nice to have this as a `const fn`, but [`Metric`] requires a [`String`] anyway.
fn metric_name(name: &str) -> String {
format!("sentry.sessions.{}", name)
}

pub fn extract_session_metrics<T: SessionLike>(
attributes: &SessionAttributes,
session: &T,
Expand All @@ -55,7 +49,7 @@ pub fn extract_session_metrics<T: SessionLike>(
// for adoption and as baseline for crash rates.
if session.total_count() > 0 {
target.push(Metric {
name: metric_name("session"),
name: "session".to_owned(),
unit: MetricUnit::None,
value: MetricValue::Counter(session.total_count() as f64),
timestamp,
Expand All @@ -64,7 +58,7 @@ pub fn extract_session_metrics<T: SessionLike>(

if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
target.push(Metric {
name: metric_name("user"),
name: "user".to_owned(),
unit: MetricUnit::None,
value: MetricValue::set_from_str(distinct_id),
timestamp,
Expand All @@ -77,14 +71,14 @@ pub fn extract_session_metrics<T: SessionLike>(
if let Some(errors) = session.errors() {
target.push(match errors {
SessionErrored::Individual(session_id) => Metric {
name: metric_name("session.error"),
name: "session.error".to_owned(),
unit: MetricUnit::None,
value: MetricValue::set_from_display(session_id),
timestamp,
tags: tags.clone(),
},
SessionErrored::Aggregated(count) => Metric {
name: metric_name("session"),
name: "session".to_owned(),
unit: MetricUnit::None,
value: MetricValue::Counter(count as f64),
timestamp,
Expand All @@ -94,7 +88,7 @@ pub fn extract_session_metrics<T: SessionLike>(

if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
target.push(Metric {
name: metric_name("user"),
name: "user".to_owned(),
unit: MetricUnit::None,
value: MetricValue::set_from_str(distinct_id),
timestamp,
Expand All @@ -107,7 +101,7 @@ pub fn extract_session_metrics<T: SessionLike>(
// sessions above.
if session.abnormal_count() > 0 {
target.push(Metric {
name: metric_name("session"),
name: "session".to_owned(),
unit: MetricUnit::None,
value: MetricValue::Counter(session.abnormal_count() as f64),
timestamp,
Expand All @@ -116,7 +110,7 @@ pub fn extract_session_metrics<T: SessionLike>(

if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
target.push(Metric {
name: metric_name("user"),
name: "user".to_owned(),
unit: MetricUnit::None,
value: MetricValue::set_from_str(distinct_id),
timestamp,
Expand All @@ -126,7 +120,7 @@ pub fn extract_session_metrics<T: SessionLike>(
}
if session.crashed_count() > 0 {
target.push(Metric {
name: metric_name("session"),
name: "session".to_owned(),
unit: MetricUnit::None,
value: MetricValue::Counter(session.crashed_count() as f64),
timestamp,
Expand All @@ -135,7 +129,7 @@ pub fn extract_session_metrics<T: SessionLike>(

if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
target.push(Metric {
name: metric_name("user"),
name: "user".to_owned(),
unit: MetricUnit::None,
value: MetricValue::set_from_str(distinct_id),
timestamp,
Expand All @@ -150,7 +144,7 @@ pub fn extract_session_metrics<T: SessionLike>(
// if session.status.is_terminal() {
if let Some((duration, status)) = session.final_duration() {
target.push(Metric {
name: metric_name("session.duration"),
name: "session.duration".to_owned(),
unit: MetricUnit::Duration(DurationPrecision::Second),
value: MetricValue::Distribution(duration),
timestamp,
Expand Down Expand Up @@ -218,14 +212,14 @@ mod tests {

let session_metric = &metrics[0];
assert_eq!(session_metric.timestamp, started());
assert_eq!(session_metric.name, "sentry.sessions.session");
assert_eq!(session_metric.name, "session");
assert!(matches!(session_metric.value, MetricValue::Counter(_)));
assert_eq!(session_metric.tags["session.status"], "init");
assert_eq!(session_metric.tags["release"], "1.0.0");

let user_metric = &metrics[1];
assert_eq!(session_metric.timestamp, started());
assert_eq!(user_metric.name, "sentry.sessions.user");
assert_eq!(user_metric.name, "user");
assert!(matches!(user_metric.value, MetricValue::Set(_)));
assert_eq!(session_metric.tags["session.status"], "init");
assert_eq!(user_metric.tags["release"], "1.0.0");
Expand Down Expand Up @@ -289,13 +283,13 @@ mod tests {

let session_metric = &metrics[expected_metrics - 2];
assert_eq!(session_metric.timestamp, started());
assert_eq!(session_metric.name, "sentry.sessions.session.error");
assert_eq!(session_metric.name, "session.error");
assert!(matches!(session_metric.value, MetricValue::Set(_)));
assert_eq!(session_metric.tags.len(), 1); // Only the release tag

let user_metric = &metrics[expected_metrics - 1];
assert_eq!(session_metric.timestamp, started());
assert_eq!(user_metric.name, "sentry.sessions.user");
assert_eq!(user_metric.name, "user");
assert!(matches!(user_metric.value, MetricValue::Set(_)));
assert_eq!(user_metric.tags["session.status"], "errored");
assert_eq!(user_metric.tags["release"], "1.0.0");
Expand Down Expand Up @@ -325,19 +319,19 @@ mod tests {

assert_eq!(metrics.len(), 4);

assert_eq!(metrics[0].name, "sentry.sessions.session.error");
assert_eq!(metrics[1].name, "sentry.sessions.user");
assert_eq!(metrics[0].name, "session.error");
assert_eq!(metrics[1].name, "user");
assert_eq!(metrics[1].tags["session.status"], "errored");

let session_metric = &metrics[2];
assert_eq!(session_metric.timestamp, started());
assert_eq!(session_metric.name, "sentry.sessions.session");
assert_eq!(session_metric.name, "session");
assert!(matches!(session_metric.value, MetricValue::Counter(_)));
assert_eq!(session_metric.tags["session.status"], status.to_string());

let user_metric = &metrics[3];
assert_eq!(session_metric.timestamp, started());
assert_eq!(user_metric.name, "sentry.sessions.user");
assert_eq!(user_metric.name, "user");
assert!(matches!(user_metric.value, MetricValue::Set(_)));
assert_eq!(user_metric.tags["session.status"], status.to_string());
}
Expand Down Expand Up @@ -367,7 +361,7 @@ mod tests {
assert_eq!(metrics.len(), 1);

let duration_metric = &metrics[0];
assert_eq!(duration_metric.name, "sentry.sessions.session.duration");
assert_eq!(duration_metric.name, "session.duration");
assert!(matches!(
duration_metric.value,
MetricValue::Distribution(_)
Expand Down Expand Up @@ -410,7 +404,7 @@ mod tests {
insta::assert_debug_snapshot!(metrics, @r###"
[
Metric {
name: "sentry.sessions.session",
name: "session",
unit: None,
value: Counter(
135.0,
Expand All @@ -423,7 +417,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.session",
name: "session",
unit: None,
value: Counter(
5.0,
Expand All @@ -436,7 +430,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.session",
name: "session",
unit: None,
value: Counter(
7.0,
Expand All @@ -449,7 +443,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.session",
name: "session",
unit: None,
value: Counter(
15.0,
Expand All @@ -462,7 +456,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.user",
name: "user",
unit: None,
value: Set(
3097475539,
Expand All @@ -475,7 +469,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.session",
name: "session",
unit: None,
value: Counter(
3.0,
Expand All @@ -488,7 +482,7 @@ mod tests {
},
},
Metric {
name: "sentry.sessions.user",
name: "user",
unit: None,
value: Set(
3097475539,
Expand Down
64 changes: 20 additions & 44 deletions relay-server/src/metrics_extraction/transactions.rs
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use serde::{Deserialize, Serialize};
use std::collections::BTreeSet;

use serde::{Deserialize, Serialize};

#[cfg(feature = "processing")]
use {
relay_common::UnixTimestamp,
relay_general::protocol::{AsPair, Event, EventType},
relay_metrics::{Metric, MetricUnit, MetricValue},
std::collections::BTreeMap,
std::fmt::Write,
};

/// Configuration in relation to extracting metrics from transaction events.
Expand All @@ -18,21 +18,6 @@ pub struct TransactionMetricsConfig {
extract_custom_tags: BTreeSet<String>,
}

#[cfg(feature = "processing")]
const METRIC_NAME_PREFIX: &str = "sentry.transactions";

/// Generate a transaction-related metric name
#[cfg(feature = "processing")]
fn metric_name(parts: &[&str]) -> String {
let mut name = METRIC_NAME_PREFIX.to_owned();
for part in parts {
// Unwrapping here should be fine:
// https://github.com/rust-lang/rust/blob/1.57.0/library/alloc/src/string.rs#L2721-L2724
write!(name, ".{}", part).unwrap();
}
name
}

#[cfg(feature = "processing")]
pub fn extract_transaction_metrics(
config: &TransactionMetricsConfig,
Expand Down Expand Up @@ -94,15 +79,15 @@ pub fn extract_transaction_metrics(
}

if let Some(measurements) = event.measurements.value() {
for (measurement_name, annotated) in measurements.iter() {
for (name, annotated) in measurements.iter() {
let measurement = match annotated.value().and_then(|m| m.value.value()) {
Some(measurement) => *measurement,
None => continue,
};

let name = metric_name(&["measurements", measurement_name]);
let name = format!("measurements.{}", name);
let mut tags = tags.clone();
if let Some(rating) = get_measurement_rating(measurement_name, measurement) {
if let Some(rating) = get_measurement_rating(&name, measurement) {
tags.insert("measurement_rating".to_owned(), rating);
}

Expand Down Expand Up @@ -130,7 +115,7 @@ pub fn extract_transaction_metrics(
};

push_metric(Metric {
name: metric_name(&["breakdowns", breakdown, name]),
name: format!("breakdown.{}.{}", breakdown, name),
unit: MetricUnit::None,
value: MetricValue::Distribution(measurement),
timestamp,
Expand All @@ -155,10 +140,10 @@ fn get_measurement_rating(name: &str, value: f64) -> Option<String> {
};

match name {
"lcp" => rate_range(2500.0, 4000.0),
"fcp" => rate_range(1000.0, 3000.0),
"fid" => rate_range(100.0, 300.0),
"cls" => rate_range(0.1, 0.25),
"measurements.lcp" => rate_range(2500.0, 4000.0),
"measurements.fcp" => rate_range(1000.0, 3000.0),
"measurements.fid" => rate_range(100.0, 300.0),
"measurements.cls" => rate_range(0.1, 0.25),
_ => None,
}
}
Expand Down Expand Up @@ -213,11 +198,11 @@ mod tests {
r#"
{
"extractMetrics": [
"sentry.transactions.measurements.foo",
"sentry.transactions.measurements.lcp",
"sentry.transactions.breakdowns.breakdown1.bar",
"sentry.transactions.breakdowns.breakdown2.baz",
"sentry.transactions.breakdowns.breakdown2.zap"
"measurements.foo",
"measurements.lcp",
"breakdown.breakdown1.bar",
"breakdown.breakdown2.baz",
"breakdown.breakdown2.zap"
],
"extractCustomTags": ["fOO"]
}
Expand All @@ -230,20 +215,11 @@ mod tests {

assert_eq!(metrics.len(), 5);

assert_eq!(metrics[0].name, "sentry.transactions.measurements.foo");
assert_eq!(metrics[1].name, "sentry.transactions.measurements.lcp");
assert_eq!(
metrics[2].name,
"sentry.transactions.breakdowns.breakdown1.bar"
);
assert_eq!(
metrics[3].name,
"sentry.transactions.breakdowns.breakdown2.baz"
);
assert_eq!(
metrics[4].name,
"sentry.transactions.breakdowns.breakdown2.zap"
);
assert_eq!(metrics[0].name, "measurements.foo");
assert_eq!(metrics[1].name, "measurements.lcp");
assert_eq!(metrics[2].name, "breakdown.breakdown1.bar");
assert_eq!(metrics[3].name, "breakdown.breakdown2.baz");
assert_eq!(metrics[4].name, "breakdown.breakdown2.zap");

assert_eq!(metrics[1].tags["measurement_rating"], "meh");

Expand Down
Loading

0 comments on commit 03de9ca

Please sign in to comment.