From bff235eecd595c9daad1c8d8e0dca14fd51c3aa4 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Wed, 4 May 2022 09:38:12 +0200 Subject: [PATCH] feat(metrics): Allow ingestion of custom metric units (#1256) Introduces `MetricUnit` in `measurements[].unit` on the event protocol. This allows SDKs to submit units for custom measurements, as well as override units on well-known measurements defined in the past. Metric extraction uses these units when supplied, falling back to the predefined units in their absence. --- CHANGELOG.md | 3 +- py/CHANGELOG.md | 4 + relay-common/src/constants.rs | 290 ++++++++++++++++++ relay-general/src/protocol/measurements.rs | 64 +++- .../src/store/normalize/breakdowns.rs | 10 + relay-metrics/src/aggregation.rs | 2 +- relay-metrics/src/protocol.rs | 278 +---------------- .../src/metrics_extraction/transactions.rs | 130 ++++++-- tests/integration/test_envelope.py | 12 +- tests/integration/test_metrics.py | 22 +- 10 files changed, 489 insertions(+), 326 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 112d1cf80d..4851dbac78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add platform, op, http.method and status tag to all extracted transaction metrics. ([#1227](https://github.com/getsentry/relay/pull/1227)) - Add units in built-in measurements. ([#1229](https://github.com/getsentry/relay/pull/1229)) +- Add protocol support for custom units on transaction measurements. ([#1256](https://github.com/getsentry/relay/pull/1256)) **Bug Fixes**: @@ -20,7 +21,7 @@ - Remove/reject nul-bytes from metric strings. ([#1235](https://github.com/getsentry/relay/pull/1235)) - Remove the unused "internal" data category. ([#1245](https://github.com/getsentry/relay/pull/1245)) - Add the client and version as `sdk` tag to extracted session metrics in the format `name/version`. ([#1248](https://github.com/getsentry/relay/pull/1248)) -- - Expose `shutdown_timeout` in `OverridableConfig` ([#1247](https://github.com/getsentry/relay/pull/1247)) +- Expose `shutdown_timeout` in `OverridableConfig` ([#1247](https://github.com/getsentry/relay/pull/1247)) ## 22.4.0 diff --git a/py/CHANGELOG.md b/py/CHANGELOG.md index 851881e029..c2f2b4c044 100644 --- a/py/CHANGELOG.md +++ b/py/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Add protocol support for custom units on transaction measurements. ([#1256](https://github.com/getsentry/relay/pull/1256)) + ## 0.8.10 - Map Windows version from raw_description to version name (XP, Vista, 11, ...). ([#1219](https://github.com/getsentry/relay/pull/1219)) diff --git a/relay-common/src/constants.rs b/relay-common/src/constants.rs index de4a0d66fa..d0d5402c2b 100644 --- a/relay-common/src/constants.rs +++ b/relay-common/src/constants.rs @@ -330,3 +330,293 @@ impl fmt::Display for SpanStatus { } } } + +/// Time duration units used in [`MetricUnit::Duration`]. +/// +/// Defaults to `millisecond`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum DurationUnit { + /// Nanosecond (`"nanosecond"`), 10^-9 seconds. + NanoSecond, + /// Microsecond (`"microsecond"`), 10^-6 seconds. + MicroSecond, + /// Millisecond (`"millisecond"`), 10^-3 seconds. + MilliSecond, + /// Full second (`"second"`). + Second, + /// Minute (`"minute"`), 60 seconds. + Minute, + /// Hour (`"hour"`), 3600 seconds. + Hour, + /// Day (`"day"`), 86,400 seconds. + Day, + /// Week (`"week"`), 604,800 seconds. + Week, +} + +impl Default for DurationUnit { + fn default() -> Self { + Self::MilliSecond + } +} + +impl fmt::Display for DurationUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NanoSecond => f.write_str("nanosecond"), + Self::MicroSecond => f.write_str("microsecond"), + Self::MilliSecond => f.write_str("millisecond"), + Self::Second => f.write_str("second"), + Self::Minute => f.write_str("minute"), + Self::Hour => f.write_str("hour"), + Self::Day => f.write_str("day"), + Self::Week => f.write_str("week"), + } + } +} + +/// An error parsing a [`MetricUnit`] or one of its variants. +#[derive(Clone, Copy, Debug)] +pub struct ParseMetricUnitError(()); + +/// Size of information derived from bytes, used in [`MetricUnit::Information`]. +/// +/// Defaults to `byte`. See also [Units of +/// information](https://en.wikipedia.org/wiki/Units_of_information). +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum InformationUnit { + /// Bit (`"bit"`), corresponding to 1/8 of a byte. + /// + /// Note that there are computer systems with a different number of bits per byte. + Bit, + /// Byte (`"byte"`). + Byte, + /// Kilobyte (`"kilobyte"`), 10^3 bytes. + KiloByte, + /// Kibibyte (`"kibibyte"`), 2^10 bytes. + KibiByte, + /// Megabyte (`"megabyte"`), 10^6 bytes. + MegaByte, + /// Mebibyte (`"mebibyte"`), 2^20 bytes. + MebiByte, + /// Gigabyte (`"gigabyte"`), 10^9 bytes. + GigaByte, + /// Gibibyte (`"gibibyte"`), 2^30 bytes. + GibiByte, + /// Terabyte (`"terabyte"`), 10^12 bytes. + TeraByte, + /// Tebibyte (`"tebibyte"`), 2^40 bytes. + TebiByte, + /// Petabyte (`"petabyte"`), 10^15 bytes. + PetaByte, + /// Pebibyte (`"pebibyte"`), 2^50 bytes. + PebiByte, + /// Exabyte (`"exabyte"`), 10^18 bytes. + ExaByte, + /// Exbibyte (`"exbibyte"`), 2^60 bytes. + ExbiByte, +} + +impl Default for InformationUnit { + fn default() -> Self { + Self::Byte + } +} + +impl fmt::Display for InformationUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Bit => f.write_str("bit"), + Self::Byte => f.write_str("byte"), + Self::KiloByte => f.write_str("kilobyte"), + Self::KibiByte => f.write_str("kibibyte"), + Self::MegaByte => f.write_str("megabyte"), + Self::MebiByte => f.write_str("mebibyte"), + Self::GigaByte => f.write_str("gigabyte"), + Self::GibiByte => f.write_str("gibibyte"), + Self::TeraByte => f.write_str("terabyte"), + Self::TebiByte => f.write_str("tebibyte"), + Self::PetaByte => f.write_str("petabyte"), + Self::PebiByte => f.write_str("pebibyte"), + Self::ExaByte => f.write_str("exabyte"), + Self::ExbiByte => f.write_str("exbibyte"), + } + } +} + +/// Units of fraction used in [`MetricUnit::Fraction`]. +/// +/// Defaults to `ratio`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum FractionUnit { + /// Floating point fraction of `1`. + Ratio, + /// Ratio expressed as a fraction of `100`. `100%` equals a ratio of `1.0`. + Percent, +} + +impl Default for FractionUnit { + fn default() -> Self { + Self::Ratio + } +} + +impl fmt::Display for FractionUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Ratio => f.write_str("ratio"), + Self::Percent => f.write_str("percent"), + } + } +} + +/// Custom user-defined units without builtin conversion. +#[derive(Clone, Copy, Eq, PartialEq, Hash)] +pub struct CustomUnit([u8; 15]); + +impl CustomUnit { + /// Parses a `CustomUnit` from a string. + pub fn parse(s: &str) -> Result { + if s.len() > 15 || !s.is_ascii() { + return Err(ParseMetricUnitError(())); + } + + let mut unit = Self(Default::default()); + unit.0.copy_from_slice(s.as_bytes()); + unit.0.make_ascii_lowercase(); + Ok(unit) + } + + /// Returns the string representation of this unit. + #[inline] + pub fn as_str(&self) -> &str { + // Safety: The string is already validated to be of length 32 and valid ASCII when + // constructing `ProjectKey`. + unsafe { std::str::from_utf8_unchecked(&self.0) } + } +} + +impl fmt::Debug for CustomUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +impl fmt::Display for CustomUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +impl std::str::FromStr for CustomUnit { + type Err = ParseMetricUnitError; + + fn from_str(s: &str) -> Result { + Self::parse(s) + } +} + +impl std::ops::Deref for CustomUnit { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.as_str() + } +} + +/// The unit of measurement of a metric value. +/// +/// Units augment metric values by giving them a magnitude and semantics. There are certain types of +/// units that are subdivided in their precision, such as the [`DurationUnit`] for time +/// measurements. +/// +/// Units and their precisions are uniquely represented by a string identifier. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum MetricUnit { + /// A time duration, defaulting to `"millisecond"`. + Duration(DurationUnit), + /// Size of information derived from bytes, defaulting to `"byte"`. + Information(InformationUnit), + /// Fractions such as percentages, defaulting to `"ratio"`. + Fraction(FractionUnit), + /// user-defined units without builtin conversion or default. + Custom(CustomUnit), + /// Untyped value without a unit (`""`). + None, +} + +impl MetricUnit { + /// Returns `true` if the metric_unit is [`None`]. + pub fn is_none(&self) -> bool { + matches!(self, Self::None) + } +} + +impl Default for MetricUnit { + fn default() -> Self { + MetricUnit::None + } +} + +impl fmt::Display for MetricUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MetricUnit::Duration(u) => u.fmt(f), + MetricUnit::Information(u) => u.fmt(f), + MetricUnit::Fraction(u) => u.fmt(f), + MetricUnit::Custom(u) => u.fmt(f), + MetricUnit::None => f.write_str("none"), + } + } +} + +impl std::str::FromStr for MetricUnit { + type Err = ParseMetricUnitError; + + fn from_str(s: &str) -> Result { + Ok(match s { + "nanosecond" | "ns" => Self::Duration(DurationUnit::NanoSecond), + "microsecond" => Self::Duration(DurationUnit::MicroSecond), + "millisecond" | "ms" => Self::Duration(DurationUnit::MilliSecond), + "second" | "s" => Self::Duration(DurationUnit::Second), + "minute" => Self::Duration(DurationUnit::Minute), + "hour" => Self::Duration(DurationUnit::Hour), + "day" => Self::Duration(DurationUnit::Day), + "week" => Self::Duration(DurationUnit::Week), + + "bit" => Self::Information(InformationUnit::Bit), + "byte" => Self::Information(InformationUnit::Byte), + "kilobyte" => Self::Information(InformationUnit::KiloByte), + "kibibyte" => Self::Information(InformationUnit::KibiByte), + "megabyte" => Self::Information(InformationUnit::MegaByte), + "mebibyte" => Self::Information(InformationUnit::MebiByte), + "gigabyte" => Self::Information(InformationUnit::GigaByte), + "gibibyte" => Self::Information(InformationUnit::GibiByte), + "terabyte" => Self::Information(InformationUnit::TeraByte), + "tebibyte" => Self::Information(InformationUnit::TebiByte), + "petabyte" => Self::Information(InformationUnit::PetaByte), + "pebibyte" => Self::Information(InformationUnit::PebiByte), + "exabyte" => Self::Information(InformationUnit::ExaByte), + "exbibyte" => Self::Information(InformationUnit::ExbiByte), + + "ratio" => Self::Fraction(FractionUnit::Ratio), + "percent" => Self::Fraction(FractionUnit::Percent), + + "" | "none" => Self::None, + _ => Self::Custom(CustomUnit::parse(s)?), + }) + } +} + +impl_str_serde!(MetricUnit, "a metric unit string"); + +#[cfg(feature = "jsonschema")] +impl schemars::JsonSchema for MetricUnit { + fn schema_name() -> String { + std::any::type_name::().to_owned() + } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + String::json_schema(gen) + } +} diff --git a/relay-general/src/protocol/measurements.rs b/relay-general/src/protocol/measurements.rs index 0aee112341..2072155f69 100644 --- a/relay-general/src/protocol/measurements.rs +++ b/relay-general/src/protocol/measurements.rs @@ -1,6 +1,54 @@ use std::ops::{Deref, DerefMut}; -use crate::types::{Annotated, Error, FromValue, Object, Value}; +use relay_common::MetricUnit; + +use crate::processor::ProcessValue; +use crate::types::{ + Annotated, Empty, Error, ErrorKind, FromValue, IntoValue, Object, SkipSerialization, Value, +}; + +impl Empty for MetricUnit { + #[inline] + fn is_empty(&self) -> bool { + // MetricUnit is never empty, even None carries significance over a missing unit. + false + } +} + +impl FromValue for MetricUnit { + fn from_value(value: Annotated) -> Annotated { + match String::from_value(value) { + Annotated(Some(value), mut meta) => match value.parse() { + Ok(unit) => Annotated(Some(unit), meta), + Err(_) => { + meta.add_error(ErrorKind::InvalidData); + meta.set_original_value(Some(value)); + Annotated(None, meta) + } + }, + Annotated(None, meta) => Annotated(None, meta), + } + } +} + +impl IntoValue for MetricUnit { + fn into_value(self) -> Value + where + Self: Sized, + { + Value::String(format!("{}", self)) + } + + fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result + where + Self: Sized, + S: serde::Serializer, + { + serde::Serialize::serialize(&self.to_string(), s) + } +} + +impl ProcessValue for MetricUnit {} /// An individual observed measurement. #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] @@ -8,6 +56,9 @@ pub struct Measurement { /// Value of observed measurement value. #[metastructure(required = "true", skip_serialization = "never")] pub value: Annotated, + + /// The unit of this measurement, defaulting to no unit. + pub unit: Annotated, } /// A map of observed measurement values. @@ -86,10 +137,11 @@ fn is_valid_measurement_name(name: &str) -> bool { #[test] fn test_measurements_serialization() { use crate::protocol::Event; + use relay_common::DurationUnit; let input = r#"{ "measurements": { - "LCP": {"value": 420.69}, + "LCP": {"value": 420.69, "unit": "millisecond"}, " lcp_final.element-Size123 ": {"value": 1}, "fid": {"value": 2020}, "cls": {"value": null}, @@ -112,7 +164,8 @@ fn test_measurements_serialization() { "value": null }, "lcp": { - "value": 420.69 + "value": 420.69, + "unit": "millisecond" }, "lcp_final.element-size123": { "value": 1.0 @@ -175,24 +228,28 @@ fn test_measurements_serialization() { "cls".to_owned(), Annotated::new(Measurement { value: Annotated::empty(), + unit: Annotated::empty(), }), ); measurements.insert( "lcp".to_owned(), Annotated::new(Measurement { value: Annotated::new(420.69), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }), ); measurements.insert( "lcp_final.element-size123".to_owned(), Annotated::new(Measurement { value: Annotated::new(1f64), + unit: Annotated::empty(), }), ); measurements.insert( "fid".to_owned(), Annotated::new(Measurement { value: Annotated::new(2020f64), + unit: Annotated::empty(), }), ); measurements.insert( @@ -202,6 +259,7 @@ fn test_measurements_serialization() { Error::expected("a floating point number"), Some("im a first paint".into()), ), + unit: Annotated::empty(), }), ); diff --git a/relay-general/src/store/normalize/breakdowns.rs b/relay-general/src/store/normalize/breakdowns.rs index bc2329b5fa..ad608b1acb 100644 --- a/relay-general/src/store/normalize/breakdowns.rs +++ b/relay-general/src/store/normalize/breakdowns.rs @@ -7,6 +7,8 @@ use std::ops::Deref; use serde::{Deserialize, Serialize}; +use relay_common::{DurationUnit, MetricUnit}; + use crate::protocol::{Breakdowns, Event, Measurement, Measurements, Timestamp}; use crate::types::Annotated; @@ -162,6 +164,8 @@ impl EmitBreakdowns for SpanOperationsConfig { let mut total_time_spent = 0.0; for (operation_name, intervals) in intervals { + // TODO(ja): Convert measurements in here, use `Duration` as typed carrier. + // use `get_metric_measurement_unit` from metric extraction for this (move!) let op_time_spent = match get_op_time_spent(intervals) { None => continue, Some(op_time_spent) => op_time_spent, @@ -176,6 +180,7 @@ impl EmitBreakdowns for SpanOperationsConfig { let time_spent_measurement = Measurement { value: Annotated::new(op_time_spent), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }; let op_breakdown_name = format!("ops.{}", operation_name); @@ -185,6 +190,7 @@ impl EmitBreakdowns for SpanOperationsConfig { let total_time_spent_measurement = Measurement { value: Annotated::new(total_time_spent), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }; breakdown.insert( "total.time".to_string(), @@ -298,6 +304,7 @@ mod tests { "lcp".to_owned(), Annotated::new(Measurement { value: Annotated::new(420.69), + unit: Annotated::empty(), }), ); @@ -393,6 +400,7 @@ mod tests { Annotated::new(Measurement { // 1 hour in milliseconds value: Annotated::new(3_600_000.0), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }), ); @@ -401,6 +409,7 @@ mod tests { Annotated::new(Measurement { // 2 hours in milliseconds value: Annotated::new(7_200_000.0), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }), ); @@ -409,6 +418,7 @@ mod tests { Annotated::new(Measurement { // 4 hours and 10 microseconds in milliseconds value: Annotated::new(14_400_000.01), + unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)), }), ); diff --git a/relay-metrics/src/aggregation.rs b/relay-metrics/src/aggregation.rs index 8d1fbe8912..2e1e6f67f8 100644 --- a/relay-metrics/src/aggregation.rs +++ b/relay-metrics/src/aggregation.rs @@ -1509,7 +1509,7 @@ mod tests { use super::*; - use crate::{DurationUnit, MetricUnit}; + use relay_common::{DurationUnit, MetricUnit}; struct BucketCountInquiry; diff --git a/relay-metrics/src/protocol.rs b/relay-metrics/src/protocol.rs index 8aa0a0c268..8257a77f5e 100644 --- a/relay-metrics/src/protocol.rs +++ b/relay-metrics/src/protocol.rs @@ -7,280 +7,10 @@ use serde::{Deserialize, Serialize}; pub use relay_common::UnixTimestamp; -/// Time duration units used in [`MetricUnit::Duration`]. -/// -/// Defaults to `millisecond`. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum DurationUnit { - /// Nanosecond (`"nanosecond"`), 10^-9 seconds. - NanoSecond, - /// Microsecond (`"microsecond"`), 10^-6 seconds. - MicroSecond, - /// Millisecond (`"millisecond"`), 10^-3 seconds. - MilliSecond, - /// Full second (`"second"`). - Second, - /// Minute (`"minute"`), 60 seconds. - Minute, - /// Hour (`"hour"`), 3600 seconds. - Hour, - /// Day (`"day"`), 86,400 seconds. - Day, - /// Week (`"week"`), 604,800 seconds. - Week, -} - -impl Default for DurationUnit { - fn default() -> Self { - Self::MilliSecond - } -} - -impl fmt::Display for DurationUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::NanoSecond => f.write_str("nanosecond"), - Self::MicroSecond => f.write_str("microsecond"), - Self::MilliSecond => f.write_str("millisecond"), - Self::Second => f.write_str("second"), - Self::Minute => f.write_str("minute"), - Self::Hour => f.write_str("hour"), - Self::Day => f.write_str("day"), - Self::Week => f.write_str("week"), - } - } -} - -/// Size of information derived from bytes, used in [`MetricUnit::Information`]. -/// -/// Defaults to `byte`. See also [Units of -/// information](https://en.wikipedia.org/wiki/Units_of_information). -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum InformationUnit { - /// Bit (`"bit"`), corresponding to 1/8 of a byte. - /// - /// Note that there are computer systems with a different number of bits per byte. - Bit, - /// Byte (`"byte"`). - Byte, - /// Kilobyte (`"kilobyte"`), 10^3 bytes. - KiloByte, - /// Kibibyte (`"kibibyte"`), 2^10 bytes. - KibiByte, - /// Megabyte (`"megabyte"`), 10^6 bytes. - MegaByte, - /// Mebibyte (`"mebibyte"`), 2^20 bytes. - MebiByte, - /// Gigabyte (`"gigabyte"`), 10^9 bytes. - GigaByte, - /// Gibibyte (`"gibibyte"`), 2^30 bytes. - GibiByte, - /// Terabyte (`"terabyte"`), 10^12 bytes. - TeraByte, - /// Tebibyte (`"tebibyte"`), 2^40 bytes. - TebiByte, - /// Petabyte (`"petabyte"`), 10^15 bytes. - PetaByte, - /// Pebibyte (`"pebibyte"`), 2^50 bytes. - PebiByte, - /// Exabyte (`"exabyte"`), 10^18 bytes. - ExaByte, - /// Exbibyte (`"exbibyte"`), 2^60 bytes. - ExbiByte, -} - -impl Default for InformationUnit { - fn default() -> Self { - Self::Byte - } -} - -impl fmt::Display for InformationUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Bit => f.write_str("bit"), - Self::Byte => f.write_str("byte"), - Self::KiloByte => f.write_str("kilobyte"), - Self::KibiByte => f.write_str("kibibyte"), - Self::MegaByte => f.write_str("megabyte"), - Self::MebiByte => f.write_str("mebibyte"), - Self::GigaByte => f.write_str("gigabyte"), - Self::GibiByte => f.write_str("gibibyte"), - Self::TeraByte => f.write_str("terabyte"), - Self::TebiByte => f.write_str("tebibyte"), - Self::PetaByte => f.write_str("petabyte"), - Self::PebiByte => f.write_str("pebibyte"), - Self::ExaByte => f.write_str("exabyte"), - Self::ExbiByte => f.write_str("exbibyte"), - } - } -} - -/// Units of fraction used in [`MetricUnit::Fraction`]. -/// -/// Defaults to `ratio`. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum FractionUnit { - /// Floating point fraction of `1`. - Ratio, - /// Ratio expressed as a fraction of `100`. `100%` equals a ratio of `1.0`. - Percent, -} - -impl Default for FractionUnit { - fn default() -> Self { - Self::Ratio - } -} - -impl fmt::Display for FractionUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Ratio => f.write_str("ratio"), - Self::Percent => f.write_str("percent"), - } - } -} - -/// Custom user-defined units without builtin conversion. -#[derive(Clone, Copy, Eq, PartialEq, Hash)] -pub struct CustomUnit([u8; 15]); - -impl CustomUnit { - /// Parses a `CustomUnit` from a string. - pub fn parse(s: &str) -> Result { - if s.len() > 15 || !s.is_ascii() { - return Err(ParseMetricError(())); - } - - let mut unit = Self(Default::default()); - unit.0.copy_from_slice(s.as_bytes()); - unit.0.make_ascii_lowercase(); - Ok(unit) - } - - /// Returns the string representation of this unit. - #[inline] - pub fn as_str(&self) -> &str { - // Safety: The string is already validated to be of length 32 and valid ASCII when - // constructing `ProjectKey`. - unsafe { std::str::from_utf8_unchecked(&self.0) } - } -} - -impl fmt::Debug for CustomUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} - -impl fmt::Display for CustomUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} - -impl std::str::FromStr for CustomUnit { - type Err = ParseMetricError; - - fn from_str(s: &str) -> Result { - Self::parse(s) - } -} - -impl std::ops::Deref for CustomUnit { - type Target = str; - - fn deref(&self) -> &Self::Target { - self.as_str() - } -} - -/// The [unit](Metric::unit) of measurement of a metric [value](Metric::value). -/// -/// Units augment metric values by giving them a magnitude and semantics. There are certain types of -/// units that are subdivided in their precision, such as the [`DurationUnit`] for time -/// measurements. -/// -/// Units and their precisions are uniquely represented by a string identifier. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum MetricUnit { - /// A time duration, defaulting to `"millisecond"`. - Duration(DurationUnit), - /// Size of information derived from bytes, defaulting to `"byte"`. - Information(InformationUnit), - /// Fractions such as percentages, defaulting to `"ratio"`. - Fraction(FractionUnit), - /// user-defined units without builtin conversion or default. - Custom(CustomUnit), - /// Untyped value without a unit (`""`). - None, -} - -impl MetricUnit { - /// Returns `true` if the metric_unit is [`None`]. - pub fn is_none(&self) -> bool { - matches!(self, Self::None) - } -} - -impl Default for MetricUnit { - fn default() -> Self { - MetricUnit::None - } -} - -impl fmt::Display for MetricUnit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - MetricUnit::Duration(u) => u.fmt(f), - MetricUnit::Information(u) => u.fmt(f), - MetricUnit::Fraction(u) => u.fmt(f), - MetricUnit::Custom(u) => u.fmt(f), - MetricUnit::None => f.write_str("none"), - } - } -} - -impl std::str::FromStr for MetricUnit { - type Err = ParseMetricError; - - fn from_str(s: &str) -> Result { - Ok(match s { - "nanosecond" | "ns" => Self::Duration(DurationUnit::NanoSecond), - "microsecond" => Self::Duration(DurationUnit::MicroSecond), - "millisecond" | "ms" => Self::Duration(DurationUnit::MilliSecond), - "second" | "s" => Self::Duration(DurationUnit::Second), - "minute" => Self::Duration(DurationUnit::Minute), - "hour" => Self::Duration(DurationUnit::Hour), - "day" => Self::Duration(DurationUnit::Day), - "week" => Self::Duration(DurationUnit::Week), - - "bit" => Self::Information(InformationUnit::Bit), - "byte" => Self::Information(InformationUnit::Byte), - "kilobyte" => Self::Information(InformationUnit::KiloByte), - "kibibyte" => Self::Information(InformationUnit::KibiByte), - "megabyte" => Self::Information(InformationUnit::MegaByte), - "mebibyte" => Self::Information(InformationUnit::MebiByte), - "gigabyte" => Self::Information(InformationUnit::GigaByte), - "gibibyte" => Self::Information(InformationUnit::GibiByte), - "terabyte" => Self::Information(InformationUnit::TeraByte), - "tebibyte" => Self::Information(InformationUnit::TebiByte), - "petabyte" => Self::Information(InformationUnit::PetaByte), - "pebibyte" => Self::Information(InformationUnit::PebiByte), - "exabyte" => Self::Information(InformationUnit::ExaByte), - "exbibyte" => Self::Information(InformationUnit::ExbiByte), - - "ratio" => Self::Fraction(FractionUnit::Ratio), - "percent" => Self::Fraction(FractionUnit::Percent), - - "" | "unit" | "none" => Self::None, - _ => Self::Custom(CustomUnit::parse(s)?), - }) - } -} - -relay_common::impl_str_serde!(MetricUnit, "a metric unit string"); +#[doc(inline)] +pub use relay_common::{ + CustomUnit, DurationUnit, FractionUnit, InformationUnit, MetricUnit, ParseMetricUnitError, +}; /// The [typed value](Metric::value) of a metric. #[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)] diff --git a/relay-server/src/metrics_extraction/transactions.rs b/relay-server/src/metrics_extraction/transactions.rs index e44b39cda2..b532a8c602 100644 --- a/relay-server/src/metrics_extraction/transactions.rs +++ b/relay-server/src/metrics_extraction/transactions.rs @@ -229,33 +229,36 @@ fn extract_universal_tags( tags } -/// Returns the unit of the provided metric. Defaults to None. +/// Returns the unit of the provided metric. +/// +/// For known measurements, this returns `Some(MetricUnit)`, which can also include +/// `Some(MetricUnit::None)`. For unknown measurement names, this returns `None`. #[cfg(feature = "processing")] -fn get_metric_measurement_unit(metric: &str) -> MetricUnit { +fn get_metric_measurement_unit(metric: &str) -> Option { match metric { // Web - "fcp" => MetricUnit::Duration(DurationUnit::MilliSecond), - "lcp" => MetricUnit::Duration(DurationUnit::MilliSecond), - "fid" => MetricUnit::Duration(DurationUnit::MilliSecond), - "fp" => MetricUnit::Duration(DurationUnit::MilliSecond), - "ttfb" => MetricUnit::Duration(DurationUnit::MilliSecond), - "ttfb.requesttime" => MetricUnit::Duration(DurationUnit::MilliSecond), - "cls" => MetricUnit::None, + "fcp" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "lcp" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "fid" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "fp" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "ttfb" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "ttfb.requesttime" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "cls" => Some(MetricUnit::None), // Mobile - "app_start_cold" => MetricUnit::Duration(DurationUnit::MilliSecond), - "app_start_warm" => MetricUnit::Duration(DurationUnit::MilliSecond), - "frames_total" => MetricUnit::None, - "frames_slow" => MetricUnit::None, - "frames_frozen" => MetricUnit::None, + "app_start_cold" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "app_start_warm" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "frames_total" => Some(MetricUnit::None), + "frames_slow" => Some(MetricUnit::None), + "frames_frozen" => Some(MetricUnit::None), // React-Native - "stall_count" => MetricUnit::None, - "stall_total_time" => MetricUnit::Duration(DurationUnit::MilliSecond), - "stall_longest_time" => MetricUnit::Duration(DurationUnit::MilliSecond), + "stall_count" => Some(MetricUnit::None), + "stall_total_time" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), + "stall_longest_time" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)), // Default - _ => MetricUnit::None, + _ => None, } } @@ -316,22 +319,35 @@ fn extract_transaction_metrics_inner( // Measurements if let Some(measurements) = event.measurements.value() { - for (measurement_name, annotated) in measurements.iter() { - let measurement = match annotated.value().and_then(|m| m.value.value()) { - Some(measurement) => *measurement, + for (name, annotated) in measurements.iter() { + let measurement = match annotated.value() { + Some(m) => m, + None => continue, + }; + + let value = match measurement.value.value() { + Some(value) => *value, None => continue, }; let mut tags_for_measurement = tags.clone(); - if let Some(rating) = get_measurement_rating(measurement_name, measurement) { + if let Some(rating) = get_measurement_rating(name, value) { tags_for_measurement.insert("measurement_rating".to_owned(), rating); } + let stated_unit = measurement.unit.value().copied(); + let default_unit = get_metric_measurement_unit(name); + if let (Some(default), Some(stated)) = (default_unit, stated_unit) { + if default != stated { + relay_log::error!("unit mismatch on measurements.{}: {}", name, stated); + } + } + push_metric(Metric::new_mri( METRIC_NAMESPACE, - format_args!("measurements.{}", measurement_name), - get_metric_measurement_unit(measurement_name), - MetricValue::Distribution(measurement), + format_args!("measurements.{}", name), + stated_unit.or(default_unit).unwrap_or_default(), + MetricValue::Distribution(value), unix_timestamp, tags_for_measurement, )); @@ -343,16 +359,23 @@ fn extract_transaction_metrics_inner( for (breakdown, measurements) in store::get_breakdown_measurements(event, breakdowns_config) { for (measurement_name, annotated) in measurements.iter() { - let measurement = match annotated.value().and_then(|m| m.value.value()) { - Some(measurement) => *measurement, + let measurement = match annotated.value() { + Some(m) => m, + None => continue, + }; + + let value = match measurement.value.value() { + Some(value) => *value, None => continue, }; + let unit = measurement.unit.value(); + push_metric(Metric::new_mri( METRIC_NAMESPACE, format_args!("breakdowns.{}.{}", breakdown, measurement_name), - MetricUnit::None, - MetricValue::Distribution(measurement), + unit.copied().unwrap_or(MetricUnit::None), + MetricValue::Distribution(value), unix_timestamp, tags.clone(), )); @@ -511,7 +534,7 @@ mod tests { "extractMetrics": [ "d:transactions/measurements.foo@none", "d:transactions/measurements.lcp@millisecond", - "d:transactions/breakdowns.span_ops.ops.react.mount@none", + "d:transactions/breakdowns.span_ops.ops.react.mount@millisecond", "d:transactions/duration@millisecond", "s:transactions/user@none" ], @@ -537,9 +560,17 @@ mod tests { metrics[1].name, "d:transactions/measurements.lcp@millisecond" ); + assert_eq!( + metrics[2].unit, + MetricUnit::Duration(DurationUnit::MilliSecond) + ); assert_eq!( metrics[2].name, - "d:transactions/breakdowns.span_ops.ops.react.mount@none" + "d:transactions/breakdowns.span_ops.ops.react.mount@millisecond" + ); + assert_eq!( + metrics[2].unit, + MetricUnit::Duration(DurationUnit::MilliSecond) ); let duration_metric = &metrics[3]; @@ -636,6 +667,43 @@ mod tests { assert_eq!(metrics[2].unit, MetricUnit::None, "{:?}", metrics[2]); } + #[test] + fn test_metric_measurement_unit_overrides() { + let json = r#"{ + "type": "transaction", + "timestamp": "2021-04-26T08:00:00+0100", + "start_timestamp": "2021-04-26T07:59:01+0100", + "measurements": { + "fcp": {"value": 1.1, "unit": "second"}, + "lcp": {"value": 2.2, "unit": "none"} + } + }"#; + + let config: TransactionMetricsConfig = serde_json::from_str( + r#"{ + "extractMetrics": [ + "d:transactions/measurements.fcp@second", + "d:transactions/measurements.lcp@none" + ] + }"#, + ) + .unwrap(); + + let event = Annotated::from_json(json).unwrap(); + + let mut metrics = vec![]; + extract_transaction_metrics(&config, None, &[], event.value().unwrap(), &mut metrics); + + assert_eq!(metrics.len(), 2); + + assert_eq!(metrics[0].name, "d:transactions/measurements.fcp@second"); + assert_eq!(metrics[0].unit, MetricUnit::Duration(DurationUnit::Second)); + + // None is an override, too. + assert_eq!(metrics[1].name, "d:transactions/measurements.lcp@none"); + assert_eq!(metrics[1].unit, MetricUnit::None); + } + #[test] fn test_transaction_duration() { let json = r#" diff --git a/tests/integration/test_envelope.py b/tests/integration/test_envelope.py index 6b079b3157..4595105d24 100644 --- a/tests/integration/test_envelope.py +++ b/tests/integration/test_envelope.py @@ -299,14 +299,14 @@ def test_ops_breakdowns(mini_sentry, relay_with_processing, transactions_consume assert "breakdowns" in event, event assert event["breakdowns"] == { "span_ops": { - "ops.http": {"value": 2000000.0}, - "ops.resource": {"value": 100001.003}, - "total.time": {"value": 2200001.003}, + "ops.http": {"value": 2000000.0, "unit": "millisecond"}, + "ops.resource": {"value": 100001.003, "unit": "millisecond"}, + "total.time": {"value": 2200001.003, "unit": "millisecond"}, }, "span_ops_2": { - "ops.http": {"value": 2000000.0}, - "ops.resource": {"value": 100001.003}, - "total.time": {"value": 2200001.003}, + "ops.http": {"value": 2000000.0, "unit": "millisecond"}, + "ops.resource": {"value": 100001.003, "unit": "millisecond"}, + "total.time": {"value": 2200001.003, "unit": "millisecond"}, }, } diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py index f0bc9e274a..d737348e34 100644 --- a/tests/integration/test_metrics.py +++ b/tests/integration/test_metrics.py @@ -491,8 +491,8 @@ def test_transaction_metrics( "extractMetrics": [ "d:transactions/measurements.foo@none", "d:transactions/measurements.bar@none", - "d:transactions/breakdowns.span_ops.total.time@none", - "d:transactions/breakdowns.span_ops.ops.react.mount@none", + "d:transactions/breakdowns.span_ops.total.time@millisecond", + "d:transactions/breakdowns.span_ops.ops.react.mount@millisecond", ] } @@ -517,8 +517,8 @@ def test_transaction_metrics( event, _ = transactions_consumer.get_event() assert event["breakdowns"] == { "span_ops": { - "ops.react.mount": {"value": 9.910106}, - "total.time": {"value": 9.910106}, + "ops.react.mount": {"value": 9.910106, "unit": "millisecond"}, + "total.time": {"value": 9.910106, "unit": "millisecond"}, } } @@ -556,19 +556,21 @@ def test_transaction_metrics( "value": [1.3], } - assert metrics["d:transactions/breakdowns.span_ops.ops.react.mount@none"] == { + assert metrics[ + "d:transactions/breakdowns.span_ops.ops.react.mount@millisecond" + ] == { **common, - "name": "d:transactions/breakdowns.span_ops.ops.react.mount@none", + "name": "d:transactions/breakdowns.span_ops.ops.react.mount@millisecond", "type": "d", - "unit": "none", + "unit": "millisecond", "value": [9.910106, 9.910106], } - assert metrics["d:transactions/breakdowns.span_ops.total.time@none"] == { + assert metrics["d:transactions/breakdowns.span_ops.total.time@millisecond"] == { **common, - "name": "d:transactions/breakdowns.span_ops.total.time@none", + "name": "d:transactions/breakdowns.span_ops.total.time@millisecond", "type": "d", - "unit": "none", + "unit": "millisecond", "value": [9.910106, 9.910106], }