Logging feature flag and tools.logging.readable (#981) (#987)

* put tools.logging behind a feature flag

* add tools.logging.readable

- move logging namespace to feature folder
- add logging.readable namespace
- add tests for logging.readable

* cleanup logging changes

- add logging env var to compile script
- remove unconditional require of logging namespace

* move old-config capture to before alter

* remove feature check from logging tests
This commit is contained in:
Bob 2021-08-29 17:43:53 -04:00 committed by GitHub
parent ad73a09c0e
commit 242c3d442f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 107 additions and 31 deletions

View file

@ -112,6 +112,7 @@ Babashka supports the following feature flags:
| `BABASHKA_FEATURE_ORACLEDB` | Includes the [Oracle](https://www.oracle.com/database/technologies/appdev/jdbc.html) JDBC driver | `false` |
| `BABASHKA_FEATURE_DATASCRIPT` | Includes [datascript](https://github.com/tonsky/datascript) | `false` |
| `BABASHKA_FEATURE_LANTERNA` | Includes [clojure-lanterna](https://github.com/babashka/clojure-lanterna) | `false` |
| `BABASHKA_FEATURE_LOGGING` | Includes [clojure.tools.logging](https://github.com/clojure/tools.logging) with [taoensso.timbre](https://github.com/ptaoussanis/timbre) as the default implementation| `true` |
Note that httpkit server is currently experimental, the feature flag could be toggled to `false` in a future release.

View file

@ -1,6 +1,7 @@
(ns babashka.impl.logging
(:require [clojure.tools.logging]
[clojure.tools.logging.impl :as impl]
[clojure.tools.logging.readable]
[sci.core :as sci]
[taoensso.encore :as enc :refer [have]]
[taoensso.timbre :as timbre]
@ -178,3 +179,10 @@
(def tools-logging-impl-namespace
(make-ns 'clojure.tools.logging.impl lins ['get-logger 'enabled?]))
(def tlr-ns (sci/create-ns 'clojure.tools.logging.readable nil))
(def tools-logging-readable-namespace
(make-ns 'clojure.tools.logging.readable tlr-ns ['trace 'tracef 'debug 'debugf 'info 'infof
'warn 'warnf 'error 'errorf 'fatal 'fatalf
'logf 'logp 'spyf]))

View file

@ -23,9 +23,7 @@
[cheshire "5.10.1"]
[nrepl/bencode "1.1.0"]
[borkdude/sci.impl.reflector "0.0.1"]
[org.clojure/test.check "1.1.0"]
[com.taoensso/timbre "5.1.2"]
[org.clojure/tools.logging "1.1.0"]]
[org.clojure/test.check "1.1.0"]]
:profiles {:feature/xml {:source-paths ["feature-xml"]
:dependencies [[org.clojure/data.xml "0.2.0-alpha6"]]}
:feature/yaml {:source-paths ["feature-yaml"]
@ -60,6 +58,9 @@
:dependencies [[rewrite-clj/rewrite-clj "1.0.644-alpha"]]}
:feature/selmer {:source-paths ["feature-selmer"]
:dependencies [[selmer/selmer "1.12.44"]]}
:feature/logging {:source-paths ["feature-logging"]
:dependencies [[com.taoensso/timbre "5.1.2"]
[org.clojure/tools.logging "1.1.0"]]}
:test [:feature/xml
:feature/lanterna
:feature/yaml
@ -77,6 +78,7 @@
:feature/spec-alpha
:feature/rewrite-clj
:feature/selmer
:feature/logging
{:dependencies [[com.clojure-goes-fast/clj-async-profiler "0.5.0"]
[com.opentable.components/otj-pg-embedded "0.13.3"]]}]
:uberjar {:global-vars {*assert* false}

View file

@ -95,6 +95,7 @@ then
export BABASHKA_FEATURE_SPEC_ALPHA="${BABASHKA_FEATURE_SPEC_ALPHA:-false}"
export BABASHKA_FEATURE_REWRITE_CLJ="${BABASHKA_FEATURE_REWRITE_CLJ:-false}"
export BABASHKA_FEATURE_SELMER="${BABASHKA_FEATURE_SELMER:-false}"
export BABASHKA_FEATURE_LOGGING="${BABASHKA_FEATURE_LOGGING:-false}"
fi
"$GRAALVM_HOME/bin/native-image" "${args[@]}"

View file

@ -22,6 +22,7 @@ then
export BABASHKA_FEATURE_HTTPKIT_CLIENT="${BABASHKA_FEATURE_HTTPKIT_CLIENT:-false}"
export BABASHKA_FEATURE_HTTPKIT_SERVER="${BABASHKA_FEATURE_HTTPKIT_SERVER:-false}"
export BABASHKA_FEATURE_CORE_MATCH="${BABASHKA_FEATURE_CORE_MATCH:-false}"
export BABASHKA_FEATURE_LOGGING="${BABASHKA_FEATURE_LOGGING:-false}"
fi
BABASHKA_LEIN_PROFILES="+uberjar"
@ -159,6 +160,13 @@ else
BABASHKA_LEIN_PROFILES+=",-feature/selmer"
fi
if [ "$BABASHKA_FEATURE_LOGGING" != "false" ]
then
BABASHKA_LEIN_PROFILES+=",+feature/logging"
else
BABASHKA_LEIN_PROFILES+=",-feature/logging"
fi
cp deps.edn resources/META-INF/babashka/deps.edn
if [ -z "$BABASHKA_JAR" ]; then

View file

@ -118,12 +118,18 @@ set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,+feature/rewrite-clj
set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,-feature/rewrite-clj
)
if not "%BABASHKA_FEATURE_REWRITE_SELMER%"=="false" (
if not "%BABASHKA_FEATURE_SELMER%"=="false" (
set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,+feature/selmer
) else (
set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,-feature/selmer
)
if not "%BABASHKA_FEATURE_LOGGING%"=="false" (
set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,+feature/logging
) else (
set BABASHKA_LEIN_PROFILES=%BABASHKA_LEIN_PROFILES%,-feature/logging
)
call lein with-profiles %BABASHKA_LEIN_PROFILES% bb "(+ 1 2 3)"
call lein with-profiles %BABASHKA_LEIN_PROFILES%,+reflection,-uberjar do run

View file

@ -16,6 +16,7 @@
(def test-check? (not= "false" (System/getenv "BABASHKA_FEATURE_TEST_CHECK")))
(def rewrite-clj? (not= "false" (System/getenv "BABASHKA_FEATURE_REWRITE_CLJ")))
(def selmer? (not= "false" (System/getenv "BABASHKA_FEATURE_SELMER")))
(def logging? (not= "false" (System/getenv "BABASHKA_FEATURE_LOGGING")))
;; excluded by default
(def jdbc? (= "true" (System/getenv "BABASHKA_FEATURE_JDBC")))
@ -74,3 +75,6 @@
(when selmer?
(require '[babashka.impl.selmer]))
(when logging?
(require '[babashka.impl.logging]))

View file

@ -23,8 +23,6 @@
[babashka.impl.error-handler :refer [error-handler]]
[babashka.impl.features :as features]
[babashka.impl.fs :refer [fs-namespace]]
[babashka.impl.logging :refer [timbre-namespace tools-logging-namespace
tools-logging-impl-namespace]]
[babashka.impl.pods :as pods]
[babashka.impl.pprint :refer [pprint-namespace]]
[babashka.impl.print-deps :as print-deps]
@ -244,7 +242,8 @@ Use bb run --help to show this help output.
:feature/test-check %s
:feature/spec-alpha %s
:feature/rewrite-clj %s
:feature/selmer %s}")
:feature/selmer %s
:feature/logging %s}")
version
features/core-async?
features/csv?
@ -263,7 +262,8 @@ Use bb run --help to show this help output.
features/test-check?
features/spec-alpha?
features/rewrite-clj?
features/selmer?)))
features/selmer?
features/logging?)))
(defn read-file [file]
(let [f (io/file file)]
@ -357,10 +357,7 @@ Use bb run --help to show this help output.
'babashka.process process-namespace
'clojure.core.server clojure-core-server-namespace
'babashka.deps deps-namespace
'babashka.tasks tasks-namespace
'taoensso.timbre timbre-namespace
'clojure.tools.logging tools-logging-namespace
'clojure.tools.logging.impl tools-logging-impl-namespace}
'babashka.tasks tasks-namespace}
features/xml? (assoc 'clojure.data.xml @(resolve 'babashka.impl.xml/xml-namespace))
features/yaml? (assoc 'clj-yaml.core @(resolve 'babashka.impl.yaml/yaml-namespace)
'flatland.ordered.map @(resolve 'babashka.impl.ordered/ordered-map-ns))
@ -416,7 +413,15 @@ Use bb run --help to show this help output.
'selmer.util
@(resolve 'babashka.impl.selmer/selmer-util-namespace)
'selmer.validator
@(resolve 'babashka.impl.selmer/selmer-validator-namespace))))
@(resolve 'babashka.impl.selmer/selmer-validator-namespace))
features/logging? (assoc 'taoensso.timbre @(resolve 'babashka.impl.logging/timbre-namespace)
'clojure.tools.logging
@(resolve 'babashka.impl.logging/tools-logging-namespace)
'clojure.tools.logging.impl
@(resolve 'babashka.impl.logging/tools-logging-impl-namespace)
'clojure.tools.logging.readable
@(resolve 'babashka.impl.logging/tools-logging-readable-namespace))))
(def edn-readers (cond-> {}
features/yaml?

View file

@ -21,6 +21,7 @@
(println "before setting log level")
(test-fn)
(def old-config timbre/*config*)
(alter-var-root #'timbre/*config* #(assoc %1 :min-level :info))
(println "after setting log level to :info")
@ -38,7 +39,8 @@
(log/infof "Hello %s" 123)
(timbre/swap-config! assoc-in [:appenders :spit] (timbre/spit-appender {:fname "/tmp/timbre.log"}))
(log/infof "Hello %s" 123)))
(log/infof "Hello %s" 123)
(timbre/swap-config! (constantly old-config))))
(deftest logging-test
(let [res (tu/bb nil (pr-str program))]
@ -47,25 +49,64 @@
(is (= 11 (count (re-seq #"INFO" res)))))
(testing "println appender works with with-out-str"
(let [res (tu/bb
nil
(pr-str '(do
(require '[taoensso.timbre :as timbre]
'[clojure.string :as str])
(str/includes? (with-out-str (timbre/info "hello")) "hello"))))
nil
(pr-str '(do
(require '[taoensso.timbre :as timbre]
'[clojure.string :as str])
(str/includes? (with-out-str (timbre/info "hello")) "hello"))))
res (edn/read-string res)]
(is (true? res))))
(testing "spit-appender"
(let [temp-file (-> (fs/create-temp-dir)
(fs/file "log.txt"))
program (pr-str '(do
(require '[taoensso.timbre :as timbre]
'[clojure.string :as str])
(def appender (timbre/spit-appender {:fname :fname-placeholder}))
(timbre/swap-config! assoc-in [:appenders :spit] appender)
(str/includes? (with-out-str (timbre/info "hello")) "hello")))
program (str/replace program ":fname-placeholder" (pr-str (.getPath temp-file)))
_ (tu/bb
nil
program)
res (slurp temp-file)]
(fs/file "log.txt"))
program (pr-str '(do
(require '[taoensso.timbre :as timbre]
'[clojure.string :as str])
(def appender (timbre/spit-appender {:fname :fname-placeholder}))
(def old-config timbre/*config*)
(timbre/swap-config! assoc-in [:appenders :spit] appender)
(str/includes? (with-out-str (timbre/info "hello")) "hello")
(timbre/swap-config! (constantly old-config))))
program (str/replace program ":fname-placeholder" (pr-str (.getPath temp-file)))
_ (tu/bb
nil
program)
res (slurp temp-file)]
(is (str/includes? res "hello")))))
(def readable-prog
'(do
(ns readble-test)
(require '[clojure.tools.logging.readable :as logr])
(require '[taoensso.timbre :as timbre])
(defn test-fn []
(logr/trace (ex-info "trace exception" {}))
(logr/debugf "%s" {"abc" 123 "def" 789})
(logr/info (list \a \b))
(logr/warnf "%s" "test warn")
(let [g (logr/spyf "%s" (apply str (interpose "," ["abc" "def" "ghi"])))]
(println g)))
(println "before setting anything")
(test-fn)
(println "with print-readably set to nil (overridden by log macros)")
(binding [*print-readably* nil]
(test-fn))
(println "setting log level")
(timbre/set-level! :warn)
(test-fn)
(timbre/set-level! :debug)))
(deftest readable-logging-test
(let [res (tu/bb nil (pr-str readable-prog))]
(testing "spied value is returned and printed (and printed from println even though spyf level isn't enabled)"
(is (= 5 (count (re-seq #"abc,def,ghi" res)))))
(testing "spied value is printed readably as a result of spyf"
(is (= 2 (count (re-seq #"\"abc,def,ghi\"" res)))))
(testing "strings logged are printed readably"
(is (= 3 (count (re-seq #"\"test warn\"" res)))))
(testing "lists are printed readably"
(is (= 2 (count (re-seq #"\(\\a \\b\)" res)))))))