chore: disable TraceLayer on_failure log (#4315)

GreptimeTeam · Jul 8, 2024 · c0e9b3d · c0e9b3d
1 parent 59afa70
commit c0e9b3d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 11 deletions.
diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs
@@ -25,7 +25,7 @@ use common_error::define_into_tonic_status;
 use common_error::ext::{BoxedError, ErrorExt};
 use common_error::status_code::StatusCode;
 use common_macro::stack_trace_debug;
-use common_telemetry::{debug, error};
+use common_telemetry::{error, warn};
 use datatypes::prelude::ConcreteDataType;
 use headers::ContentType;
 use query::parser::PromQuery;
@@ -737,8 +737,17 @@ impl From<std::io::Error> for Error {
     }
 }
 
+fn log_error_if_necessary(error: &Error) {
+    if error.status_code().should_log_error() {
+        error!(error; "Failed to handle HTTP request ");
+    } else {
+        warn!(error; "Failed to handle HTTP request ");
+    }
+}
+
 impl IntoResponse for Error {
     fn into_response(self) -> Response {
+        use pipeline::error::Error as PipelineError;
         let error_msg = self.output_msg();
         let status = match self {
             Error::InfluxdbLineProtocol { .. }
@@ -752,16 +761,17 @@ impl IntoResponse for Error {
             | Error::InvalidPromRemoteRequest { .. }
             | Error::InvalidQuery { .. }
             | Error::TimePrecision { .. } => HttpStatusCode::BAD_REQUEST,
-            _ => {
-                if self.status_code().should_log_error() {
-                    error!(self; "Failed to handle HTTP request: ");
-                } else {
-                    debug!("Failed to handle HTTP request: {self:?}");
-                }
-
-                HttpStatusCode::INTERNAL_SERVER_ERROR
-            }
+            Error::Pipeline { ref source, .. } => match source {
+                PipelineError::CompilePipeline { .. }
+                | PipelineError::InvalidPipelineVersion { .. }
+                | PipelineError::PipelineNotFound { .. } => HttpStatusCode::BAD_REQUEST,
+                _ => HttpStatusCode::INTERNAL_SERVER_ERROR,
+            },
+            _ => HttpStatusCode::INTERNAL_SERVER_ERROR,
         };
+
+        log_error_if_necessary(&self);
+
         let body = Json(json!({
             "error": error_msg,
         }));

diff --git a/src/servers/src/http.rs b/src/servers/src/http.rs
@@ -697,7 +697,9 @@ impl HttpServer {
             .layer(
                 ServiceBuilder::new()
                     .layer(HandleErrorLayer::new(handle_error))
-                    .layer(TraceLayer::new_for_http())
+                    // disable on failure tracing. because printing out isn't very helpful,
+                    // and we have impl IntoResponse for Error. It will print out more detailed error messages
+                    .layer(TraceLayer::new_for_http().on_failure(()))
                     .option_layer(timeout_layer)
                     .option_layer(body_limit_layer)
                     // auth layer