viesti / timbre-json-appender

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

Trailing map interaction with formatting calls #33

Closed NoahTheDuke closed 8 months ago

NoahTheDuke commented 8 months ago

Hey Kimmo!

Thanks for the library, we really love it at Crossbeam.

I've run into a small bug or maybe an unintended interaction. If you use the trailing map form with a format logging call, it'll throw: (log/infof "Hello %s" "world!" {:role "admin"}) This is due to the unconditional (apply hash-map vargs) call in handle-vargs.

I think this could be fixed by checking the one branch from count-vargs in the format side: args (if (and (= 1 (count vargs)) (map? (first vargs))) (first vargs) (apply hash-map vargs))

viesti commented 8 months ago

Hi!

Thanks for the library, we really love it at Crossbeam.

Really nice to hear that the lib has been of use! :)

Took a look at the change and I think it could work.

Kind of resilient this logging function we have here, taking in data and making sure json comes out :D Not sure at what point it gets too complex in keeping up the convenience, but maybe we are not too far yet :D.

Thanks for the PR too! :)

NoahTheDuke commented 8 months ago

Kind of resilient this logging function we have here, taking in data and making sure json comes out :D Not sure at what point it gets too complex in keeping up the convenience, but maybe we are not too far yet :D.

haha Yeah, we've definitely stretched it to its limits. I wrote a 150 line clj-kondo hook for our 200k line codebase to enforce usage because of how tricky it can be. Maybe in a 1.0 release you could drop support for the trailing map style? That would simplify the logic greatly.

viesti commented 8 months ago

I wrote a 150 line clj-kondo hook

😅 Sorry for this :D

I think I also initially missed :output-fn in timbre, we could just have a really resilient data -> json string serializer, though there is probably something in the convenience of being able to optionally put an exception as the first argument, then add more arguments, and have that single call format and print, but yeah, can't stop learning :)