viesti / timbre-json-appender

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

Adds pre-processor functionality for ex-info data map fields #23

Closed coyotesqrl closed 2 years ago

coyotesqrl commented 2 years ago

My team encountered a problem this week when logging exceptions including non-Serializable data in their maps. Specifically, when clj-http is logging an exception, it adds the org.apache.http.impl.client.InternalHttpClient it's using into the :http-client field of the ex-info's data. As that class is not Serializable, it fails JSON parsing and uses the fallback logger. We still get all the log data we want, but lose the formatting and end up with many log entries instead of a singular entry.

We have a quick fix for this for ourselves - a sanitizer that walks the exception chain and filters out any non-Serializable data values - but I thought a more general purpose solution might be useful. To that end, I created this PR.

It doesn't work the same as our current solution, in that it cannot be used to filter out entries from the map; however, I think it's preferred that when logging exceptions the knowledge that something was there would be better anyway.

The default behavior I provided works exactly as it does today. For the simplest sanitization, configuring the appender like this would be sufficient:

(tas/install {:data-field-fn (fn [f] (when (instance? java.io.Serializable f) f))})
viesti commented 2 years ago

Whoa, this is really neat, thank you! :) I've been hit by the same thing and nice to see people contributing always better solutions :)

Took a really brief look, it looks nice, but two things that I'm thinking about:

(defn- process-ex-data-map [data-field-fn ex]
  (if (and ex (instance? clojure.lang.ExceptionInfo ex))
    (let [cause (process-ex-data-map data-field-fn (ex-cause ex))]
      (ex-info (ex-message ex)
               (into {} (map (fn [[k v]] {k (data-field-fn v)}) (ex-data ex)))
               cause))
    ex))

Anyways, thank you! :)

coyotesqrl commented 2 years ago

Requested changes made.

viesti commented 2 years ago

Thank you! :)

I think I can add a couple of tests and tweak some docstrings after merging :)

viesti commented 2 years ago

Decided to release as 0.2.4. Thank you! :)