viesti / timbre-json-appender

JSON appender for Timbre
MIT License
44 stars 11 forks source link

'Semi-pretty' format for stacktraces when using :pretty #39

Closed vemv closed 4 months ago

vemv commented 6 months ago

Hi!

While the fix for https://github.com/viesti/timbre-json-appender/issues/36 was certainly an improvement, now I see stacktraces that can take a lot of vertical space:

      [
        "clojure.core$apply",
        "invokeStatic",
        "core.clj",
        667
      ],
      [
        "clojure.core$with_bindings_STAR_",
        "invokeStatic",
        "core.clj",
        1990
      ],
      [
        "clojure.core$with_bindings_STAR_",
        "doInvoke",
        "core.clj",
        1990
      ],
      [
        "clojure.lang.RestFn",
        "invoke",
        "RestFn.java",
        425
      ],

I'd suggest that for the specific case of stacktraces, each StackTraceElement takes just exactly one line.

Improved version:

["clojure.core$apply", "invokeStatic", "core.clj", 667]
["clojure.core$with_bindings_STAR_", "invokeStatic", "core.clj", 1990],
["clojure.core$with_bindings_STAR_", "doInvoke", "core.clj", 1990],
["clojure.lang.RestFn", "invoke", "RestFn.java", 425 ],

This particularly matters since stacktraces can be arbitrarily long (think of one with nested causes).

Thanks - V

viesti commented 6 months ago

Right, might be doable, maybe with custom Indenter via

diff --git a/src/timbre_json_appender/core.clj b/src/timbre_json_appender/core.clj
index 2d34570..c5f286c 100644
--- a/src/timbre_json_appender/core.clj
+++ b/src/timbre_json_appender/core.clj
@@ -152,7 +152,13 @@
                          (when pretty
                            (.setDefaultPrettyPrinter mapper
                                                      (doto (com.fasterxml.jackson.core.util.DefaultPrettyPrinter.)
-                                                       (.indentArraysWith com.fasterxml.jackson.core.util.DefaultIndenter/SYSTEM_LINEFEED_INSTANCE))))
+                                                       (.indentArraysWith (reify com.fasterxml.jackson.core.util.DefaultPrettyPrinter$Indenter
+                                                                            (isInline [_this]
+                                                                              ;; TODO
+                                                                              false)
+                                                                            (writeIndentation [_this json-generator level]
+                                                                              ;; TODO
+                                                                              (.writeRaw json-generator (clojure.string/join (repeat level " ")))))))))
                          mapper)
          data-field-processor (partial #'process-ex-data-map ex-data-field-fn)]
      (fn [{:keys [level ?ns-str ?file ?line ?err vargs ?msg-fmt hostname_ context timestamp_] :as data}]

but didn't immediately see how to detect, that the previous nesting level had an array, so the on the current nesting level, items could be printed with spaces as separators etc.

Could even be some custom serializer for throwables even...

viesti commented 6 months ago

Somekind of nerd-sniping trying to bite on writing a pretty-printer...

Got this output with the diff after the output. Could almost be there, maybe...

user> (do 
        (tas/install {:pretty true :json-error-fn (fn [e] (println e))}) 
        (println (with-out-str (log/warn (Exception. "poks") "moi"))))
{
 "ns": "user",
 "file": "*cider-repl home/timbre-json-appender:localhost:59271(clj)*",
 "err": {
  "via": [{
    "type": "java.lang.Exception",
    "message": "poks",
    "at": ["user$eval28048$fn__28049$fn__28050","invoke","NO_SOURCE_FILE",1136]
   }
],
  "trace": [
   ["user$eval28048$fn__28049$fn__28050","invoke","NO_SOURCE_FILE",1136],
   ["clojure.lang.Delay","deref","Delay.java",42],
   ["clojure.core$deref","invokeStatic","core.clj",2337],
   ["clojure.core$deref","invoke","core.clj",2323],
   ["taoensso.timbre$_log_BANG_","invokeStatic","timbre.cljc",641],
   ["taoensso.timbre$_log_BANG_","invoke","timbre.cljc",631],
   ["user$eval28048$fn__28049","invoke","NO_SOURCE_FILE",1136],
   ["user$eval28048","invokeStatic","NO_SOURCE_FILE",1136],
   ["user$eval28048","invoke","NO_SOURCE_FILE",1136],
   ["clojure.lang.Compiler","eval","Compiler.java",7194],
   ["clojure.lang.Compiler","eval","Compiler.java",7184],
   ["clojure.lang.Compiler","eval","Compiler.java",7149],
   ["clojure.core$eval","invokeStatic","core.clj",3215],
   ["clojure.core$eval","invoke","core.clj",3211],
   ["nrepl.middleware.interruptible_eval$evaluate$fn__1265$fn__1266","invoke","interruptible_eval.clj",87],
   ["clojure.lang.AFn","applyToHelper","AFn.java",152],
   ["clojure.lang.AFn","applyTo","AFn.java",144],
   ["clojure.core$apply","invokeStatic","core.clj",667],
   ["clojure.core$with_bindings_STAR_","invokeStatic","core.clj",1990],
   ["clojure.core$with_bindings_STAR_","doInvoke","core.clj",1990],
   ["clojure.lang.RestFn","invoke","RestFn.java",425],
   ["nrepl.middleware.interruptible_eval$evaluate$fn__1265","invoke","interruptible_eval.clj",87],
   ["clojure.main$repl$read_eval_print__9206$fn__9209","invoke","main.clj",437],
   ["clojure.main$repl$read_eval_print__9206","invoke","main.clj",437],
   ["clojure.main$repl$fn__9215","invoke","main.clj",458],
   ["clojure.main$repl","invokeStatic","main.clj",458],
   ["clojure.main$repl","doInvoke","main.clj",368],
   ["clojure.lang.RestFn","invoke","RestFn.java",1523],
   ["nrepl.middleware.interruptible_eval$evaluate","invokeStatic","interruptible_eval.clj",84],
   ["nrepl.middleware.interruptible_eval$evaluate","invoke","interruptible_eval.clj",56],
   ["nrepl.middleware.interruptible_eval$interruptible_eval$fn__1298$fn__1302","invoke","interruptible_eval.clj",152],
   ["clojure.lang.AFn","run","AFn.java",22],
   ["nrepl.middleware.session$session_exec$main_loop__1368$fn__1372","invoke","session.clj",218],
   ["nrepl.middleware.session$session_exec$main_loop__1368","invoke","session.clj",217],
   ["clojure.lang.AFn","run","AFn.java",22],
   ["java.lang.Thread","run","Thread.java",1583]],
  "cause": "poks"
 }
,
 "msg": "moi",
 "hostname": "aronia.local",
 "level": "warn",
 "line": 1136,
 "thread": "nREPL-session-64f779dd-e524-4d0f-809f-e9c18ef6d2fc",
 "timestamp": "2024-04-27T17:35:38Z"
}

The diff:

diff --git a/src/timbre_json_appender/core.clj b/src/timbre_json_appender/core.clj
index 2d34570..965f334 100644
--- a/src/timbre_json_appender/core.clj
+++ b/src/timbre_json_appender/core.clj
@@ -1,5 +1,6 @@
 (ns timbre-json-appender.core
-  (:require [jsonista.core :as json]
+  (:require [clojure.string :as str]
+            [jsonista.core :as json]
             [taoensso.timbre :as timbre])
   (:import (com.fasterxml.jackson.databind SerializationFeature)
            (clojure.lang ExceptionInfo)))
@@ -150,9 +151,46 @@
                        (get key-names :level))
          object-mapper (let [^com.fasterxml.jackson.databind.ObjectMapper mapper (object-mapper {:pretty pretty})]
                          (when pretty
-                           (.setDefaultPrettyPrinter mapper
-                                                     (doto (com.fasterxml.jackson.core.util.DefaultPrettyPrinter.)
-                                                       (.indentArraysWith com.fasterxml.jackson.core.util.DefaultIndenter/SYSTEM_LINEFEED_INSTANCE))))
+                           (let [nesting (atom 0)
+                                 array-nesting (atom 0)
+                                 pretty-printer (reify com.fasterxml.jackson.core.PrettyPrinter
+                                                  (writeStartObject [_ g]
+                                                    (reset! array-nesting 0)
+                                                    (.writeRaw g "{\n")
+                                                    (swap! nesting inc)
+                                                    (.writeRaw g (str/join (repeat @nesting " "))))
+                                                  (writeEndObject [_ g _nrOfEntries]
+                                                    (.writeRaw g "\n")
+                                                    (swap! nesting dec)
+                                                    (.writeRaw g (str/join (repeat @nesting " ")))
+                                                    (.writeRaw g "}\n"))
+                                                  (beforeObjectEntries [_ _g])
+                                                  (writeObjectFieldValueSeparator [_ g]
+                                                    (.writeRaw g ": "))
+                                                  (writeObjectEntrySeparator [_ g]
+                                                    (.writeRaw g ",\n")
+                                                    (.writeRaw g (str/join (repeat @nesting " "))))
+                                                  (writeStartArray [_ g]
+                                                    (if (pos? @array-nesting)
+                                                      (do
+                                                        (.writeRaw g "\n")
+                                                        (.writeRaw g (str/join (repeat @nesting " ")))
+                                                        (.writeRaw g "["))
+                                                      (.writeRaw g "["))
+                                                    (swap! nesting inc)
+                                                    (swap! array-nesting inc))
+                                                  (writeEndArray [_ g _nrOfValues]
+                                                    (.writeRaw g "]")
+                                                    (swap! array-nesting (fn [v]
+                                                                           (max 0 (dec v))))
+                                                    (swap! nesting dec))
+                                                  (beforeArrayValues [_ _g]
+                                                    )
+                                                  (writeArrayValueSeparator [_ g]
+                                                    (.writeRaw g ","))
+                                                  (writeRootValueSeparator [_ _g]
+                                                    ))]
+                             (.setDefaultPrettyPrinter mapper pretty-printer)))
                          mapper)
          data-field-processor (partial #'process-ex-data-map ex-data-field-fn)]
      (fn [{:keys [level ?ns-str ?file ?line ?err vargs ?msg-fmt hostname_ context timestamp_] :as data}]
viesti commented 6 months ago

Maybe it even works now:

user> (println (jsonista.core/write-value-as-string {:a {:b {:c 1 :d (range 10) 
                                                             :e (map (fn [x]
                                                                       (range x))
                                                                     (range 5))
                                                             :f (map (fn [x]
                                                                       (map (fn [y]
                                                                              {:y y})
                                                                            (range x)))
                                                                     (range 3))}}
                                                     :x 1} (doto (jsonista.core/object-mapper {:encode-key-fn true, :decode-key-fn true :pretty true})
                                                             (.setDefaultPrettyPrinter (timbre-json-appender.core/make-pretty-printer)))))
{
 "a": {
  "b": {
   "c": 1,
   "d": [0,1,2,3,4,5,6,7,8,9],
   "e": [
    [],
    [0],
    [0,1],
    [0,1,2],
    [0,1,2,3]],
   "f": [
    [],
    [{
      "y": 0
     }],[{
      "y": 0
     },{
      "y": 1
     }]]
  }
 },
 "x": 1
}
viesti commented 6 months ago

Threw a thing into master at c039233, maybe you can test it via git dep.

vemv commented 6 months ago

Thanks much! I'm stoked for the changes.

Perhaps you can add a flag to choose the pretty-print impl?

I reckon that some people would want the old format.

It would also make sense to me as it would be most cautious to choose the new impl in our staging environment only, for now.

Cheers - V

viesti commented 6 months ago

Added a :pretty-array option to select the new pretty-printer and pushed 0.2.14. Cheers! :)

vemv commented 6 months ago

🍻! I see it working nicely so far.