[nop] Refresh tools-logging code

taoensso · Aug 29, 2024 · ae5d0d8 · ae5d0d8
1 parent 7b7d009
commit ae5d0d8
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 44 deletions.
diff --git a/src/taoensso/timbre.cljc b/src/taoensso/timbre.cljc
@@ -444,16 +444,17 @@
   default-output-msg-fn
   default-output-error-fn)
 
-(defn -log! "Core low-level log fn. Implementation detail!"
+(defn ^:no-doc -log!
+  "Core low-level log fn. Private, don't use!"
 
   ;; Back compatible arities for convenience of AOT tools,
-  ;; Ref. https://github.com/fzakaria/slf4j-timbre/issues/20
-  ([config level ?ns-str ?file ?line msg-type ?err vargs_ ?base-data                    ] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data nil         false))
-  ([config level ?ns-str ?file ?line msg-type ?err vargs_ ?base-data callsite-id        ] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data callsite-id false))
-  ([config level ?ns-str ?file ?line msg-type ?err vargs_ ?base-data callsite-id spying?] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data callsite-id spying?))
-
-  ([config level ?ns-str ?file ?line ?column msg-type ?err vargs_ ?base-data callsite-id spying?]
-   (when (may-log? :trace level ?ns-str config)
+  ;; Ref. <https://github.com/fzakaria/slf4j-timbre/issues/20>
+  ([config level ?ns-str ?file ?line         msg-type ?err vargs_ ?base-data                            ] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data nil         false   nil))
+  ([config level ?ns-str ?file ?line         msg-type ?err vargs_ ?base-data callsite-id                ] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data callsite-id false   nil))
+  ([config level ?ns-str ?file ?line         msg-type ?err vargs_ ?base-data callsite-id spying?        ] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data callsite-id spying? nil))
+  ([config level ?ns-str ?file ?line         msg-type ?err vargs_ ?base-data callsite-id spying? may-log] (-log! config level ?ns-str ?file ?line nil msg-type ?err vargs_ ?base-data callsite-id spying? may-log))
+  ([config level ?ns-str ?file ?line ?column msg-type ?err vargs_ ?base-data callsite-id spying? may-log]
+   (when (or may-log (may-log? :trace level ?ns-str config))
      (let [instant (enc/now-dt*)
            context *context*
            vargs   @vargs_
@@ -624,7 +625,7 @@
 
      ([{:as   opts
         :keys [loc level msg-type args vargs
-               config ?err ?base-data spying?]
+               config ?err ?base-data spying? #_may-log?]
         :or
         {config `*config*
          ?err   :auto}}]
@@ -650,7 +651,7 @@
 
             ;; Note pre-resolved expansion
             `(taoensso.timbre/-log! ~config ~level ~ns ~file ~line ~column ~msg-type ~?err
-               (delay ~vargs-form) ~?base-data ~callsite-id ~spying?)))))
+               (delay ~vargs-form) ~?base-data ~callsite-id ~spying? ~(get opts :may-log?))))))
 
      ([level msg-type args & [opts]]
       (let [loc  (enc/get-source &form &env)

diff --git a/src/taoensso/timbre/tools/logging.clj b/src/taoensso/timbre/tools/logging.clj
@@ -1,45 +1,46 @@
 (ns taoensso.timbre.tools.logging
-  "`clojure.tools.logging.impl/Logger` implementation.
+  "Interop support for tools.logging -> Timbre."
+  (:require
+   [clojure.tools.logging :as ctl]
+   [taoensso.encore :as enc]
+   [taoensso.timbre :as timbre]))
 
-  Please note that the tools.logging API has some significant limits
-  that native Timbre does not. Would strongly recommend against using
-  Timbre through tools.logging unless you absolutely must (e.g. you're
-  working with a legacy codebase)."
+(defmacro ^:private when-debug [& body] (when #_true false `(do ~@body)))
+(defn- force-var [x] (if (var? x) (deref x) x))
 
-  (:require [clojure.tools.logging]
-            [taoensso.encore :as enc]
-            [taoensso.timbre :as timbre]))
-
-(defn- force-var "To support dynamic vars, etc."
-  [x] (if (var? x) (deref x) x))
-
-(deftype Logger [logger-ns-str timbre-config]
+(deftype TimbreLogger [logger-name config]
+  ;; `logger-name` is typically ns string
   clojure.tools.logging.impl/Logger
 
   (enabled? [_ level]
-    ;; No support for per-call config
-    (timbre/may-log? level logger-ns-str
-      (force-var timbre-config)))
+    (when-debug (println [:tools-logging/enabled? level logger-name]))
+    (timbre/may-log? level logger-name (force-var config)))
 
   (write! [_ level throwable message]
-    (timbre/log! level :p
-      [message] ; No support for pre-msg raw args
-      {:config (force-var timbre-config) ; No support for per-call config
-       :loc    {:ns logger-ns-str} ; No support for other callsite info
-       :?err   throwable})))
-
-(deftype LoggerFactory [get-logger-fn]
+    (when-debug (println [:tools-logging/write! level logger-name]))
+    (timbre/log!
+      {:may-log? true ; Pre-filtered by `enabled?` call
+       :level    level
+       :msg-type :p
+       :config   (force-var config)
+       :loc      {:ns logger-name}
+       :?err     throwable
+       :vargs    [message]})))
+
+(deftype TimbreLoggerFactory [config]
   clojure.tools.logging.impl/LoggerFactory
-  (name [_] "Timbre")
-  (get-logger [_ logger-ns] (get-logger-fn logger-ns)))
+  (name       [_            ] "taoensso.timbre")
+  (get-logger [_ logger-name] (TimbreLogger. (str logger-name) config)))
 
 (defn use-timbre
   "Sets the root binding of `clojure.tools.logging/*logger-factory*`
   to use Timbre."
-  ([             ] (use-timbre #'timbre/*config*))
-  ([timbre-config]
+  ([      ] (use-timbre #'timbre/*config*))
+  ([config]
    (alter-var-root #'clojure.tools.logging/*logger-factory*
-     (fn [_]
-       (LoggerFactory.
-         (enc/fmemoize
-           (fn [logger-ns] (Logger. (str logger-ns) timbre-config))))))))
+     (fn [_] (TimbreLoggerFactory. config)))))
+
+(comment
+  (use-timbre)
+  (ctl/info                    "a" "b" "c")
+  (ctl/error (ex-info "ex" {}) "a" "b" "c"))
diff --git a/test/taoensso/timbre_tests.cljc b/test/taoensso/timbre_tests.cljc
@@ -1,8 +1,10 @@
 (ns taoensso.timbre-tests
   (:require
-   [clojure.test    :as test :refer [deftest testing is]]
-   [taoensso.encore :as enc]
-   [taoensso.timbre :as timbre])
+   [clojure.test :as test :refer [deftest testing is]]
+   #?(:clj [clojure.tools.logging :as ctl])
+   [taoensso.encore :as enc :refer [throws? submap?] :rename {submap? sm?}]
+   [taoensso.timbre :as timbre]
+   #?(:clj [taoensso.timbre.tools.logging :as ttl]))
 
   #?(:cljs
      (:require-macros
@@ -146,6 +148,16 @@
 
      "Appender :output-opts overrides top-level :output-opts")])
 
+;;;; Interop
+
+#?(:clj (def dt-pred (enc/pred (fn [x] (instance? java.util.Date x)))))
+#?(:clj
+   (deftest _interop
+     [(testing "tools.logging -> Timbre"
+        (ttl/use-timbre)
+        [                            (is (sm? (log-data (ctl/info     "a" "b" "c")) {:level :info,  :?ns-str "taoensso.timbre-tests", :instant dt-pred, :msg_ "a b c"}))
+         (is (let [ex (ex-info "Ex" {})] (sm? (log-data (ctl/error ex "a" "b" "c")) {:level :error, :?ns-str "taoensso.timbre-tests", :instant dt-pred, :msg_ "a b c", :?err ex})))])]))
+
 ;;;;
 
 #?(:cljs