BrunoBonacci / safely

Safely is a Clojure's circuit-breaker library for handling retries in an elegant declarative way.
https://cljdoc.org/d/com.brunobonacci/safely
Apache License 2.0
207 stars 9 forks source link

Feature request: Custom callback function #4

Closed wontheone1 closed 4 years ago

wontheone1 commented 4 years ago
;; set to false if you don't want to log errors
 :log-errors false

 ;; or choose the logging level
 :log-level :warn

 ;; to disable the stacktrace reporting in the logs
 :log-stacktrace false

I see there is API for custom logging but I want to provide Clojure map for logging and let my configuration for timbre format it (JSON with some error handling while encoding). And there could be other use cases for custom call back function besides logging. To satisfy various need to do something (including logging or anything) between retries, api for providing :callback-fn would be useful. (that is what another retrying library again is doing)

e.g.

:callback-fn (fn [] (println "I wanna do this between retries"))

Does it make sense?

wontheone1 commented 4 years ago

The reason the existing log related options are not enough is in our team we provide Clojure map to timbre and then it is formatted with the following configuration.

(defn format-logline [{:keys [level ?ns-str ?msg-fmt vargs ?err]}]
  (let [log-data (merge {:level  (clojure.string/upper-case (name level))
                         :thread (.getName (Thread/currentThread))
                         :ns     ?ns-str}
                        (when ?err
                          {:error      (str ?err)
                           :stacktrace (timbre/stacktrace ?err {:stacktrace-fonts nil})})
                        (cond
                          ?msg-fmt {:msg (apply format ?msg-fmt vargs)}
                          (map? (first vargs)) (first vargs)
                          :else {:msg (clojure.string/join vargs)}))]
    (try
      (cheshire/generate-string log-data)
      (catch JsonGenerationException _
        (str log-data)))))

(timbre/merge-config! {:level     :info
                       :output-fn format-logline})
BrunoBonacci commented 4 years ago

Hi @wontheone1 ,

the design goal of safely is to manage declaratively retries and error handling. For this reason I'm not keen on adding custom callbacks which can interfere with the declarative approach. What happens if the callback itself fails? what happens if it blocks? how does this interfere with timeouts and circuit breakers? A similar discussion as https://github.com/BrunoBonacci/safely/issues/1 which was more about monitoring.

Safely uses clojure.tools.logging which is just a logging facade like SLF4j. You can pick any concrete logger implementation, including Timbre.

If what you are looking for is structured logging or even better event logging then I suggest you look into µ/log. I'm planning to integrate µ/log inside safely soon, which will provide loads of advantages from analytics and monitoring perspective.

Additionally, you will be able to subscribe to µ/log events and do whatever you want with the events which are just value (maps). Chances are that probably µ/log already does all you need.

However, if you still want to have your "special" logging it is fairly easy to write a small macro that would do what you want.

here an example:

(defn mylog [& args]
    (apply println args))

  (defmacro my-safely
    [& forms]
    (let [[body _ options :as seg] (partition-by #{:on-error} forms)]
      `(safely
           (try
             ~@body
             (catch Exception x#
               (mylog "my special logging" x#)
               (throw x#)))
         :on-error
         :log-errors false
         ~@options)))

Now you can use my-safely like the normal one with the same behaviour + your logging.

  (my-safely
   (/ 1 0)
   :on-error
   :default 1)

I hope this help. Best regards Bruno

wontheone1 commented 4 years ago

@BrunoBonacci What you said makes a whole lot sense. I agree then we don't need a custom callback. If you get this feature request often like mine and #1, you might as well document the reasoning in the Readme.md so that you don't have to answer many times. I will look into how to fulfill my logging need and mulog library. I might come back with a few questions but thank you for explaining the reasoning and suggesting alternatives!

BrunoBonacci commented 4 years ago

Hi @wontheone1, Thanks for the suggestion, I will add a section in doc for this.

Bruno