Add caching & http status code information to metrics

Author: TymanWasTaken

src/api/v1/caching.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use actix_web::{
 	HttpResponse,
 	body::{BoxBody, EitherBody, MessageBody},
@@ -6,26 +8,41 @@ use actix_web::{
 	middleware::Next,
 	web
 };
+use prometheus_client::encoding::EncodeLabelSet;
 use sha2::{Digest as _, Sha256};
 
 use super::{ApiData, CacheKey, CacheValue};
 
+#[derive(Debug, Hash, PartialEq, Eq, Clone, EncodeLabelSet)]
+pub struct CacheLabels {
+	endpoint: String
+}
+
 pub async fn middleware(
 	service_request: ServiceRequest,
 	next: Next<impl MessageBody>
 ) -> Result<ServiceResponse<EitherBody<impl MessageBody>>, actix_web::Error> {
-	let cache = match service_request.app_data::<web::Data<ApiData>>() {
-		Some(app_data) => app_data.cache.clone(),
-		None => {
-			// If we don't have ApiData for whatever reason, we can't do much
-			// cache-related Technically this could probably be an unwrap, but this is
-			// cleaner
-			return next
-				.call(service_request)
-				.await
-				.map(|resp| resp.map_into_left_body());
-		}
+	let match_pattern = service_request.match_pattern().unwrap_or("default".to_string());
+	let Some(app_data) = service_request.app_data::<web::Data<ApiData>>() else {
+		// If we don't have ApiData for whatever reason, we can't do much
+		// cache-related Technically this could probably be an unwrap, but this is
+		// cleaner
+		return next
+			.call(service_request)
+			.await
+			.map(|resp| resp.map_into_left_body());
 	};
+
+	// Check whether the endpoint should actually be cached
+	if !app_data.cache_allowlist.contains(match_pattern.as_str()) {
+		return next
+			.call(service_request)
+			.await
+			.map(|resp| resp.map_into_left_body());
+	}
+
+	let cache = app_data.cache.clone();
+	let metric_labels = CacheLabels { endpoint: match_pattern };
 	let cache_key = CacheKey {
 		path: service_request.path().to_string(),
 		query: service_request.query_string().to_string()
@@ -44,6 +61,9 @@ pub async fn middleware(
 
 	// Resolve cache entry with path & query
 	if let Some(cache_value) = cache.get(&cache_key).await {
+		// Record a cache hit to the metrics
+		app_data.metrics.cache_hits.get_or_create(&metric_labels).inc();
+
 		// Short circuit with HttpResponse::NotModified() if the If-None-Match header
 		// matches cache
 		if let Some((if_none_match, etag)) = if_none_match
@@ -74,6 +94,9 @@ pub async fn middleware(
 		}
 
 		return Ok(service_request.into_response(res).map_into_right_body());
+	} else {
+		// Record a cache miss to the metrics
+		app_data.metrics.cache_misses.get_or_create(&metric_labels).inc();
 	}
 
 	// If none of the caching cases were handled, pass through to other handlers

src/api/v1/metrics.rs

@@ -15,8 +15,7 @@ use prometheus_client::{
 };
 
 use crate::api::v1::{
-	ApiData,
-	endpoints::artifacts::{ArtifactQuery, OneConfigVersionInfo}
+	caching::CacheLabels, endpoints::artifacts::{ArtifactQuery, OneConfigVersionInfo}, ApiData
 };
 
 /// A macro that automatically initializes and registers a metric using inferred
@@ -37,6 +36,12 @@ macro_rules! make_api_metric {
 	};
 }
 
+#[derive(Debug, Hash, PartialEq, Eq, Clone, EncodeLabelSet)]
+struct ApiRequestLabels {
+	path: String,
+	status_code: u16
+}
+
 #[derive(Debug, Hash, PartialEq, Eq, Clone, EncodeLabelSet)]
 struct PlatformAgnosticArtifactLabels {
 	r#type: String
@@ -47,10 +52,16 @@ struct PlatformAgnosticArtifactLabels {
 pub struct ApiMetrics {
 	/// The registry used for storing metrics
 	registry: Registry,
+	/// The generic amount of API requests by path and response code
+	api_requests: Family<ApiRequestLabels, Counter>,
 	/// The amount of OneConfig artifacts requests, by version and loader
 	oneconfig_artifacts_requests: Family<OneConfigVersionInfo, Counter>,
 	/// The amount of platform-agnostic artifacts requests, by type
-	platform_agnostic_artifacts_requests: Family<PlatformAgnosticArtifactLabels, Counter>
+	platform_agnostic_artifacts_requests: Family<PlatformAgnosticArtifactLabels, Counter>,
+	/// The amount of cache hits by endpoint
+	pub cache_hits: Family<CacheLabels, Counter>,
+	/// The amount of cache misses by endpoint
+	pub cache_misses: Family<CacheLabels, Counter>
 }
 
 /// Configures the metrics endpoint. In addition to this, the metrics middleware
@@ -66,13 +77,19 @@ pub fn configure() -> impl FnOnce(&mut ServiceConfig) {
 pub fn init_metrics() -> ApiMetrics {
 	let mut registry = <Registry>::default();
 
+	make_api_metric!(registry, api_requests);
 	make_api_metric!(registry, oneconfig_artifacts_requests);
 	make_api_metric!(registry, platform_agnostic_artifacts_requests);
+	make_api_metric!(registry, cache_hits);
+	make_api_metric!(registry, cache_misses);
 
 	ApiMetrics {
 		registry,
+		api_requests,
 		oneconfig_artifacts_requests,
-		platform_agnostic_artifacts_requests
+		platform_agnostic_artifacts_requests,
+		cache_hits,
+		cache_misses
 	}
 }
 
@@ -96,37 +113,50 @@ pub async fn middleware(
 	mut service_request: ServiceRequest,
 	next: Next<impl MessageBody>
 ) -> Result<ServiceResponse<impl MessageBody>, actix_web::Error> {
-	if let Some(pattern) = service_request.match_pattern() {
-		let data = service_request.extract::<web::Data<ApiData>>().await?;
-
-		match pattern.as_str() {
-			"/v1/artifacts/oneconfig" => {
-				data.metrics
-					.oneconfig_artifacts_requests
-					.get_or_create(
-						&service_request
-							.extract::<web::Query<ArtifactQuery<OneConfigVersionInfo>>>()
-							.await?
-							.version_info
-					)
-					.inc();
-			}
-			"/v1/artifacts/{artifact:stage1|relaunch}" => {
-				data.metrics
-					.platform_agnostic_artifacts_requests
-					.get_or_create(&PlatformAgnosticArtifactLabels {
-						// Unfortunately actix makes it difficult to extract the real
-						// parsed URL parameter, so just substring instead as a substitute
-						r#type: service_request.uri().path()
-							[const { "/v1/artifacts/".len() }..]
-							.to_string()
-					})
-					.inc();
-			}
-			_ => ()
-		};
-	}
+	let data = service_request.extract::<web::Data<ApiData>>().await?;
+
+	match service_request.match_pattern().unwrap_or("default".to_string()).as_str() {
+		"/v1/artifacts/oneconfig" => {
+			data.metrics
+				.oneconfig_artifacts_requests
+				.get_or_create(
+					&service_request
+						.extract::<web::Query<ArtifactQuery<OneConfigVersionInfo>>>()
+						.await?
+						.version_info
+				)
+				.inc();
+		}
+		"/v1/artifacts/{artifact:stage1|relaunch}" => {
+			data.metrics
+				.platform_agnostic_artifacts_requests
+				.get_or_create(&PlatformAgnosticArtifactLabels {
+					// Unfortunately actix makes it difficult to extract the real
+					// parsed URL parameter, so just substring instead as a substitute
+					r#type: service_request.uri().path()
+						[const { "/v1/artifacts/".len() }..]
+						.to_string()
+				})
+				.inc();
+		}
+		_ => ()
+	};
 
 	// Let the real request handler continue
-	return next.call(service_request).await;
+	let path = service_request.uri().path().to_string();
+	let response = next.call(service_request).await;
+
+	let labels = match &response {
+		Ok(r) => ApiRequestLabels {
+			path,
+			status_code: r.status().as_u16()
+		},
+		Err(e) => ApiRequestLabels {
+			path,
+			status_code: e.as_response_error().status_code().as_u16()
+		}
+	};
+	data.metrics.api_requests.get_or_create(&labels).inc();
+
+	response
 }

src/api/v1/mod.rs

@@ -3,7 +3,7 @@ pub mod endpoints;
 pub mod metrics;
 pub mod responses;
 
-use std::sync::Arc;
+use std::{collections::HashSet, sync::Arc};
 
 use actix_web::{
 	http::header::HeaderMap,
@@ -35,10 +35,12 @@ pub struct ApiData {
 	pub internal_maven_url: Option<String>,
 	/// A reqwest client to use to fetch maven data
 	pub client: Arc<reqwest::Client>,
+	/// The allowlist of paths that should be cached
+	pub cache_allowlist: HashSet<&'static str>,
 	/// The internal cache used to cache artifact responses.
 	pub cache: Cache<CacheKey, CacheValue>,
 	/// All the metrics objects used for encoding and recording metrics
-	pub metrics: ApiMetrics
+	pub metrics: ApiMetrics,
 }
 
 pub fn configure() -> impl FnOnce(&mut ServiceConfig) {

src/main.rs

@@ -4,7 +4,7 @@ mod api;
 mod maven;
 mod types;
 
-use std::{net::SocketAddr, time::Duration};
+use std::{collections::HashSet, net::SocketAddr, time::Duration};
 
 use actix_web::{App, HttpServer, web};
 use api::v1::{ApiData, CacheKey, CacheValue, ETagType};
@@ -66,6 +66,10 @@ async fn main() {
 			.build()
 			.unwrap()
 			.into(),
+		cache_allowlist: HashSet::from([
+			"/v1/artifacts/oneconfig",
+			"/v1/artifacts/{artifact:stage1|relaunch}"
+		]),
 		cache: Cache::builder()
 			.time_to_live(Duration::from_mins(2))
 			.weigher(|k: &CacheKey, v: &CacheValue| {