Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[log] Cache logger object in a private var #184

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions src/toucan2/log.clj
Original file line number Diff line number Diff line change
Expand Up @@ -173,23 +173,31 @@
(let [a-level-int (level->int a-level 0)]
`(>= ~a-level-int (-current-level-int))))

(defn ^:no-doc -enabled-logger
"Get a logger factor for the namespace named by symbol `ns-symb` at `a-level`, **iff** logging is enabled for that
namespace and level. The logger returned is something that satisfies the `clojure.tools.logging.impl.LoggerFactory`
protocol."
[ns-symb a-level]
(let [logger (tools.log.impl/get-logger tools.log/*logger-factory* ns-symb)]
(when (tools.log.impl/enabled? logger a-level)
logger)))
(def ^:private logger-var-sym
"Unique var that is automatically created and interned into the namespace that uses any toucan2.log logging macros."
(with-meta (gensym "__toucan2_log_logger") {:private true}))

(defn ^:no-doc -get-logger
"Get the logger value from the provided Var if it already bound, otherwise compute the logger object and bind it to the
var."
[^clojure.lang.Var logger-var ns-sym]
(when-not (.hasRoot logger-var)
(.bindRoot logger-var (tools.log.impl/get-logger tools.log/*logger-factory* ns-sym)))
(.getRawRoot logger-var))

(defmacro ^:no-doc -log
"Implementation of various `log` macros. Don't use this directly."
[a-level e doc]
`(let [doc# (delay ~doc)]
(when (-enable-level? ~a-level)
(-pprint-doc '~(ns-name *ns*) @doc#))
(when-let [logger# (-enabled-logger '~(ns-name *ns*) ~a-level)]
(tools.log/log* logger# ~a-level ~e (-pprint-doc-to-str @doc#)))))
(intern *ns* logger-var-sym)
`(let [enable-level?# (-enable-level? ~a-level)
logger# (-get-logger (var ~logger-var-sym) '~(ns-name *ns*))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to move the call to intern inside -get-logger, that way the macroexpansion is a little more concise? If that call needs to happen ever time might as well move it into a helper function so the code exists once total instead of once per macro usage

enable-logger?# (some-> logger# (tools.log.impl/enabled? ~a-level))]
(when (or enable-level?# enable-logger?#)
(let [doc# ~doc]
(when enable-level?#
(-pprint-doc '~(ns-name *ns*) doc#))
(when enable-logger?#
(tools.log/log* logger# ~a-level ~e (-pprint-doc-to-str doc#)))))))

(defmacro ^:no-doc logf
"Implementation of various `log` macros. Don't use this directly."
Expand Down
Loading