Fork me on GitHub
#pedestal
<
2020-05-27
>
hlship20:05:39

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

hlship20:05:14

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.

hlship20:05:20

Sorry I didn't notice this in the PR.

hlship20:05:29

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.

hlship20:05:20

(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)))))

ddeaguiar20:05:44

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?

ddeaguiar20:05:07

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.

hlship20:05:30

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.

hlship20:05:46

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

hlship20:05:10

My workaround seems to be working:

hlship20:05:13

(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")

ddeaguiar20:05:28

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

ddeaguiar20:05:08

Will also drop a ref to your workaround

hlship20:05:19

Thanks!

👍 4