Clojurians Log v2

If you are accepting PRs for this, I can help. (I am not an expert on frontend technologies though, so it would be good if someone can provide meaningful feedback.)

Was it intentional in io.pedestal.logging 0.5.8 to not reference the override-logger at this line: https://github.com/pedestal/pedestal/blob/master/log/src/io/pedestal/log.clj#L277

Essentially, for my code that bridges from io.pedestal.log to our internal logging code, changing io.pedestal.log/override-logger has no effect, as that only works for the pretty much unused io.pedestal.log/log function, and not for log-expr which is where logging really takes place.

Sorry I didn't notice this in the PR.

And that puts me pack at square 1, because io.pedestal.log/debug has already been compiled from the result of log-expr before my code has a chance to either set override-logger or set the JVM system property.

(macroexpand-all '(pl/debug :from :pedestal :a 1 :b 2))
=>
(let*
 [logger2682 (. org.slf4j.LoggerFactory getLogger "user")]
 (if
  (io.pedestal.log/-level-enabled? logger2682 :debug)
  (do
   (let*
    [string2683
     (let*
      []
      (clojure.core/push-thread-bindings (clojure.core/hash-map (var clojure.core/*print-length*) 80))
      (try
       (#object[clojure.core$pr_str 0x16f7b4af "clojure.core$pr_str@16f7b4af"] {:from :pedestal, :a 1, :b 2, :line 1})
       (finally (clojure.core/pop-thread-bindings))))]
    (io.pedestal.log/-debug logger2682 string2683)))))

Hi @hlship, the only change in 0.5.8 re: logging was to the log fn. The particular code you linked hasn’t changed in years. What version of Pedestal are you migrating from?

I recall running into an issue regarding logger overriding and thought I created a GitHub issue for it but that does not seem to be the case.

There was the issue that overrideLogger couldn't be used, because it was not treated as a function, that was fixed in 0.5.8.

https://github.com/pedestal/pedestal/commit/621b6896aab6582499e0076bd9c73d5aeb01dae9

But from my perspective, it's only a partial fix, because it doesn't cover log-expr.

My workaround seems to be working:

(def logger (memoize logger*))

;; This actually doesn't accomplish anything in Pedestal 0.5.8

(alter-var-root #'io.pedestal.log/override-logger (fn [_] logger))

(System/setProperty  "io.pedestal.log.overrideLogger" "com.walmartlabs.log.pedestal/logger")

Ah I see. Ok going to create another issue which refs #638. I’ll prioritize that one

Will also drop a ref to your workaround

Thanks!