Auto merge of #709 - Mark-Simulacrum:add-metric, r=Mark-Simulacrum

bors · bors · commit 4d35849b3abd · 2023-11-05T16:45:23.000Z
Emit metrics for record_progress endpoint Previously we were only tracking the worker time, not the endpoint. We see that there is a direct correlation with the throughput of a job and the worker time. This seems wrong to me, because as long as the worker is keeping up with the input rate, the throughput shouldn't be affected. Note that we believe that the worker should not affect the HTTP endpoint at all - we connect these with a bounded queue and pushing into the queue is done with `try_send`, which shouldn't block (https://docs.rs/crossbeam-channel/latest/crossbeam_channel/struct.Sender.html#method.try_send) and returns an error if the queue is full. We already emit a metric if the queue is full, and that's not happening here. The hope is that the extra metric here gives us some clue for what the problem is. Metric graphs: <img width="1197" alt="image" src="https://github.com/rust-lang/crater/assets/5047365/34fac874-254f-4a91-b75a-4d2d9e25aea0"> <img width="1015" alt="image" src="https://github.com/rust-lang/crater/assets/5047365/d21ea685-56ef-49bf-943b-6dbe2648f336">
diff --git a/src/server/routes/agent.rs b/src/server/routes/agent.rs
@@ -12,6 +12,7 @@ use http::Response;
 use hyper::Body;
 use std::collections::HashMap;
 use std::sync::{Arc, Condvar, Mutex};
+use std::time::Instant;
 use warp::{self, Filter, Rejection};
 
 #[derive(Deserialize)]
@@ -214,7 +215,7 @@ impl RecordProgressThread {
 
                     metrics
                         .crater_endpoint_time
-                        .with_label_values(&["record_progress"])
+                        .with_label_values(&["record_progress_worker"])
                         .observe(start.elapsed().as_secs_f64());
                 }
             }));
@@ -298,14 +299,23 @@ fn endpoint_record_progress(
     data: Arc<Data>,
     _auth: AuthDetails,
 ) -> Fallible<Response<Body>> {
-    match data.record_progress_worker.queue.try_send(result) {
+    let start = Instant::now();
+
+    let ret = match data.record_progress_worker.queue.try_send(result) {
         Ok(()) => Ok(ApiResponse::Success { result: true }.into_response()?),
         Err(crossbeam_channel::TrySendError::Full(_)) => {
             data.metrics.crater_bounced_record_progress.inc_by(1);
             Ok(ApiResponse::<()>::SlowDown.into_response()?)
         }
         Err(crossbeam_channel::TrySendError::Disconnected(_)) => unreachable!(),
-    }
+    };
+
+    data.metrics
+        .crater_endpoint_time
+        .with_label_values(&["record_progress_endpoint"])
+        .observe(start.elapsed().as_secs_f64());
+
+    ret
 }
 
 fn endpoint_heartbeat(data: Arc<Data>, auth: AuthDetails) -> Fallible<Response<Body>> {
