Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metrics): Prefix all metric names [INGEST-548] #1147

Merged
merged 11 commits into from
Dec 14, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- 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: 32 additions & 26 deletions relay-server/src/metrics_extraction/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ 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 @@ -49,7 +55,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: "session".to_owned(),
name: metric_name("session"),
unit: MetricUnit::None,
value: MetricValue::Counter(session.total_count() as f64),
timestamp,
Expand All @@ -58,7 +64,7 @@ pub fn extract_session_metrics<T: SessionLike>(

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

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

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

if let Some(distinct_id) = nil_to_none(session.distinct_id()) {
target.push(Metric {
name: "user".to_owned(),
name: metric_name("user"),
unit: MetricUnit::None,
value: MetricValue::set_from_str(distinct_id),
timestamp,
Expand All @@ -144,7 +150,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: "session.duration".to_owned(),
name: metric_name("session.duration"),
unit: MetricUnit::Duration(DurationPrecision::Second),
value: MetricValue::Distribution(duration),
timestamp,
Expand Down Expand Up @@ -212,14 +218,14 @@ mod tests {

let session_metric = &metrics[0];
assert_eq!(session_metric.timestamp, started());
assert_eq!(session_metric.name, "session");
assert_eq!(session_metric.name, "sentry.sessions.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, "user");
assert_eq!(user_metric.name, "sentry.sessions.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 @@ -283,13 +289,13 @@ mod tests {

let session_metric = &metrics[expected_metrics - 2];
assert_eq!(session_metric.timestamp, started());
assert_eq!(session_metric.name, "session.error");
assert_eq!(session_metric.name, "sentry.sessions.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, "user");
assert_eq!(user_metric.name, "sentry.sessions.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 @@ -319,19 +325,19 @@ mod tests {

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

assert_eq!(metrics[0].name, "session.error");
assert_eq!(metrics[1].name, "user");
assert_eq!(metrics[0].name, "sentry.sessions.session.error");
assert_eq!(metrics[1].name, "sentry.sessions.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, "session");
assert_eq!(session_metric.name, "sentry.sessions.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, "user");
assert_eq!(user_metric.name, "sentry.sessions.user");
assert!(matches!(user_metric.value, MetricValue::Set(_)));
assert_eq!(user_metric.tags["session.status"], status.to_string());
}
Expand Down Expand Up @@ -361,7 +367,7 @@ mod tests {
assert_eq!(metrics.len(), 1);

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

use serde::{Deserialize, Serialize};
use std::collections::BTreeSet;

#[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,6 +18,21 @@ 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 @@ -79,15 +94,15 @@ pub fn extract_transaction_metrics(
}

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

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

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

push_metric(Metric {
name: format!("breakdown.{}.{}", breakdown, name),
name: metric_name(&["breakdowns", breakdown, name]),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this renames metrics from breakdown.foo to breakdowns.foo, to be consistent with measurements.

unit: MetricUnit::None,
value: MetricValue::Distribution(measurement),
timestamp,
Expand All @@ -140,10 +155,10 @@ fn get_measurement_rating(name: &str, value: f64) -> Option<String> {
};

match name {
"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),
"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),
_ => None,
}
}
Expand Down Expand Up @@ -198,11 +213,11 @@ mod tests {
r#"
{
"extractMetrics": [
"measurements.foo",
"measurements.lcp",
"breakdown.breakdown1.bar",
"breakdown.breakdown2.baz",
"breakdown.breakdown2.zap"
"sentry.transactions.measurements.foo",
"sentry.transactions.measurements.lcp",
"sentry.transactions.breakdowns.breakdown1.bar",
"sentry.transactions.breakdowns.breakdown2.baz",
"sentry.transactions.breakdowns.breakdown2.zap"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I briefly considered omitting the prefixes in the project config, since we're already in a config object called transactionMetrics. But I figured that listing the full metric name would simplify searching for it, as with snuba referrers.

],
"extractCustomTags": ["fOO"]
}
Expand All @@ -215,11 +230,20 @@ mod tests {

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

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[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[1].tags["measurement_rating"], "meh");

Expand Down
Loading