Protect appenders from delay replacement through middleware

- This simplifies the requirements for appender authors.
  - Note that middleware authors still need to be careful since
    it'd be infeasible to offer similar protection between each
    individual layer of middleware.
This commit is contained in:
Peter Taoussanis 2016-01-23 13:58:10 +07:00
parent f8a83fd798
commit a26ecc6a96
8 changed files with 34 additions and 26 deletions

View File

@ -3,6 +3,7 @@
## Pending ## Pending
* **New**: added 3rd-party gelf appender [#147 @davewo] * **New**: added 3rd-party gelf appender [#147 @davewo]
* **Implementation**: appenders no longer need to worry about using `force` instead of `@`/`deref`

View File

@ -38,13 +38,13 @@
([{:keys [no-stacktrace? stacktrace-fonts] :as opts} data] ([{:keys [no-stacktrace? stacktrace-fonts] :as opts} data]
(let [{:keys [level ?err_ vargs_ msg_ ?ns-str hostname_ timestamp_]} data] (let [{:keys [level ?err_ vargs_ msg_ ?ns-str hostname_ timestamp_]} data]
(str (str
#+clj (force timestamp_) #+clj " " #+clj @timestamp_ #+clj " "
#+clj (force hostname_) #+clj " " #+clj @hostname_ #+clj " "
(str/upper-case (name level)) " " (str/upper-case (name level)) " "
"[" (or ?ns-str "?ns") "] - " "[" (or ?ns-str "?ns") "] - "
(force msg_) @msg_
(when-not no-stacktrace? (when-not no-stacktrace?
(when-let [err (force ?err_)] (when-let [err @?err_]
(str "\n" (stacktrace err opts)))))))) (str "\n" (stacktrace err opts))))))))
;;; Alias core appenders here for user convenience ;;; Alias core appenders here for user convenience
@ -251,7 +251,7 @@
rate-limiting purposes, etc." rate-limiting purposes, etc."
[data] [data]
(let [{:keys [?ns-str ?line vargs_]} data (let [{:keys [?ns-str ?line vargs_]} data
vargs (force vargs_)] vargs @vargs_]
(str (str
(or (some #(and (map? %) (:timbre/hash %)) vargs) ; Explicit hash given (or (some #(and (map? %) (:timbre/hash %)) vargs) ; Explicit hash given
#_[?ns-str ?line] ; TODO Waiting on http://goo.gl/cVVAYA #_[?ns-str ?line] ; TODO Waiting on http://goo.gl/cVVAYA
@ -330,7 +330,7 @@
[config level ?ns-str ?file ?line msg-type vargs_ ?base-data] [config level ?ns-str ?file ?line msg-type vargs_ ?base-data]
(when (log? level ?ns-str config) (when (log? level ?ns-str config)
(let [instant (enc/now-dt) (let [instant (enc/now-dt)
vargs*_ (delay (vsplit-err1 (force vargs_))) vargs*_ (delay (vsplit-err1 @vargs_))
?err_ (delay (get @vargs*_ 0)) ?err_ (delay (get @vargs*_ 0))
vargs_ (delay (get @vargs*_ 1)) vargs_ (delay (get @vargs*_ 1))
context *context* context *context*
@ -352,7 +352,7 @@
msg-fn msg-fn
(fn [vargs_] ; For use *after* middleware, etc. (fn [vargs_] ; For use *after* middleware, etc.
(when-not (nil? msg-type) (when-not (nil? msg-type)
(when-let [vargs (have [:or nil? vector?] (force vargs_))] (when-let [vargs (have [:or nil? vector?] @vargs_)]
(case msg-type (case msg-type
:p (str-join vargs) :p (str-join vargs)
:f (let [[fmt args] (enc/vsplit-first vargs)] :f (let [[fmt args] (enc/vsplit-first vargs)]
@ -373,7 +373,15 @@
(when (and (:enabled? appender) (when (and (:enabled? appender)
(level>= level (or (:min-level appender) :trace))) (level>= level (or (:min-level appender) :trace)))
(let [rate-limit-specs (:rate-limit appender) (let [;; As a convenience to appenders, make sure that middleware
;; hasn't replaced any delays with non-delays
data
(merge data
{:?err_ (->delay (:?err_ data))
:vargs_ (->delay (:vargs_ data))
#+clj :hostname_ #+clj (->delay (:hostname_ data))})
rate-limit-specs (:rate-limit appender)
data-hash-fn (inherit-over :data-hash-fn appender config data-hash-fn (inherit-over :data-hash-fn appender config
default-data-hash-fn) default-data-hash-fn)
rate-limit-okay? rate-limit-okay?

View File

@ -20,9 +20,9 @@
(fn [data] (fn [data]
(let [{:keys [level timestamp_ msg_]} data] (let [{:keys [level timestamp_ msg_]} data]
(str (str
(force timestamp_) " " @timestamp_ " "
(str/upper-case (name level)) " " (str/upper-case (name level)) " "
(force msg_)))) @msg_)))
:fn :fn
(fn [data] (fn [data]
@ -30,7 +30,7 @@
ns (str ?ns-str "") ns (str ?ns-str "")
output-str (output-fn data)] output-str (output-fn data)]
(if-let [throwable (force ?err_)] (if-let [throwable @?err_]
(case level (case level
:trace (android.util.Log/d ns output-str throwable) :trace (android.util.Log/d ns output-str throwable)
:debug (android.util.Log/d ns output-str throwable) :debug (android.util.Log/d ns output-str throwable)

View File

@ -22,9 +22,9 @@
(let [entry {:instant instant (let [entry {:instant instant
:level level :level level
:?ns-str (str (:?ns-str data)) :?ns-str (str (:?ns-str data))
:hostname (str (force (:hostname_ data))) :hostname (str @(:hostname_ data))
:vargs (str (force (:vargs_ data))) :vargs (str @(:vargs_ data))
:?err (str (force (:?err_ data)))}] :?err (str @(:?err_ data))}]
(mongo/with-mongo (ensure-conn config) (mongo/with-mongo (ensure-conn config)
(mongo/insert! collection entry)))) (mongo/insert! collection entry))))

View File

@ -52,6 +52,6 @@
(let [{:keys [appender msg_ level hostname_]} data (let [{:keys [appender msg_ level hostname_]} data
gelf-transport (:gelf-transport appender) gelf-transport (:gelf-transport appender)
log-level (timbre-to-gelf-level level) log-level (timbre-to-gelf-level level)
gelf-message (-> (GelfMessageBuilder. (force msg_) (force hostname_)) gelf-message (-> (GelfMessageBuilder. @msg_ @hostname_)
(.level log-level) .build)] (.level log-level) .build)]
(.send gelf-transport gelf-message)))}))) (.send gelf-transport gelf-message)))})))

View File

@ -46,8 +46,8 @@
(let [{:keys [level ?err_ msg_]}] (let [{:keys [level ?err_ msg_]}]
(format "[%s] %s%s" (format "[%s] %s%s"
(-> level name (str/upper-case)) (-> level name (str/upper-case))
(or (force msg_) "") (or @msg_ "")
(if-let [err (force ?err_)] (if-let [err @?err_]
(str "\n" (timbre/stacktrace err)) (str "\n" (timbre/stacktrace err))
"")))) ""))))

View File

@ -58,9 +58,9 @@
{:instant instant {:instant instant
:level level :level level
:?ns-str (:?ns-str data) :?ns-str (:?ns-str data)
:hostname (force (:hostname_ data)) :hostname @(:hostname_ data)
:vargs (force (:vargs_ data)) :vargs @(:vargs_ data)
:?err (force (:?err_ data))} :?err @(:?err_ data)}
(when-let [pstats (:profile-stats data)] (when-let [pstats (:profile-stats data)]
{:profile-stats pstats})) {:profile-stats pstats}))

View File

@ -41,9 +41,8 @@
]} ]}
data data
;;; Use `force` to realise possibly-delayed args: ?err @?err_ ; ?err non-nil iff first given arg was an error
?err (force ?err_) ; ?err non-nil iff first given arg was an error vargs @vargs_ ; Vector of raw args (excl. possible first error)
vargs (force vargs_) ; Vector of raw args (excl. possible first error)
;; You'll often want an output string with ns, timestamp, vargs, etc. ;; You'll often want an output string with ns, timestamp, vargs, etc.
;; A (fn [data]) -> string formatter is provided under the :output-fn ;; A (fn [data]) -> string formatter is provided under the :output-fn
@ -153,7 +152,7 @@
:fn :fn
(fn [data] (fn [data]
(let [{:keys [level output-fn vargs_]} data (let [{:keys [level output-fn vargs_]} data
vargs (force vargs_) vargs @vargs_
[v1 vnext] (enc/vsplit-first vargs) [v1 vnext] (enc/vsplit-first vargs)
output (if (= v1 :timbre/raw) output (if (= v1 :timbre/raw)
(into-array vnext) (into-array vnext)