viesti / timbre-json-appender

JSON appender for Timbre
MIT License
44 stars 11 forks source link

Allow custom filtering of fields using should-log-field-fn option #18

Closed Limess closed 3 years ago

Limess commented 3 years ago

This has the existing default behaviour, but also adds hostnames to the output logs.

e.g.

(sut/json-appender {:should-log-field-fn (fn [field-name data]
                                           (if (contains? #{:hostname} field-name)
                                             false
                                             (sut/default-should-log-field-fn field-name data)))})

I originally just considered providing set, but this seemed more flexible. Currently :args can't be filtered, neither can :timestamp :err or :log-level (or a log level alias) as these seem somewhat essential.

I don't love the name of the function at the mo'

As part of this PR I also make changes to ensure precidence is respected for default fields, context and an individual log:

default fields -> individual log -> context

i.e. the caller can't override timestamp when using inline-args, and context is always overriden by a matching log argument.

Closes https://github.com/viesti/timbre-json-appender/issues/17

viesti commented 3 years ago

i.e. the caller can't override timestamp when using inline-args, and context is always overriden by a matching log argument.

This is really neat, have though about the same previously, but it has escaped my mind :)

viesti commented 3 years ago

I looked briefly through the code, looks cool. I think the initial idea about defaults fields to log had more glory than it turns out in the implementation 😅 , but I think this is cool.

I think I'll merge this, thanks! :)

Limess commented 3 years ago

Yer, we could have re-implemented the equivalent of output-fn in timbre but to me it wouldn't make a whole lot of sense as the varg parsing + context merging, and the stack trace processing are the two complex parts. Making that overly customisable would essentially make the whole library a very thin wrapper around jsonista.

Ideally I'd like it if timbre's output-fn didn't return a string and actually handled vargs + context as this library does.

Limess commented 3 years ago

Looks like CI didn't like this what I assume is change? https://github.com/viesti/timbre-json-appender/pull/18/commits/17ad806a80a636e8d6077c5e292111b2da1016fe. Weird, it runs fine locally 🤷🏻

viesti commented 3 years ago

Yeah, might have too old clojure cli in CI.

viesti commented 3 years ago

Bumped the image used by CircleCI, so the build works now alo. I'll try to remember to release these changes as 0.2.1 in near future :)

Limess commented 3 years ago

Hey @viesti, are you still planning to release this? Thanks!

viesti commented 3 years ago

Agh! Funny how I create a habbit of procrastination so easily :D Pushed out 0.2.1 now, thanks for reminding me :)

Also fixed passing of should-log-field-fn in tas/install and added a test around tas/install.