adds the error class into the request log (#992)

* adds the error class into the request log

* updates the docstring

waiter/src/waiter/process_request.clj

@@ -62,13 +62,14 @@
 (defn- classify-error
   "Classifies the error responses from the backend into the following vector:
    - error cause (:client-eagerly-closed, :client-error, :instance-error or :generic-error),
-   - associated error message, and
-   - the http status code."
+   - associated error message,
+   - the http status code, and
+   - the canonical name of the exception that 'caused' the error."
   [error]
   (let [classification (cond (instance? ExceptionInfo error)
-                             (let [[error-cause message status] (classify-error (ex-cause error))
+                             (let [[error-cause message status error-class] (classify-error (ex-cause error))
                                    error-cause (or (-> error ex-data :error-cause) error-cause)]
-                               [error-cause message status])
+                               [error-cause message status error-class])
                              (instance? IllegalStateException error)
                              [:generic-error (.getMessage error) 400]
                              ;; cancel_stream_error is used to indicate that the stream is no longer needed
@@ -83,9 +84,10 @@
                              [:instance-error (utils/message :backend-request-timed-out) 504]
                              :else
                              [:instance-error (utils/message :backend-request-failed) 502])
-        [error-cause _ _] classification]
-    (log/info (-> error .getClass .getCanonicalName) (.getMessage error) "identified as" error-cause)
-    classification))
+        [error-cause _ _] classification
+        error-class (-> error .getClass .getCanonicalName)]
+    (log/info error-class (.getMessage error) "identified as" error-cause)
+    (conj classification error-class)))
 
 (defn- determine-client-error
   "Classifies the error into one of :client-eagerly-closed or :client-error"
@@ -233,10 +235,13 @@
 (defn- handle-response-error
   "Handles error responses from the backend."
   [error reservation-status-promise service-id request]
-  (let [[error-cause message status] (classify-error error)
-        metrics-map (metrics/retrieve-local-stats-for-service service-id)]
+  (let [[error-cause message status error-class] (classify-error error)
+        metrics-map (metrics/retrieve-local-stats-for-service service-id)
+        error-map (assoc metrics-map 
+                    :error-class error-class
+                    :status status)]
     (deliver reservation-status-promise error-cause)
-    (utils/exception->response (ex-info message (assoc metrics-map :status status) error) request)))
+    (utils/exception->response (ex-info message error-map error) request)))
 
 (let [min-buffer-size 1024
       max-buffer-size 32768

waiter/src/waiter/request_log.clj

@@ -51,9 +51,9 @@
 
 (defn response->context
   "Convert a response into a context suitable for logging."
-  [{:keys [authorization/method authorization/principal backend-response-latency-ns descriptor latest-service-id
-           get-instance-latency-ns handle-request-latency-ns headers instance instance-proto protocol status
-           waiter-api-call?] :as response}]
+  [{:keys [authorization/method authorization/principal backend-response-latency-ns descriptor error-class
+           get-instance-latency-ns handle-request-latency-ns headers instance instance-proto latest-service-id
+           protocol status waiter-api-call?] :as response}]
   (let [{:keys [service-id service-description source-tokens]} descriptor
         token (some->> source-tokens (map #(get % "token")) seq (str/join ","))
         {:strs [metric-group run-as-user version]} service-description
@@ -84,7 +84,8 @@
       handle-request-latency-ns (assoc :handle-request-latency-ns handle-request-latency-ns)
       location (assoc :response-location location)
       token (assoc :token token)
-      (some? waiter-api-call?) (assoc :waiter-api waiter-api-call?))))
+      (some? waiter-api-call?) (assoc :waiter-api waiter-api-call?)
+      error-class (assoc :waiter-error-class error-class))))
 
 (defn log-request!
   "Log a request"

waiter/src/waiter/util/utils.clj

@@ -307,10 +307,16 @@
     (hu/grpc? headers client-protocol)
     (add-grpc-headers-and-trailers error-context)))
 
+(defn attach-error-class
+  "Attaches error-class on Waiter generated responses when it is available in the provided error data."
+  [response {:keys [error-class]}]
+  (cond-> response
+    error-class (assoc :error-class error-class)))
+
 (defn data->error-response
   "Converts the provided data map into a ring response.
    The data map is expected to contain the following keys: details, headers, message, and status."
-  [{:keys [headers status] :or {status 400} :as data-map} request]
+  [{:keys [details headers status] :or {status 400} :as data-map} request]
   (let [error-context (build-error-context data-map request)
         content-type (request->content-type request)]
     (-> {:body (case content-type
@@ -322,17 +328,18 @@
          :headers (-> headers
                     (assoc-if-absent "content-type" content-type))
          :status status}
+      (attach-error-class details)
       (attach-grpc-status error-context request)
       attach-waiter-source)))
 
 (defn- wrap-unhandled-exception
   "Wraps any exception that doesn't already set status in a parent
   exception with a generic error message and a 500 status."
   [ex]
-  (let [{:keys [status]} (ex-data ex)]
+  (let [{:keys [status] :as error-data} (ex-data ex)]
     (if status
       ex
-      (ex-info (str "Internal error: " (.getMessage ex)) {:status 500} ex))))
+      (ex-info (str "Internal error: " (.getMessage ex)) (assoc error-data :status 500) ex))))
 
 (defn exception->response
   "Converts an exception into a ring response."

waiter/test/waiter/request_log_test.clj

@@ -64,6 +64,7 @@
                                                      "version" "service-version"}
                                :source-tokens [{"token" "test-token1" "version" "E-1234"}
                                                {"token" "test-token2" "version" "E-4321"}]}
+                  :error-class "java.lang.Exception"
                   :get-instance-latency-ns 500
                   :handle-request-latency-ns 2000
                   :headers {"content-length" "40"
@@ -106,7 +107,8 @@
             :service-version "service-version"
             :status 200
             :token "test-token1,test-token2"
-            :waiter-api false}
+            :waiter-api false
+            :waiter-error-class "java.lang.Exception"}
            (response->context response)))))
 
 (deftest test-wrap-log