Skip to content

Commit

Permalink
feat(transactions): Extract computed measurements (#1373)
Browse files Browse the repository at this point in the history
Extract the following derived mobile measurements, and write them back
into the transaction:

frames_slow_rate := measurements.frames_slow / measurements.frames_total
frames_frozen_rate := measurements.frames_frozen / measurements.frames_total
stall_percentage := measurements.stall_total_time / transaction.duration

These new measurements will be available in both the metrics and the
transactions dataset.

The addition to the measurement allowlist is done in
getsentry/sentry#37330.
  • Loading branch information
jjbayer authored Aug 3, 2022
1 parent 6f66945 commit 3832874
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
- Refactor profile processing into its own crate. ([#1340](https://github.com/getsentry/relay/pull/1340))
- Treat "unknown" transaction source as low cardinality for safe SDKs. ([#1352](https://github.com/getsentry/relay/pull/1352), [#1356](https://github.com/getsentry/relay/pull/1356))
- Conditionally write a default transaction source to the transaction payload. ([#1354](https://github.com/getsentry/relay/pull/1354))
- Generate mobile measurements frames_frozen_rate, frames_slow_rate, stall_percentage. ([#1373](https://github.com/getsentry/relay/pull/1373))
- Change to the internals of the healthcheck endpoint. ([#1374](https://github.com/getsentry/relay/pull/1374))
- Re-encode the Typescript payload to normalize. ([#1372](https://github.com/getsentry/relay/pull/1372))


**Bug Fixes**:

- Fix a bug where unreal crash reports were dropped when metrics extraction is enabled. ([#1355](https://github.com/getsentry/relay/pull/1355))
Expand Down
8 changes: 8 additions & 0 deletions relay-general/src/protocol/measurements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ impl Measurements {
pub fn into_inner(self) -> Object<Measurement> {
self.0
}

/// Return the value of the measurement with the given name, if it exists.
pub fn get_value(&self, key: &str) -> Option<f64> {
self.get(key)
.and_then(Annotated::value)
.and_then(|x| x.value.value())
.copied()
}
}

impl FromValue for Measurements {
Expand Down
2 changes: 1 addition & 1 deletion relay-general/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub use self::geo::{GeoIpError, GeoIpLookup};
pub use normalize::breakdowns::{
get_breakdown_measurements, BreakdownConfig, BreakdownsConfig, SpanOperationsConfig,
};
pub use normalize::{is_valid_platform, normalize_dist};
pub use normalize::{compute_measurements, is_valid_platform, normalize_dist};
pub use transactions::{
get_measurement, get_transaction_op, is_high_cardinality_sdk, validate_timestamps,
};
Expand Down
118 changes: 117 additions & 1 deletion relay-general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ use chrono::{Duration, Utc};
use itertools::Itertools;
use lazy_static::lazy_static;
use regex::Regex;
use relay_common::{DurationUnit, FractionUnit, MetricUnit};
use smallvec::SmallVec;

use crate::processor::{MaxChars, ProcessValue, ProcessingState, Processor};
use crate::protocol::{
self, AsPair, Breadcrumb, ClientSdkInfo, Context, Contexts, DebugImage, Event, EventId,
EventType, Exception, Frame, HeaderName, HeaderValue, Headers, IpAddr, Level, LogEntry,
Request, SpanStatus, Stacktrace, Tags, TraceContext, User, VALID_PLATFORMS,
Measurement, Measurements, Request, SpanStatus, Stacktrace, Tags, TraceContext, User,
VALID_PLATFORMS,
};
use crate::store::{ClockDriftProcessor, GeoIpLookup, StoreConfig};
use crate::types::{
Expand Down Expand Up @@ -82,6 +84,50 @@ pub fn normalize_dist(dist: &mut Option<String>) {
}
}

/// Compute additional measurements derived from existing ones
pub fn compute_measurements(transaction_duration_ms: f64, measurements: &mut Measurements) {
if let Some(frames_total) = measurements.get_value("frames_total") {
if frames_total > 0.0 {
if let Some(frames_frozen) = measurements.get_value("frames_frozen") {
let frames_frozen_rate = Measurement {
value: (frames_frozen / frames_total).into(),
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
};
measurements.insert("frames_frozen_rate".to_owned(), frames_frozen_rate.into());
}
if let Some(frames_slow) = measurements.get_value("frames_slow") {
let frames_slow_rate = Measurement {
value: (frames_slow / frames_total).into(),
unit: MetricUnit::Fraction(FractionUnit::Ratio).into(),
};
measurements.insert("frames_slow_rate".to_owned(), frames_slow_rate.into());
}
}
}

// Get stall_percentage
if transaction_duration_ms > 0.0 {
if let Some(stall_total_time) = measurements
.get("stall_total_time")
.and_then(Annotated::value)
{
if matches!(
stall_total_time.unit.value(),
// Accept milliseconds or None, but not other units
Some(&MetricUnit::Duration(DurationUnit::MilliSecond) | &MetricUnit::None) | None
) {
if let Some(stall_total_time) = stall_total_time.value.0 {
let stall_percentage = Measurement {
value: (stall_total_time / transaction_duration_ms).into(),
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
};
measurements.insert("stall_percentage".to_owned(), stall_percentage.into());
}
}
}
}
}

/// The processor that normalizes events for store.
pub struct NormalizeProcessor<'a> {
config: Arc<StoreConfig>,
Expand Down Expand Up @@ -117,6 +163,13 @@ impl<'a> NormalizeProcessor<'a> {
if event.ty.value() != Some(&EventType::Transaction) {
// Only transaction events may have a measurements interface
event.measurements = Annotated::empty();
} else if let Some(measurements) = event.measurements.value_mut() {
let duration_millis = match (event.start_timestamp.0, event.timestamp.0) {
(Some(start), Some(end)) => relay_common::chrono_to_positive_millis(end - start),
_ => 0.0,
};

compute_measurements(duration_millis, measurements);
}
}

Expand Down Expand Up @@ -1646,3 +1699,66 @@ fn test_normalize_dist_whitespace() {
normalize_dist(&mut dist);
assert_eq!(dist.unwrap(), ""); // Not sure if this is what we want
}

#[test]
fn test_computed_measurements() {
use crate::types::SerializableAnnotated;
use insta::assert_ron_snapshot;

let json = r#"
{
"type": "transaction",
"timestamp": "2021-04-26T08:00:05+0100",
"start_timestamp": "2021-04-26T08:00:00+0100",
"measurements": {
"frames_slow": {"value": 1},
"frames_frozen": {"value": 2},
"frames_total": {"value": 4},
"stall_total_time": {"value": 4000, "unit": "millisecond"}
}
}
"#;

let mut event = Annotated::<Event>::from_json(json).unwrap().0.unwrap();

let processor = NormalizeProcessor::new(
Arc::new(StoreConfig {
..Default::default()
}),
None,
);
processor.normalize_measurements(&mut event);

assert_ron_snapshot!(SerializableAnnotated(&Annotated::new(event)), {}, @r###"{
"type": "transaction",
"timestamp": 1619420405,
"start_timestamp": 1619420400,
"measurements": {
"frames_frozen": {
"value": 2,
},
"frames_frozen_rate": {
"value": 0.5,
"unit": "ratio",
},
"frames_slow": {
"value": 1,
},
"frames_slow_rate": {
"value": 0.25,
"unit": "ratio",
},
"frames_total": {
"value": 4,
},
"stall_percentage": {
"value": 0.8,
"unit": "ratio",
},
"stall_total_time": {
"value": 4000,
"unit": "millisecond",
},
},
}"###);
}
70 changes: 68 additions & 2 deletions relay-server/src/metrics_extraction/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::metrics_extraction::conditional_tagging::run_conditional_tagging;
use crate::metrics_extraction::utils;
use crate::metrics_extraction::TaggingRule;
use crate::statsd::RelayCounters;
use relay_common::FractionUnit;
use relay_common::{SpanStatus, UnixTimestamp};
use relay_general::protocol::{
AsPair, Context, ContextInner, Event, EventType, Timestamp, TraceContext, TransactionSource,
Expand Down Expand Up @@ -357,12 +358,15 @@ fn get_metric_measurement_unit(metric: &str) -> Option<MetricUnit> {
"app_start_warm" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)),
"frames_total" => Some(MetricUnit::None),
"frames_slow" => Some(MetricUnit::None),
"frames_slow_rate" => Some(MetricUnit::Fraction(FractionUnit::Ratio)),
"frames_frozen" => Some(MetricUnit::None),
"frames_frozen_rate" => Some(MetricUnit::Fraction(FractionUnit::Ratio)),

// React-Native
"stall_count" => Some(MetricUnit::None),
"stall_total_time" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)),
"stall_longest_time" => Some(MetricUnit::Duration(DurationUnit::MilliSecond)),
"stall_percentage" => Some(MetricUnit::Fraction(FractionUnit::Ratio)),

// Default
_ => None,
Expand Down Expand Up @@ -424,6 +428,7 @@ fn extract_transaction_metrics_inner(
return; // invalid transaction
}
};
let duration_millis = relay_common::chrono_to_positive_millis(end_timestamp - start_timestamp);

let unix_timestamp = match UnixTimestamp::from_datetime(end_timestamp.into_inner()) {
Some(ts) => ts,
Expand All @@ -434,6 +439,8 @@ fn extract_transaction_metrics_inner(

// Measurements
if let Some(measurements) = event.measurements.value() {
let mut measurements = measurements.clone();
store::compute_measurements(duration_millis, &mut measurements);
for (name, annotated) in measurements.iter() {
let measurement = match annotated.value() {
Some(m) => m,
Expand Down Expand Up @@ -510,8 +517,6 @@ fn extract_transaction_metrics_inner(
};

// Duration
let duration_millis = relay_common::chrono_to_positive_millis(end_timestamp - start_timestamp);

push_metric(Metric::new_mri(
METRIC_NAMESPACE,
"duration",
Expand Down Expand Up @@ -1681,6 +1686,67 @@ mod tests {
}
}

#[test]
fn test_computed_metrics() {
let json = r#"{
"type": "transaction",
"timestamp": 1619420520,
"start_timestamp": 1619420400,
"measurements": {
"frames_frozen": {
"value": 2
},
"frames_slow": {
"value": 1
},
"frames_total": {
"value": 4
},
"stall_total_time": {
"value": 4
}
}
}"#;

let config: TransactionMetricsConfig = serde_json::from_str(
r#"
{
"extractMetrics": [
"d:transactions/measurements.frames_frozen_rate@ratio",
"d:transactions/measurements.frames_slow_rate@ratio",
"d:transactions/measurements.stall_percentage@ratio"
]
}
"#,
)
.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(), 3, "{:?}", metrics);

assert_eq!(
metrics[0].name, "d:transactions/measurements.frames_frozen_rate@ratio",
"{:?}",
metrics[0]
);

assert_eq!(
metrics[1].name, "d:transactions/measurements.frames_slow_rate@ratio",
"{:?}",
metrics[1]
);

assert_eq!(
metrics[2].name, "d:transactions/measurements.stall_percentage@ratio",
"{:?}",
metrics[2]
);
}

#[test]
fn test_get_eventuser_tag() {
// Note: If this order changes,
Expand Down

0 comments on commit 3832874

Please sign in to comment.