Don't crash on summaries

This cleans up a todo! so that we return an error rather than crashing when we try and merge a summary. There's no real way to merge individual summaries together in a way that makes sense, so an error is the best we can do. Signed-off-by: sinkingpoint <[email protected]>
sinkingpoint · Jan 22, 2023 · b6c8704 · b6c8704
1 parent b554ebb
commit b6c8704
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 9 deletions.
diff --git a/src/aggregator.rs b/src/aggregator.rs
@@ -199,7 +199,7 @@ fn merge_buckets(val1: &Vec<HistogramBucket>, val2: &Vec<HistogramBucket>) -> Ve
 }
 
 /// Merges two metrics into one another (using the given clearmode), storing the result in the first one.
-pub fn merge_metric(into: &mut Sample<GravelValue>, merge: Sample<GravelValue>, clear_mode: ClearMode) {
+pub fn merge_metric(into: &mut Sample<GravelValue>, merge: Sample<GravelValue>, clear_mode: ClearMode) -> Result<(), AggregationError> {
     match (&mut into.value, &merge.value) {
         (GravelValue::Prometheus(PrometheusValue::Unknown(val1)), GravelValue::Prometheus(PrometheusValue::Unknown(val2))) => {
             match clear_mode {
@@ -260,9 +260,11 @@ pub fn merge_metric(into: &mut Sample<GravelValue>, merge: Sample<GravelValue>,
                 _ => {}
             }
         },
-        (GravelValue::Prometheus(PrometheusValue::Summary(_)), GravelValue::Prometheus(PrometheusValue::Summary(_))) => todo!(),
+        (GravelValue::Prometheus(PrometheusValue::Summary(_)), GravelValue::Prometheus(PrometheusValue::Summary(_))) => return Err(AggregationError::Error("cannot merge summaries".to_string())),
         _ => unreachable!(),
     };
+
+    Ok(())
 }
 
 impl AggregationFamily {
@@ -322,7 +324,7 @@ impl AggregationFamily {
                     },
                     Some(s) => {
                         // Otherwise we have to merge
-                        merge_metric(s, metric, clear_mode);
+                        merge_metric(s, metric, clear_mode)?;
                     }
                 }
             }

diff --git a/src/aggregator_test.rs b/src/aggregator_test.rs
@@ -22,7 +22,7 @@ fn test_clear_mode_replace() {
     let mut sample = Sample::new(vec![], None, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(1))));
     merge_metric(&mut sample, 
                 Sample::new(vec![], None, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(2)))),
-                      ClearMode::Replace);
+                      ClearMode::Replace).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(2))));
 
@@ -43,7 +43,7 @@ fn test_clear_mode_replace() {
                         }),
                     }
                 ))),
-                ClearMode::Replace);
+                ClearMode::Replace).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Counter(PrometheusCounterValue{
         value: MetricNumber::Int(1000),
@@ -70,7 +70,7 @@ fn test_clear_mode_replace() {
                         exemplar: None
                     }
                 ))),
-                ClearMode::Replace);
+                ClearMode::Replace).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Counter(PrometheusCounterValue{
         value: MetricNumber::Int(1000),
@@ -83,7 +83,7 @@ fn test_clear_mode_aggregate() {
     let mut sample = Sample::new(vec![], None, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(1))));
     merge_metric(&mut sample, 
                 Sample::new(vec![], None, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(2)))),
-                      ClearMode::Aggregate);
+                      ClearMode::Aggregate).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Gauge(MetricNumber::Int(3))));
 
@@ -104,7 +104,7 @@ fn test_clear_mode_aggregate() {
                         }),
                     }
                 ))),
-                ClearMode::Aggregate);
+                ClearMode::Aggregate).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Counter(PrometheusCounterValue{
         value: MetricNumber::Int(1001),
@@ -131,7 +131,7 @@ fn test_clear_mode_aggregate() {
                         exemplar: None
                     }
                 ))),
-                ClearMode::Aggregate);
+                ClearMode::Aggregate).unwrap();
 
     assert_eq!(sample.value, GravelValue::Prometheus(PrometheusValue::Counter(PrometheusCounterValue{
         value: MetricNumber::Int(1001),