Skip to content

Commit 61dc452

Browse files
authored
ref: Use FiniteF64 for measurements (#4828)
The `FiniteF64` type is intended for `f64` values that can't be ±∞ or NaN. It's currently only used for metrics, but measurements ought to work the same way. Therefore, this PR changes measurements to `FiniteF64`. * It moves `FiniteF64` to `relay-protocol` so that all parts of Relay that need it have access to it. * It implements a bunch of traits for `FiniteF64`: `FromValue`, `IntoValue`, `ProcessValue`, `Empty`, `PartialOrd<f64>`, `PartialEq<f64>`, and `AddAssign`. The only notable thing here is that in order to implement `ProcessValue` I added a new method `process_finite_f64` to the `Processor` trait, as simply reusing `process_f64` wouldn't be sound. Since measurement values were `Annotated<f64>` before, it costs us very little to change them to `Annotated<FiniteF64>` and use the `Annotated` for invalid result values.
1 parent b4dcfb2 commit 61dc452

File tree

30 files changed

+249
-201
lines changed

30 files changed

+249
-201
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
- Produce spans to the items topic. ([#4735](https://github.com/getsentry/relay/pull/4735))
1212
- Take into account more types of tokens when doing AI cost calculation. ([#4840](https://github.com/getsentry/relay/pull/4840))
13+
- Use the `FiniteF64` type for measurements. ([#4828](https://github.com/getsentry/relay/pull/4828))
1314

1415
## 25.6.1
1516

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

relay-event-normalization/src/event.rs

Lines changed: 56 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ use relay_event_schema::protocol::{
2121
SpanStatus, Tags, Timestamp, TraceContext, User, VALID_PLATFORMS,
2222
};
2323
use relay_protocol::{
24-
Annotated, Empty, Error, ErrorKind, FromValue, Getter, Meta, Object, Remark, RemarkType, Value,
24+
Annotated, Empty, Error, ErrorKind, FiniteF64, FromValue, Getter, Meta, Object, Remark,
25+
RemarkType, TryFromFloatError, Value,
2526
};
2627
use smallvec::SmallVec;
2728
use uuid::Uuid;
@@ -866,10 +867,9 @@ pub fn normalize_measurements(
866867
normalize_mobile_measurements(measurements);
867868
normalize_units(measurements);
868869

869-
let duration_millis = match (start_timestamp, end_timestamp) {
870-
(Some(start), Some(end)) => relay_common::time::chrono_to_positive_millis(end - start),
871-
_ => 0.0,
872-
};
870+
let duration_millis = start_timestamp.zip(end_timestamp).and_then(|(start, end)| {
871+
FiniteF64::new(relay_common::time::chrono_to_positive_millis(end - start))
872+
});
873873

874874
compute_measurements(duration_millis, measurements);
875875
if let Some(measurements_config) = measurements_config {
@@ -910,8 +910,8 @@ pub fn normalize_performance_score(
910910
// a measurement with weight is missing.
911911
continue;
912912
}
913-
let mut score_total = 0.0f64;
914-
let mut weight_total = 0.0f64;
913+
let mut score_total = FiniteF64::ZERO;
914+
let mut weight_total = FiniteF64::ZERO;
915915
for component in &profile.score_components {
916916
// Skip optional components if they are not present on the event.
917917
if component.optional
@@ -921,44 +921,55 @@ pub fn normalize_performance_score(
921921
}
922922
weight_total += component.weight;
923923
}
924-
if weight_total.abs() < f64::EPSILON {
924+
if weight_total.abs() < FiniteF64::EPSILON {
925925
// All components are optional or have a weight of `0`. We cannot compute
926926
// component weights, so we bail.
927927
continue;
928928
}
929929
for component in &profile.score_components {
930930
// Optional measurements that are not present are given a weight of 0.
931-
let mut normalized_component_weight = 0.0;
931+
let mut normalized_component_weight = FiniteF64::ZERO;
932+
932933
if let Some(value) = measurements.get_value(component.measurement.as_str()) {
933-
normalized_component_weight = component.weight / weight_total;
934+
normalized_component_weight = component.weight.saturating_div(weight_total);
934935
let cdf = utils::calculate_cdf_score(
935-
value.max(0.0), // Webvitals can't be negative, but we need to clamp in case of bad data.
936-
component.p10,
937-
component.p50,
936+
value.to_f64().max(0.0), // Webvitals can't be negative, but we need to clamp in case of bad data.
937+
component.p10.to_f64(),
938+
component.p50.to_f64(),
938939
);
939940

941+
let cdf = Annotated::try_from(cdf);
942+
940943
measurements.insert(
941944
format!("score.ratio.{}", component.measurement),
942945
Measurement {
943-
value: cdf.into(),
946+
value: cdf.clone(),
944947
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
945948
}
946949
.into(),
947950
);
948951

949-
let component_score = cdf * normalized_component_weight;
950-
score_total += component_score;
951-
should_add_total = true;
952+
let component_score =
953+
cdf.and_then(|cdf| match cdf * normalized_component_weight {
954+
Some(v) => Annotated::new(v),
955+
None => Annotated::from_error(TryFromFloatError, None),
956+
});
957+
958+
if let Some(component_score) = component_score.value() {
959+
score_total += *component_score;
960+
should_add_total = true;
961+
}
952962

953963
measurements.insert(
954964
format!("score.{}", component.measurement),
955965
Measurement {
956-
value: component_score.into(),
966+
value: component_score,
957967
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
958968
}
959969
.into(),
960970
);
961971
}
972+
962973
measurements.insert(
963974
format!("score.weight.{}", component.measurement),
964975
Measurement {
@@ -1042,7 +1053,10 @@ impl MutMeasurements for Span {
10421053
/// frames_frozen_rate := measurements.frames_frozen / measurements.frames_total
10431054
/// stall_percentage := measurements.stall_total_time / transaction.duration
10441055
/// ```
1045-
fn compute_measurements(transaction_duration_ms: f64, measurements: &mut Measurements) {
1056+
fn compute_measurements(
1057+
transaction_duration_ms: Option<FiniteF64>,
1058+
measurements: &mut Measurements,
1059+
) {
10461060
if let Some(frames_total) = measurements.get_value("frames_total") {
10471061
if frames_total > 0.0 {
10481062
if let Some(frames_frozen) = measurements.get_value("frames_frozen") {
@@ -1063,22 +1077,25 @@ fn compute_measurements(transaction_duration_ms: f64, measurements: &mut Measure
10631077
}
10641078

10651079
// Get stall_percentage
1066-
if transaction_duration_ms > 0.0 {
1067-
if let Some(stall_total_time) = measurements
1068-
.get("stall_total_time")
1069-
.and_then(Annotated::value)
1070-
{
1071-
if matches!(
1072-
stall_total_time.unit.value(),
1073-
// Accept milliseconds or None, but not other units
1074-
Some(&MetricUnit::Duration(DurationUnit::MilliSecond) | &MetricUnit::None) | None
1075-
) {
1076-
if let Some(stall_total_time) = stall_total_time.value.0 {
1077-
let stall_percentage = Measurement {
1078-
value: (stall_total_time / transaction_duration_ms).into(),
1079-
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
1080-
};
1081-
measurements.insert("stall_percentage".to_owned(), stall_percentage.into());
1080+
if let Some(transaction_duration_ms) = transaction_duration_ms {
1081+
if transaction_duration_ms > 0.0 {
1082+
if let Some(stall_total_time) = measurements
1083+
.get("stall_total_time")
1084+
.and_then(Annotated::value)
1085+
{
1086+
if matches!(
1087+
stall_total_time.unit.value(),
1088+
// Accept milliseconds or None, but not other units
1089+
Some(&MetricUnit::Duration(DurationUnit::MilliSecond) | &MetricUnit::None)
1090+
| None
1091+
) {
1092+
if let Some(stall_total_time) = stall_total_time.value.0 {
1093+
let stall_percentage = Measurement {
1094+
value: (stall_total_time / transaction_duration_ms).into(),
1095+
unit: (MetricUnit::Fraction(FractionUnit::Ratio)).into(),
1096+
};
1097+
measurements.insert("stall_percentage".to_owned(), stall_percentage.into());
1098+
}
10821099
}
10831100
}
10841101
}
@@ -1409,7 +1426,7 @@ fn remove_invalid_measurements(
14091426
// Check if this is a builtin measurement:
14101427
for builtin_measurement in measurements_config.builtin_measurement_keys() {
14111428
if builtin_measurement.name() == name {
1412-
let value = measurement.value.value().unwrap_or(&0.0);
1429+
let value = measurement.value.value().unwrap_or(&FiniteF64::ZERO);
14131430
// Drop negative values if the builtin measurement does not allow them.
14141431
if !builtin_measurement.allow_negative() && *value < 0.0 {
14151432
meta.add_error(Error::invalid(format!(
@@ -2051,7 +2068,7 @@ mod tests {
20512068
fn test_keeps_valid_measurement() {
20522069
let name = "lcp";
20532070
let measurement = Measurement {
2054-
value: Annotated::new(420.69),
2071+
value: Annotated::new(420.69.try_into().unwrap()),
20552072
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
20562073
};
20572074

@@ -2062,7 +2079,7 @@ mod tests {
20622079
fn test_drops_too_long_measurement_names() {
20632080
let name = "lcpppppppppppppppppppppppppppp";
20642081
let measurement = Measurement {
2065-
value: Annotated::new(420.69),
2082+
value: Annotated::new(420.69.try_into().unwrap()),
20662083
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
20672084
};
20682085

@@ -2073,7 +2090,7 @@ mod tests {
20732090
fn test_drops_measurements_with_invalid_characters() {
20742091
let name = "i æm frøm nørwåy";
20752092
let measurement = Measurement {
2076-
value: Annotated::new(420.69),
2093+
value: Annotated::new(420.69.try_into().unwrap()),
20772094
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
20782095
};
20792096

relay-event-normalization/src/normalize/breakdowns.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl EmitBreakdowns for SpanOperationsConfig {
139139
};
140140

141141
let op_value = Measurement {
142-
value: Annotated::new(relay_common::time::duration_to_millis(op_duration)),
142+
value: Annotated::try_from(relay_common::time::duration_to_millis(op_duration)),
143143
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
144144
};
145145

@@ -148,7 +148,7 @@ impl EmitBreakdowns for SpanOperationsConfig {
148148
}
149149

150150
let total_time_value = Annotated::new(Measurement {
151-
value: Annotated::new(relay_common::time::duration_to_millis(total_time)),
151+
value: Annotated::try_from(relay_common::time::duration_to_millis(total_time)),
152152
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
153153
});
154154
breakdown.insert("total.time".to_string(), total_time_value);
@@ -264,7 +264,7 @@ mod tests {
264264
span_ops_breakdown.insert(
265265
"lcp".to_owned(),
266266
Annotated::new(Measurement {
267-
value: Annotated::new(420.69),
267+
value: Annotated::new(420.69.try_into().unwrap()),
268268
unit: Annotated::empty(),
269269
}),
270270
);
@@ -366,7 +366,7 @@ mod tests {
366366
"ops.http".to_owned(),
367367
Annotated::new(Measurement {
368368
// 1 hour in milliseconds
369-
value: Annotated::new(3_600_000.0),
369+
value: Annotated::new(3_600_000.0.try_into().unwrap()),
370370
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
371371
}),
372372
);
@@ -375,7 +375,7 @@ mod tests {
375375
"ops.db".to_owned(),
376376
Annotated::new(Measurement {
377377
// 2 hours in milliseconds
378-
value: Annotated::new(7_200_000.0),
378+
value: Annotated::new(7_200_000.0.try_into().unwrap()),
379379
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
380380
}),
381381
);
@@ -384,7 +384,7 @@ mod tests {
384384
"total.time".to_owned(),
385385
Annotated::new(Measurement {
386386
// 4 hours and 10 microseconds in milliseconds
387-
value: Annotated::new(14_400_000.01),
387+
value: Annotated::new(14_400_000.01.try_into().unwrap()),
388388
unit: Annotated::new(MetricUnit::Duration(DurationUnit::MilliSecond)),
389389
}),
390390
);

relay-event-normalization/src/normalize/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::hash::Hash;
44
use relay_base_schema::metrics::MetricUnit;
55
use relay_common::glob2::LazyGlob;
66
use relay_event_schema::protocol::{Event, VALID_PLATFORMS};
7-
use relay_protocol::RuleCondition;
7+
use relay_protocol::{FiniteF64, RuleCondition};
88
use serde::{Deserialize, Serialize};
99

1010
pub mod breakdowns;
@@ -176,11 +176,11 @@ pub struct PerformanceScoreWeightedComponent {
176176
/// profile will be discarded.
177177
pub measurement: String,
178178
/// Weight [0,1.0] of this component in the performance score
179-
pub weight: f64,
179+
pub weight: FiniteF64,
180180
/// p10 used to define the log-normal for calculation
181-
pub p10: f64,
181+
pub p10: FiniteF64,
182182
/// Median used to define the log-normal for calculation
183-
pub p50: f64,
183+
pub p50: FiniteF64,
184184
/// Whether the measurement is optional. If the measurement is missing, performance score processing
185185
/// may still continue, and the weight will be 0.
186186
#[serde(default)]

relay-event-normalization/src/normalize/span/ai.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub fn map_ai_measurements_to_data(span: &mut Span) {
5454
if let Some(measurements) = measurements {
5555
if target_field.value().is_none() {
5656
if let Some(value) = measurements.get_value(measurement_key) {
57-
target_field.set_value(Value::F64(value).into());
57+
target_field.set_value(Value::F64(value.to_f64()).into());
5858
}
5959
}
6060
}

relay-event-normalization/src/normalize/span/tag_extraction.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use relay_event_schema::protocol::{
1313
AppContext, BrowserContext, DeviceContext, Event, GpuContext, Measurement, MonitorContext,
1414
OsContext, ProfileContext, RuntimeContext, SentryTags, Span, Timestamp, TraceContext,
1515
};
16-
use relay_protocol::{Annotated, Empty, Value};
16+
use relay_protocol::{Annotated, Empty, FiniteF64, Value};
1717
use sqlparser::ast::Visit;
1818
use sqlparser::ast::{ObjectName, Visitor};
1919
use url::Url;
@@ -456,7 +456,7 @@ fn extract_segment_measurements(event: &Event) -> BTreeMap<String, Measurement>
456456
MetricUnit::Information(InformationUnit::Byte),
457457
),
458458
] {
459-
if let Some(value) = value_to_f64(field.value()) {
459+
if let Some(value) = value_to_finite_f64(field.value()) {
460460
measurements.insert(
461461
key.into(),
462462
Measurement {
@@ -1108,13 +1108,15 @@ pub fn extract_tags(
11081108
span_tags
11091109
}
11101110

1111-
fn value_to_f64(val: Option<&Value>) -> Option<f64> {
1112-
match val {
1111+
fn value_to_finite_f64(val: Option<&Value>) -> Option<FiniteF64> {
1112+
let float = match val {
11131113
Some(Value::F64(f)) => Some(*f),
11141114
Some(Value::I64(i)) => Some(*i as f64),
11151115
Some(Value::U64(u)) => Some(*u as f64),
11161116
_ => None,
1117-
}
1117+
};
1118+
1119+
float.and_then(FiniteF64::new)
11181120
}
11191121

11201122
/// Copies specific numeric values from span data to span measurements.
@@ -1125,7 +1127,7 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) {
11251127

11261128
if span_op.starts_with("cache.") {
11271129
if let Some(data) = span.data.value() {
1128-
if let Some(value) = value_to_f64(data.cache_item_size.value()) {
1130+
if let Some(value) = value_to_finite_f64(data.cache_item_size.value()) {
11291131
let measurements = span.measurements.get_or_insert_with(Default::default);
11301132
measurements.insert(
11311133
"cache.item_size".to_owned(),
@@ -1155,7 +1157,7 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) {
11551157
"http.response_transfer_size",
11561158
),
11571159
] {
1158-
if let Some(value) = value_to_f64(field.value()) {
1160+
if let Some(value) = value_to_finite_f64(field.value()) {
11591161
let measurements = span.measurements.get_or_insert_with(Default::default);
11601162
measurements.insert(
11611163
key.into(),
@@ -1189,7 +1191,7 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) {
11891191
MetricUnit::Information(InformationUnit::Byte),
11901192
),
11911193
] {
1192-
if let Some(value) = value_to_f64(field.value()) {
1194+
if let Some(value) = value_to_finite_f64(field.value()) {
11931195
let measurements = span.measurements.get_or_insert_with(Default::default);
11941196
measurements.insert(
11951197
key.into(),
@@ -1216,7 +1218,7 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) {
12161218
MetricUnit::Duration(DurationUnit::Second),
12171219
),
12181220
] {
1219-
if let Some(value) = value_to_f64(field.value()) {
1221+
if let Some(value) = value_to_finite_f64(field.value()) {
12201222
let measurements = span.measurements.get_or_insert_with(Default::default);
12211223
measurements.insert(
12221224
key.into(),

0 commit comments

Comments
 (0)