amperity / dialog

Simple just-enough logging library for Clojure
MIT License
39 stars 2 forks source link

Does it support a rolling file appender? #22

Open ieugen opened 2 years ago

ieugen commented 2 years ago

For long running apps that write to disk it might be good to have rolling file appenders.

There is a good overview of these in https://www.baeldung.com/java-logging-rolling-file-appenders .

I'm also wondering if it's possible to use existing slf4j appender implementations in dialog ?! This might be hard if they are bundled with slf4j implementation - like logback has it's appenders. But there are appenders out there that might not be.

greglook commented 2 years ago

Not yet, but certainly open to adding that capability to the file output writer.

ieugen commented 2 years ago

I've experimented with the output writer and I got it working using the FileAppender. This should work ok with with RollingFile appender as well. It is required to (.stop appender) during logger re-loading. FileAppender and related use ch.qos.logback.core.OutputStreamAppender . And stop will close the underlying stream.

@greglook : does dialog offer any helpers to implement a lifecycle for this or do we need to roll our own ?

(ns dialog.examples.logback
  (:require [dialog.logger :as log]
            [dialog.format.simple :as simple])
  (:import (java.nio.charset StandardCharsets)
           (ch.qos.logback.core ContextBase FileAppender)
           (ch.qos.logback.core.encoder Encoder)))

(defn encoder
  "In Logback, an encoder converts a log event to byte array."
  [output]
  (let [formatter (simple/formatter output)]
    (reify Encoder
    ;; no header 
      (headerBytes [_this] nil)
      (encode [_this event]
        (let [event-str (formatter event)
              event-str (str event-str "\n")]
          (.getBytes event-str StandardCharsets/UTF_8)))
      (footerBytes [_this] nil))))

(defn file-appender
  "Create a ^FileAppender from dialog output map."
  [opts]
  (let [{:keys [appender-name file-name context encoder
                immediate-flush]
         :or {appender-name "file-appender"
              file-name "dialog-file-appender.log"
              immediate-flush true}} opts
        appender (doto (FileAppender.)
                   (.setEncoder encoder)
                   (.setName appender-name)
                   (.setFile file-name)
                   (.setContext context)
                   (.setImmediateFlush immediate-flush)
                   (.start))]
    appender))

(defn file-appender-logger
  "dialog function to write log events using ^FileAppender."
  [output]
  (let [context (ContextBase.)
        encoder (encoder output)
        output (assoc output
                      :context context
                      :encoder encoder)
        appender (file-appender output)]
    (fn write-event
      [event _message]
      (.doAppend appender event))))

(comment
  (log/initialize!)
  (log/info "Hello")
  )

dialog.edn configuration file

{:level :debug
 :levels {"vault" :info}
 :outputs {:logback {:type dialog.examples.logback/file-appender-logger
                     :format :simple
                     :timestamp :short
                     :file-name "target/dialog-logback.log"}}}

deps.edn

{:paths ["src" "resources"]
 :deps {;; run clj -X:deps prep 
        amperity/dialog {:local/root "../../"}
        ch.qos.logback/logback-core {:mvn/version "1.2.6"}}
}
greglook commented 2 years ago

does dialog offer any helpers to implement a lifecycle for this or do we need to roll our own ?

Ah, an interesting question - given the original goal of simplicity and basic stdout/file outputs, this wasn't a concern. I think there's room to move to a writer protocol which could provide this kind of advanced capability. 🤔 By "logger reloading" I'm assuming you mean a full reinitialization, not just changing the current logger levels.

It seems like this stop protocol method should probably also be invoked by a JVM shutdown hook.

ieugen commented 2 years ago

Thanks. Probably this will work okish by adding a call to Runtime.getRuntime().AddShutDownHook() . I don't imagine people will re-initialize it that often in production. It's hacky and a known issue so far.

One way to do it now would be to register all the initialized appenders in an atom and make sure to stop them before / after initialize! . But it's not pretty without support in the library.

If you have any ideas to make this work while keeping it simple that would be great.

I would imagine that the output map could define a set of special keys like :dialog.logger.hooks/output-init, :dialog.logger.hooks/output-start, :dialog.logger.hooks/output-stop .

Then initialize! could check for those functions and call them at specific points ?! (I did not put a lot of thought in this :) )

I'm going to let the issue open for a bit more time so I can test RollingFileAppender, but IMO it's good to know we can reuse logback appenders.

greglook commented 2 years ago

I was thinking of something along the lines of:

(defprotocol Writer

  (write-event
    [writer event message]
    "Write an event with a formatted message to the output.")

  (close
    [writer]
    "Shut down the output writer."))

Then to support the current use-cases in the library:

(extend-protocol Writer

  clojure.lang.Fn

  (write-event
    [f event message]
    (f event message))

  (close
    [f]
    nil))

Then of course there would need to be some logic in the initialize! call to check whether there was already a populated configuration map, and call close on each writer.

It occurs to me that one way you could handle this with the current logic is to provide a custom :init function which does the FileAppender construction and adds those to each of the :outputs maps, then your output :type function could use those to construct the writer function. That would at least preserve references to the appenders themselves for your custom stop call to use before it invokes dialog.logger/initialize! again. 🤔

ieugen commented 2 years ago

Thanks. I'll focus on delivering sentry first and do the migration and then maybe give this a try.

It occurs to me that one way you could handle this with the current logic is to provide a custom :init function which does ...

This might work if you build your app with this in mind. If you are on the devops side and just want to configure things without having access to source, then only the protocol option will work, provided there is a library of appenders that implement Writer .