feat: add metrics for failed requests (#414)

* feat: add metrics for failed requests

* test: add tests

* test: fix test

* Update server/src/metrics/actix_web_metrics.rs

Co-authored-by: Simon Hornby <liquidwicked64@gmail.com>

* refactor: prefer map/unwrap_or over match

* refactor: simplify status_code match

* fix: no longer log status code in active_requests

---------

Co-authored-by: Simon Hornby <liquidwicked64@gmail.com>

server/src/http/unleash_client.rs

@@ -957,6 +957,7 @@ mod tests {
  assert!(validate_result.is_err());
  }
 
+ #[cfg(target_os = "linux")]
  #[test]
  pub fn can_instantiate_pkcs_12_client() {
  let pfx = "./testdata/pkcs12/snakeoil.pfx";

server/src/metrics/actix_web_metrics.rs

@@ -363,32 +363,35 @@ where
 
  let request_metrics = self.metrics.clone();
  Box::pin(self.service.call(req).map(move |res| {
- request_metrics
- .http_server_active_requests
- .add(-1, &attributes);
-
- // Ignore actix errors for metrics
- if let Ok(res) = res {
- attributes.push(HTTP_RESPONSE_STATUS_CODE.i64(res.status().as_u16() as i64));
- let response_size = res
- .response()
+ request_metrics.http_server_active_requests.add(-1, &attributes);
+
+ let status_code = match &res {
+ Ok(res) => res.status(),
+ Err(e) => e.as_response_error().status_code(),
+ }
+ .as_u16() as i64;
+
+ attributes.push(HTTP_RESPONSE_STATUS_CODE.i64(status_code));
+
+ let response_size = res
+ .as_ref()
+ .map(|res| {
+ res.response()
  .headers()
  .get(CONTENT_LENGTH)
  .and_then(|len| len.to_str().ok().and_then(|s| s.parse().ok()))
- .unwrap_or(0);
- request_metrics
- .http_server_response_size
- .record(response_size, &attributes);
-
- request_metrics.http_server_duration.record(
- timer.elapsed().map(|t| t.as_secs_f64()).unwrap_or_default(),
- &attributes,
- );
- Ok(res)
- } else {
- res
- }
- }))
+ .unwrap_or(0u64)
+ })
+ .unwrap_or(0); 
+ request_metrics.http_server_response_size.record(response_size, &attributes);
+
+ request_metrics.http_server_duration.record(
+ timer.elapsed().map(|t| t.as_secs_f64()).unwrap_or_default(),
+ &attributes,
+ );
+
+ res
+ }))
  }
 }
 
@@ -428,3 +431,72 @@ impl dev::Handler<actix_web::HttpRequest> for PrometheusMetricsHandler {
  )))
  }
 }
+
+#[cfg(test)]
+mod tests {
+ use actix_web::{test, web, App, HttpResponse, http::StatusCode};
+ use prometheus::{Encoder, Registry, TextEncoder};
+ use crate::prom_metrics;
+
+
+ async fn test_ok_endpoint() -> HttpResponse {
+ HttpResponse::Ok().append_header(("Content-length", 7)).body("Test OK")
+ }
+
+ async fn test_client_error_endpoint() -> HttpResponse {
+ HttpResponse::BadRequest().append_header(("Content-length", 17)).body("Test Client Error")
+ }
+
+ async fn test_server_error_endpoint() -> HttpResponse {
+ HttpResponse::InternalServerError().append_header(("Content-length", 17)).body("Test Server Error")
+ }
+
+ fn parse_metrics_for_status_code(metrics_output: &str, status_code: i64) -> Option<f64> {
+ metrics_output.lines()
+ .filter(|line| line.contains("http_server_response_size_bytes_sum") && line.contains(&format!("http_response_status_code=\"{}\"", status_code)))
+ .flat_map(|line| line.split_whitespace().last())
+ .flat_map(|value| value.parse::<f64>().ok())
+ .next()
+ }
+
+ #[tokio::test]
+ async fn test_middleware_response_metrics() {
+ let registry = Registry::new();
+ let (_, request_metrics) = prom_metrics::test_instantiate_without_tracing_and_logging(Some(registry.clone()));
+
+ let mut app = test::init_service(
+ App::new()
+ .wrap(request_metrics.clone())
+ .service(web::resource("/test_ok").to(test_ok_endpoint))
+ .service(web::resource("/test_client_error").to(test_client_error_endpoint))
+ .service(web::resource("/test_server_error").to(test_server_error_endpoint))
+ ).await;
+
+ let req_ok = test::TestRequest::get().uri("/test_ok").to_request();
+ let resp_ok = test::call_service(&mut app, req_ok).await;
+ assert_eq!(resp_ok.status(), StatusCode::OK);
+
+ let req_client_error = test::TestRequest::get().uri("/test_client_error").to_request();
+ let resp_client_error = test::call_service(&mut app, req_client_error).await;
+ assert_eq!(resp_client_error.status(), StatusCode::BAD_REQUEST);
+
+ let req_server_error = test::TestRequest::get().uri("/test_server_error").to_request();
+ let resp_server_error = test::call_service(&mut app, req_server_error).await;
+ assert_eq!(resp_server_error.status(), StatusCode::INTERNAL_SERVER_ERROR);
+
+ let mut buffer = Vec::new();
+ let encoder = TextEncoder::new();
+ let metric_families = registry.gather();
+ encoder.encode(&metric_families, &mut buffer).unwrap();
+ let metrics_output = String::from_utf8(buffer).unwrap();
+
+ let value_ok = parse_metrics_for_status_code(&metrics_output, 200).expect("Metric with status code 200 not found");
+ assert_eq!(value_ok, 7.0, "Metric value for status code 200 did not match expected");
+
+ let value_client_error = parse_metrics_for_status_code(&metrics_output, 400).expect("Metric with status code 400 not found");
+ assert_eq!(value_client_error, 17.0, "Metric value for status code 400 did not match expected");
+
+ let value_server_error = parse_metrics_for_status_code(&metrics_output, 500).expect("Metric with status code 500 not found");
+ assert_eq!(value_server_error, 17.0, "Metric value for status code 500 did not match expected");
+ }
+}

server/src/prom_metrics.rs

@@ -134,3 +134,10 @@ fn register_custom_metrics(registry: &prometheus::Registry) {
  ))
  .unwrap();
 }
+
+#[cfg(test)]
+pub fn test_instantiate_without_tracing_and_logging(registry: Option<prometheus::Registry>) -> (PrometheusMetricsHandler, RequestMetrics) {
+ let registry = registry.unwrap_or_else(instantiate_registry);
+ register_custom_metrics(&registry);
+ instantiate_prometheus_metrics_handler(registry)
+}