feat: disable trace_layer on_failure (#1608)

additionaly, switch to only emit error event for 500s in error intoresponse impls
shuttle-hq · Feb 2, 2024 · 8c6e931 · 8c6e931
1 parent a51c70c
commit 8c6e931
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 8 deletions.
diff --git 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
@@ -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<MakeSpan: tower_http::trace::MakeSpan<Body> + MakeSpanBuilder> TraceLayer<M
         MakeSpan,
         DefaultOnRequest,
         OnResponseStatusCode,
+        DefaultOnBodyChunk,
+        DefaultOnEos,
+        (),
     > {
         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
@@ -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
@@ -114,10 +114,13 @@ impl From<AcmeClientError> 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()
     }
 }