From 13bec1c6d2997ac83f0ddb79c6862d1f76c1f856 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Fri, 2 Feb 2024 14:17:18 +0100 Subject: [PATCH] feat: disable trace_layer on_failure additionaly, switch to only emit error event for 500s in error intoresponse impls --- auth/src/error.rs | 11 +++++++++-- common/src/backends/metrics.rs | 6 +++++- deployer/src/handlers/error.rs | 12 +++++++++--- gateway/src/lib.rs | 7 +++++-- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/auth/src/error.rs b/auth/src/error.rs index 4b4fccc1f..8e3e1c38b 100644 --- a/auth/src/error.rs +++ b/auth/src/error.rs @@ -43,8 +43,15 @@ impl IntoResponse for Error { let code = match self { Error::Forbidden => StatusCode::FORBIDDEN, Error::Unauthorized | Error::KeyMissing => StatusCode::UNAUTHORIZED, - Error::Database(_) | Error::UserNotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, + Error::Database(sqlx::Error::RowNotFound) | Error::UserNotFound => { + StatusCode::NOT_FOUND + } + _ => { + // We only want to emit error events for internal errors, not e.g. 404s. + tracing::error!(error = %self, "control plane request error"); + + StatusCode::INTERNAL_SERVER_ERROR + } }; ApiError { diff --git a/common/src/backends/metrics.rs b/common/src/backends/metrics.rs index dafcb9a1b..1cb2b062b 100644 --- a/common/src/backends/metrics.rs +++ b/common/src/backends/metrics.rs @@ -9,7 +9,7 @@ use axum::http::{request::Parts, Request, Response}; use opentelemetry::global; use opentelemetry_http::HeaderExtractor; use tower_http::classify::{ServerErrorsAsFailures, SharedClassifier}; -use tower_http::trace::DefaultOnRequest; +use tower_http::trace::{DefaultOnBodyChunk, DefaultOnEos, DefaultOnRequest}; use tracing::{debug, Span}; use tracing_opentelemetry::OpenTelemetrySpanExt; @@ -83,10 +83,14 @@ impl + MakeSpanBuilder> TraceLayer { tower_http::trace::TraceLayer::new_for_http() .make_span_with(MakeSpan::new(self.fn_span)) .on_response(OnResponseStatusCode) + .on_failure(()) } } diff --git a/deployer/src/handlers/error.rs b/deployer/src/handlers/error.rs index ef51ced07..5a597ad49 100644 --- a/deployer/src/handlers/error.rs +++ b/deployer/src/handlers/error.rs @@ -42,11 +42,17 @@ impl Serialize for Error { impl IntoResponse for Error { fn into_response(self) -> Response { - error!(error = &self as &dyn std::error::Error, "request error"); - let code = match self { Error::NotFound(_) => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, + _ => { + // We only want to emit error events for internal errors, not e.g. 404s. + error!( + error = &self as &dyn std::error::Error, + "control plane request error" + ); + + StatusCode::INTERNAL_SERVER_ERROR + } }; ApiError { diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index cf81fc7dd..361dba618 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -114,10 +114,13 @@ impl From for Error { impl IntoResponse for Error { fn into_response(self) -> Response { - error!(error = %self, "request had an error"); - let error: ApiError = self.kind.into(); + if error.status_code >= 500 { + // We only want to emit error events for internal errors, not e.g. 404s. + error!(error = error.message, "control plane request error"); + } + error.into_response() } }