Clean up v2.x appenders to conform to v3.x style

[ptaoussanis]

The v3.0.0 Carmine and Postal appenders have adopted a new style that I'd like to see carried over into 3rd party appenders. Basically each appender ns now exposes a make-<X>-appender (ƒ [& [appender-opts make-opts]]) that'll return a ready-to-use Timbre appender.

appender-opts are any general opts like {:enabled? true :min-level :error}. The appender creator fn will supply defaults, override-able with appender-opts.

make-opts are any opts specific to this appender - e.g. database connections, special formatting options, etc. The options exposed here will be up to the appender author.

If anyone feels like helping out (esp. if you're the author of a v2.x appender), please consider a PR to bring the old appenders in line with the new, more flexible style. You can see the taoensso.timbre.appenders.postal or taoensso.timbre.appenders.carmine namespaces for examples.

Thanks a ton! Feel free to ping me with questions, etc.

[ptaoussanis]

Full example:

(ns taoensso.timbre.appenders.postal
  "Email appender. Requires https://github.com/drewr/postal."
  {:author "Peter Taoussanis"}
  (:require [clojure.string  :as str]
            [postal.core     :as postal]
            [taoensso.timbre :as timbre]))

(defn- str-trunc [^String s max-len]
  (if (<= (.length s) max-len) s
      (.substring s 0 max-len)))

(comment (str-trunc "Hello this is a long string" 5))

(defn make-postal-appender
  "Returns a Postal email appender.
  A Postal config map can be provided here as an argument, or as a :postal key
  in :shared-appender-config.

  (make-postal-appender {:enabled? true}
   {:postal-config
    ^{:host \"mail.isp.net\" :user \"jsmith\" :pass \"sekrat!!1\"}
    {:from \"Bob's logger <me@draines.com>\" :to \"foo@example.com\"}})"

  [& [appender-opts {:keys [postal-config subject-len]
                     :or   {subject-len 150}}]]

  (let [default-appender-opts
        {:enabled?   true
         :min-level  :error
         :async?     true ; Slow!
         :rate-limit [5 (* 1000 60 2)] ; 5 calls / 2 mins
         :fmt-output-opts {:nofonts? true} ; Disable ANSI escaped stuff
         }]

    (merge default-appender-opts appender-opts
      {:fn
       (fn [{:keys [ap-config output]}]
         (when-let [postal-config (or postal-config (:postal ap-config))]
           (postal/send-message
            (assoc postal-config
              :subject (-> (str output)
                           (str/trim)
                           (str-trunc subject-len)
                           (str/replace #"\s+" " "))
              :body output))))})))

(def postal-appender "DEPRECATED: Use `make-postal-appender` instead."
  (make-postal-appender))

[dobladez]

Hmm... I might be missing something here. I'm just evaluating timbre and here's a point I don't get.

The example usage of make-postal-appender above passes 2 config maps as arg. But the impl (and most of the make-*-appender fns I've looked at in the source tree) have an arg destructuring form like [& [appender-opts]], which ignores the 2nd map you pass to it as argument, right?. I mean, appender-opts gets bound to the first argument, but the 2nd argument is ignored.
In other words, I don't see how your 2nd argument {:postal-config ...} is used.

What am I missing? Thanks!

[ptaoussanis]

Hi Fernando,

In the example here, appender-opts is merged into the make-postal-appender fn's output (an appender map).

The extra config map is an optional, private config map for make-postal-appender itself.

So appender constructors will usually take a single arg (appender-opts), but may choose to take extra args if they've got some sort of config that may influence their output.

That sounds more complicated than it is :-)

gets bound to the first argument, but the 2nd argument is ignored.
In other words, I don't see how your 2nd argument {:postal-config ...} is used.

The 2nd argument isn't ignored, it's destructured to get postal-config and subject-len bindings, which are then used to influence the output of make-postal-appender.

So:

• A Timbre appender is just a map with certain keys.
• A Timbre appender constructor is just a regular fn that helps to build an appropriate map.
• The constructor's input arguments and what it does with them are entirely up to the constructor, so long as it returns an appropriate map.

Does that make sense / help?

Cheers! :-)

[dobladez]

Thanks a lot for the response, Peter.

My mistake: I was actually looking at make-rolling-appender's docstring and impl. That's where my previous question applies. The doc example passes 2 args, but the impl ignores the 2nd one... its signature reads [& [appender-opts]]. And I got confused because that's the only appender I was playing with.

[ptaoussanis]

Hi Fernando, sorry for the delay following-up - was on vacation!

The doc example passes 2 args, but the impl ignores the 2nd one... its signature reads [& [appender-opts]]

Sure, gotcha. So sometimes an unused arg will still be given a name just to clarify that it is there/available, even if it's unused.

I'll actually usually use a _name convention here (leading underscore) to make it immediately obvious that the thing isn't currently being used.

Does that help / make sense?

[dobladez]

Good for you! Here's the issue... I want to configure the rolling-appender's :path for example. From the docstring of make-rolling-appender, it looks like I can configure it right there with the 2nd arg

(make-rolling-appender {:enabled? true}
    {:path \"log/app.log\"
     :pattern :daily})
...

... however, the implementation ignores it. And that's my problem. Took me awhile to find out why it wasn't working.

The way I'm configuring it now is by setting the :path under [:shared-appender-config :rolling] using set-config! or merge-config!. That works.

Am I clearer now? Thanks again.

[ptaoussanis]

From the docstring of make-rolling-appender, it looks like I can configure it right there with the 2nd arg

Ahh, that appears to be a bug in the rolling appender. I can't recall now who the original author is, but this should be a relatively small/quick change if you felt making a PR?

[devth]

Did anyone ever fix this? Was wondering why my rolling appender wasn't working :)

Update guessing not, so I opened a PR.