From f3bee243d1004867d2e5c7504726c93a42bd3c80 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 3 Mar 2026 16:09:06 +0100 Subject: [PATCH 1/3] feat(server): Track request rejections with a unified server.rejected metric Add a RejectionReason enum that classifies why a request was rejected (killswitch, rate_limit, auth, bad_request, internal, task_concurrency, web_concurrency) and emits a server.rejected counter tagged with the reason on every rejection. Previously, killswitch and rate limit rejections were only logged at debug level with no metric. The web concurrency limit had its own standalone web.concurrency.rejected counter. Now all rejection paths emit a single server.rejected metric that can be summed or split by reason. Refs FS-275 Co-Authored-By: Claude --- objectstore-server/src/endpoints/common.rs | 40 +++++++++++++++++++ objectstore-server/src/extractors/id.rs | 13 ++++++ objectstore-server/src/lib.rs | 1 + objectstore-server/src/rejection.rs | 46 ++++++++++++++++++++++ objectstore-server/src/web/middleware.rs | 3 +- 5 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 objectstore-server/src/rejection.rs diff --git a/objectstore-server/src/endpoints/common.rs b/objectstore-server/src/endpoints/common.rs index dd2bd4e7..26d9364c 100644 --- a/objectstore-server/src/endpoints/common.rs +++ b/objectstore-server/src/endpoints/common.rs @@ -11,6 +11,7 @@ use thiserror::Error; use crate::auth::AuthError; use crate::extractors::batch::BatchError; +use crate::rejection::RejectionReason; /// Error type for API operations. #[derive(Debug, Error)] @@ -100,8 +101,47 @@ impl ApiError { } } +impl ApiError { + /// Returns the [`RejectionReason`] for this error, used to emit rejection metrics. + pub fn rejection_reason(&self) -> RejectionReason { + match self { + ApiError::Client(_) => RejectionReason::BadRequest, + + ApiError::Auth(AuthError::BadRequest(_)) => RejectionReason::BadRequest, + ApiError::Auth( + AuthError::ValidationFailure(_) + | AuthError::VerificationFailure + | AuthError::NotPermitted, + ) => RejectionReason::Auth, + ApiError::Auth(AuthError::InternalError(_)) => RejectionReason::Internal, + + ApiError::Batch( + BatchError::BadRequest(_) + | BatchError::Multipart(_) + | BatchError::Metadata(_) + | BatchError::LimitExceeded(_), + ) => RejectionReason::BadRequest, + ApiError::Batch(BatchError::RateLimited) => RejectionReason::RateLimit, + ApiError::Batch(BatchError::ResponseSerialization { .. }) => RejectionReason::Internal, + + ApiError::Service(ServiceError::Metadata(_)) => RejectionReason::BadRequest, + ApiError::Service(ServiceError::AtCapacity) => RejectionReason::TaskConcurrency, + ApiError::Service( + ServiceError::Io(_) + | ServiceError::Serde { .. } + | ServiceError::Reqwest { .. } + | ServiceError::GcpAuth(_) + | ServiceError::Panic(_) + | ServiceError::Dropped + | ServiceError::Generic { .. }, + ) => RejectionReason::Internal, + } + } +} + impl IntoResponse for ApiError { fn into_response(self) -> Response { + self.rejection_reason().emit(); let body = ApiErrorResponse::from_error(&self); (self.status(), Json(body)).into_response() } diff --git a/objectstore-server/src/extractors/id.rs b/objectstore-server/src/extractors/id.rs index a38e3fdb..672f2e9d 100644 --- a/objectstore-server/src/extractors/id.rs +++ b/objectstore-server/src/extractors/id.rs @@ -10,6 +10,7 @@ use serde::{Deserialize, de}; use crate::extractors::Xt; use crate::extractors::downstream_service::DownstreamService; +use crate::rejection::RejectionReason; use crate::state::ServiceState; #[derive(Debug)] @@ -19,8 +20,20 @@ pub enum ObjectRejection { RateLimited, } +impl ObjectRejection { + /// Returns the [`RejectionReason`] for this rejection, used to emit rejection metrics. + pub fn rejection_reason(&self) -> RejectionReason { + match self { + ObjectRejection::Path(_) => RejectionReason::BadRequest, + ObjectRejection::Killswitched => RejectionReason::Killswitch, + ObjectRejection::RateLimited => RejectionReason::RateLimit, + } + } +} + impl IntoResponse for ObjectRejection { fn into_response(self) -> Response { + self.rejection_reason().emit(); match self { ObjectRejection::Path(rejection) => rejection.into_response(), ObjectRejection::Killswitched => ( diff --git a/objectstore-server/src/lib.rs b/objectstore-server/src/lib.rs index 50328baf..cf0bb745 100644 --- a/objectstore-server/src/lib.rs +++ b/objectstore-server/src/lib.rs @@ -13,5 +13,6 @@ pub mod killswitches; pub mod multipart; pub mod observability; pub mod rate_limits; +pub mod rejection; pub mod state; pub mod web; diff --git a/objectstore-server/src/rejection.rs b/objectstore-server/src/rejection.rs new file mode 100644 index 00000000..7c7a2ab2 --- /dev/null +++ b/objectstore-server/src/rejection.rs @@ -0,0 +1,46 @@ +//! Rejection reason classification and metrics emission. +//! +//! [`RejectionReason`] classifies why a request was rejected, providing a single +//! point for emitting the `server.rejected` counter metric with a `reason` tag. + +/// Classifies the reason a request was rejected by the server. +/// +/// Used to emit a unified `server.rejected` counter tagged with `reason`, +/// providing visibility into each rejection category. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RejectionReason { + /// Request was blocked by a configured killswitch. + Killswitch, + /// Request exceeded a throughput or bandwidth rate limit. + RateLimit, + /// Request failed authentication or authorization checks. + Auth, + /// An internal server error prevented the request from being handled. + Internal, + /// Request was malformed or violated API constraints. + BadRequest, + /// The backend task concurrency limit was reached. + TaskConcurrency, + /// The web server in-flight request concurrency limit was reached. + WebConcurrency, +} + +impl RejectionReason { + /// Returns the string used as the `reason` metric tag value. + pub fn as_str(&self) -> &'static str { + match self { + RejectionReason::Killswitch => "killswitch", + RejectionReason::RateLimit => "rate_limit", + RejectionReason::Auth => "auth", + RejectionReason::Internal => "internal", + RejectionReason::BadRequest => "bad_request", + RejectionReason::TaskConcurrency => "task_concurrency", + RejectionReason::WebConcurrency => "web_concurrency", + } + } + + /// Emits a `server.rejected` counter tagged with this rejection reason. + pub fn emit(&self) { + merni::counter!("server.rejected": 1, "reason" => self.as_str()); + } +} diff --git a/objectstore-server/src/web/middleware.rs b/objectstore-server/src/web/middleware.rs index 1ed50229..046304b9 100644 --- a/objectstore-server/src/web/middleware.rs +++ b/objectstore-server/src/web/middleware.rs @@ -10,6 +10,7 @@ use tokio::time::Instant; use tower_http::set_header::SetResponseHeaderLayer; use crate::endpoints::is_internal_route; +use crate::rejection::RejectionReason; use crate::web::RequestCounter; /// The value for the `Server` HTTP header. @@ -29,7 +30,7 @@ pub async fn limit_web_concurrency( let route = matched_path.as_ref().map_or("unknown", |m| m.as_str()); if !is_internal_route(route) && counter.count() >= counter.limit() { - merni::counter!("web.concurrency.rejected": 1); + RejectionReason::WebConcurrency.emit(); return StatusCode::SERVICE_UNAVAILABLE.into_response(); } From 43d7fbcc8db7185cc2e8f1000707e508f32697a5 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 3 Mar 2026 16:32:06 +0100 Subject: [PATCH 2/3] fix(server): Emit server.rejected for batch per-item errors and invalid multipart Two gaps in the batch flow were missed in the initial implementation: - Per-item errors in the batch endpoint passed through create_error_part, which called error.status() and serialized the body but never called into_response(), so rejection_reason().emit() was never reached. Fixed by calling error.rejection_reason().emit() directly in create_error_part. - Invalid multipart requests (missing Content-Type boundary) were rejected by Axum via MultipartRejection's own IntoResponse, bypassing ApiError entirely. Fixed by introducing BatchStreamRejection, a thin newtype wrapper that emits RejectionReason::BadRequest before delegating to the original rejection response. Refs FS-275 Co-Authored-By: Claude --- objectstore-server/src/endpoints/batch.rs | 1 + objectstore-server/src/extractors/batch.rs | 24 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/objectstore-server/src/endpoints/batch.rs b/objectstore-server/src/endpoints/batch.rs index f07adf55..788dc2c7 100644 --- a/objectstore-server/src/endpoints/batch.rs +++ b/objectstore-server/src/endpoints/batch.rs @@ -269,6 +269,7 @@ fn create_success_part( } fn create_error_part(idx: usize, error: &ApiError) -> Part { + error.rejection_reason().emit(); let mut headers = HeaderMap::new(); insert_index_header(&mut headers, idx); insert_status_header(&mut headers, error.status()); diff --git a/objectstore-server/src/extractors/batch.rs b/objectstore-server/src/extractors/batch.rs index 4f84f677..9e1d3bfd 100644 --- a/objectstore-server/src/extractors/batch.rs +++ b/objectstore-server/src/extractors/batch.rs @@ -9,12 +9,14 @@ use axum::extract::{ FromRequest, Multipart, Request, multipart::{Field, MultipartError, MultipartRejection}, }; +use axum::response::{IntoResponse, Response}; use futures::{StreamExt, stream::BoxStream}; use objectstore_service::streaming::{Delete, Get, Insert, Operation}; use objectstore_types::metadata::Metadata; use thiserror::Error; use crate::batch::{HEADER_BATCH_OPERATION_KEY, HEADER_BATCH_OPERATION_KIND}; +use crate::rejection::RejectionReason; /// Errors that can occur when processing or executing batch operations. #[derive(Debug, Error)] @@ -124,6 +126,26 @@ async fn try_operation_from_field(field: Field<'_>) -> Result for BatchStreamRejection { + fn from(r: MultipartRejection) -> Self { + Self(r) + } +} + +impl IntoResponse for BatchStreamRejection { + fn into_response(self) -> Response { + RejectionReason::BadRequest.emit(); + self.0.into_response() + } +} + /// A lazily-parsed stream of batch operations extracted from a multipart request body. pub struct BatchOperationStream(pub BoxStream<'static, Result>); @@ -140,7 +162,7 @@ impl FromRequest for BatchOperationStream where S: Send + Sync, { - type Rejection = MultipartRejection; + type Rejection = BatchStreamRejection; async fn from_request(request: Request, state: &S) -> Result { let mut multipart = Multipart::from_request(request, state).await?; From 7de10f2b00a0dd75d968689126559b9bee4a48b8 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 3 Mar 2026 18:39:31 +0100 Subject: [PATCH 3/3] fix(server): Rename server.rejected to server.requests.rejected Aligns with the server.requests.* metric family. Reverts per-operation emit from create_error_part to keep the metric strictly request-level. Refs FS-275 Co-Authored-By: Claude --- objectstore-server/src/endpoints/batch.rs | 1 - objectstore-server/src/rejection.rs | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/objectstore-server/src/endpoints/batch.rs b/objectstore-server/src/endpoints/batch.rs index 2c4757a7..7b5b7240 100644 --- a/objectstore-server/src/endpoints/batch.rs +++ b/objectstore-server/src/endpoints/batch.rs @@ -269,7 +269,6 @@ fn create_success_part( } fn create_error_part(idx: usize, error: &ApiError) -> Part { - error.rejection_reason().emit(); let mut headers = HeaderMap::new(); insert_index_header(&mut headers, idx); insert_status_header(&mut headers, error.status()); diff --git a/objectstore-server/src/rejection.rs b/objectstore-server/src/rejection.rs index c0d63805..fbc3a145 100644 --- a/objectstore-server/src/rejection.rs +++ b/objectstore-server/src/rejection.rs @@ -1,11 +1,11 @@ //! Rejection reason classification and metrics emission. //! //! [`RejectionReason`] classifies why a request was rejected, providing a single -//! point for emitting the `server.rejected` counter metric with a `reason` tag. +//! point for emitting the `server.requests.rejected` counter metric with a `reason` tag. /// Classifies the reason a request was rejected by the server. /// -/// Used to emit a unified `server.rejected` counter tagged with `reason`, +/// Used to emit a unified `server.requests.rejected` counter tagged with `reason`, /// providing visibility into each rejection category. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RejectionReason { @@ -39,8 +39,8 @@ impl RejectionReason { } } - /// Emits a `server.rejected` counter tagged with this rejection reason. + /// Emits a `server.requests.rejected` counter tagged with this rejection reason. pub fn emit(&self) { - objectstore_metrics::counter!("server.rejected": 1, "reason" => self.as_str()); + objectstore_metrics::counter!("server.requests.rejected": 1, "reason" => self.as_str()); } }