Merge 745f109 into 996de56

src/configuration.rs

@@ -30,8 +30,6 @@ pub struct BenchmarkConfig {
     pub verbose: bool,
     #[builder(default = "1")]
     pub concurrency: usize,
-    #[builder(default = "1000")]
-    pub noise_threshold: usize,
     pub rate_ladder: RateLadder,
     pub mode: BenchmarkMode,
     #[builder(default)]
@@ -55,7 +53,6 @@ impl BenchmarkConfig {
             (@arg RATE_STEP: --rate_step +takes_value "Rate increase step (until it reaches --rate_max).")
             (@arg RATE_MAX: --rate_max +takes_value "Max rate per second. Requires --rate-step")
             (@arg MAX_RATE_ITERATIONS: --max_iter -m +takes_value "The number of iterations with the max rate. By default `1`.")
-            (@arg NOISE_THRESHOLD: --noise_threshold +takes_value "Noise threshold (in standard deviations) - a positive integer. By default it's `6`, which means latency deviated more than 6 stddev from the mean are ignored")
             (@arg PROMETHEUS_ADDR: --prometheus +takes_value "If you'd like to send metrics to Prometheus PushGateway, specify the server URL. E.g. 10.0.0.1:9091")
             (@arg PROMETHEUS_JOB: --prometheus_job +takes_value "Prometheus Job (by default `pushgateway`)")
             (@subcommand http =>
@@ -78,7 +75,6 @@ impl BenchmarkConfig {
         let rate_per_second = matches.value_of("RATE");
         let rate_step = matches.value_of("RATE_STEP");
         let rate_max = matches.value_of("RATE_MAX");
-        let noise_threshold = matches.value_of("NOISE_THRESHOLD");
         let max_rate_iterations = matches.value_of("MAX_RATE_ITERATIONS").unwrap_or("1");
 
         let duration = matches.value_of("DURATION").map(|d| {
@@ -144,10 +140,6 @@ impl BenchmarkConfig {
             .name(test_case_name)
             .rate_ladder(rate_ladder)
             .concurrency(parse_num(concurrency, "Cannot parse CONCURRENCY"))
-            .noise_threshold(parse_num(
-                noise_threshold.unwrap_or("1000"),
-                "Cannot parse NOISE_THRESHOLD",
-            ))
             .verbose(false)
             .mode(BenchmarkConfig::build_mode(&matches))
             .reporters(metrics_destinations)

src/main.rs

@@ -52,9 +52,7 @@ async fn main() -> io::Result<()> {
         let batch_run_result = batch.run(metrics).await;
         match batch_run_result {
             Ok(stats) => {
-                batch_metric_sender
-                    .send(stats.reduce_noise(benchmark_config.noise_threshold))
-                    .unwrap_or_default();
+                batch_metric_sender.send(stats).unwrap_or_default();
             }
             Err(e) => {
                 error!("Unexpected error during batch run: {}", e);

src/metrics.rs

@@ -1,7 +1,7 @@
 use bytesize::ByteSize;
 use core::fmt;
 use histogram::Histogram;
-use log::info;
+use log::{error, info};
 use std::collections::HashMap;
 use std::ops::AddAssign;
 use std::time::{Duration, Instant};
@@ -78,43 +78,35 @@ impl BenchRunMetrics {
         self.summary.entry(stats.status).or_insert(0).add_assign(1);
     }
 
-    pub fn reduce_noise(&self, sigmas: usize) -> Self {
-        Self {
-            bench_begin: self.bench_begin,
-            total_bytes: self.total_bytes,
-            total_requests: self.total_requests,
-            successful_requests: self.successful_requests,
-            summary: self.summary.clone(),
-            success_latency: BenchRunMetrics::remove_noise(&self.success_latency, sigmas),
-            error_latency: self.error_latency.clone(),
-        }
-    }
-
-    fn remove_noise(histogram: &Histogram, sigmas: usize) -> Histogram {
-        let mean = histogram.mean().unwrap_or_default() as i64;
-        let stddev = histogram.stddev().unwrap_or_default() as i64;
-        let mut result = Histogram::new();
+    pub fn truncated_mean(histogram: &Histogram, threshold: f64) -> u64 {
+        let lowest = histogram.percentile(threshold).unwrap_or_default() as i64;
+        let highest = histogram.percentile(100. - threshold).unwrap_or_default() as i64;
         let mut ignored_count = 0;
-        let mut total_count = 0;
+        let mut count = 0;
+        let mut sum = 0_u64;
         for bucket in histogram.into_iter() {
-            total_count += bucket.count();
-            if bucket.count() > 0 && (bucket.value() as i64 - mean).abs() < stddev * sigmas as i64 {
-                for _ in 0..bucket.count() {
-                    result
-                        .increment(bucket.value())
-                        .expect("Histogram failure during noise reduction");
-                }
+            if bucket.value() as i64 >= lowest && bucket.value() as i64 <= highest {
+                count += bucket.count();
+                sum += bucket.value() * bucket.count();
             } else {
                 ignored_count += bucket.count();
             }
         }
-        info!(
-            "Noise reduction: ignored {} data points out of {}, the %={:.6}",
-            ignored_count,
-            total_count,
-            ignored_count as f64 * 100. / total_count as f64
-        );
-        result
+        if count > 0 {
+            let truncated_mean = sum / count;
+            info!(
+                "Truncated mean {:.3}: ignored {} data points out of {}, the %={:.6}. TM={}µs",
+                threshold,
+                ignored_count,
+                count + ignored_count,
+                ignored_count as f64 * 100. / count as f64,
+                truncated_mean
+            );
+            truncated_mean
+        } else {
+            error!("No data points");
+            0
+        }
     }
 }
 
@@ -169,6 +161,18 @@ impl BenchRunReport {
             ("Max".to_string(), latency.maximum().unwrap_or_default()),
             ("Mean".to_string(), latency.mean().unwrap_or_default()),
             ("StdDev".to_string(), latency.stddev().unwrap_or_default()),
+            (
+                "tm95".to_string(),
+                BenchRunMetrics::truncated_mean(&latency, 5.0),
+            ),
+            (
+                "tm99".to_string(),
+                BenchRunMetrics::truncated_mean(&latency, 1.0),
+            ),
+            (
+                "tm99.9".to_string(),
+                BenchRunMetrics::truncated_mean(&latency, 0.1),
+            ),
         ]
     }
 }
@@ -395,5 +399,8 @@ mod tests {
         assert!(as_str.contains("Max"));
         assert!(as_str.contains("Mean"));
         assert!(as_str.contains("StdDev"));
+        assert!(as_str.contains("tm95"));
+        assert!(as_str.contains("tm99"));
+        assert!(as_str.contains("tm99.9"));
     }
 }

src/prometheus_reporter.rs

@@ -210,6 +210,18 @@ impl PrometheusReporter {
             ("max".to_string(), histogram.maximum().unwrap_or_default()),
             ("mean".to_string(), histogram.mean().unwrap_or_default()),
             ("stddev".to_string(), histogram.stddev().unwrap_or_default()),
+            (
+                "tm95".to_string(),
+                BenchRunMetrics::truncated_mean(&histogram, 5.0),
+            ),
+            (
+                "tm99".to_string(),
+                BenchRunMetrics::truncated_mean(&histogram, 1.0),
+            ),
+            (
+                "tm99_9".to_string(),
+                BenchRunMetrics::truncated_mean(&histogram, 0.1),
+            ),
         ];
         for (label, value) in percentiles {
             PrometheusReporter::register_gauge(
@@ -281,7 +293,7 @@ mod test {
 
         let metrics = registry.gather();
 
-        assert_eq!(10, metrics.len());
+        assert_eq!(13, metrics.len());
         assert_eq!("latency", metrics[0].get_name());
         assert_eq!("Latency of requests", metrics[0].get_help());
         assert_eq!(MetricType::HISTOGRAM, metrics[0].get_field_type());
@@ -306,6 +318,9 @@ mod test {
             "latency_max",
             "latency_mean",
             "latency_stddev",
+            "latency_tm95",
+            "latency_tm99",
+            "latency_tm99_9",
         ];
 
         precalculated.sort();