BrunoBonacci / mulog

μ/log is a micro-logging library that logs events and data, not words!
https://cljdoc.org/d/com.brunobonacci/mulog/
Apache License 2.0
490 stars 48 forks source link

Logging maps #109

Closed samwagg closed 1 year ago

samwagg commented 1 year ago

I have a use-case that I'm not sure is well-supported. I would like to log a map that exists in my code independently of event logging. But because log is a variadic macro that takes key-value pairs, there is no way I know of to pass my map without enumerating every key contained by it.

The API for global and local context already use maps. The solution could be as simple as creating a variant of log that accepts a map.

Thanks for your consideration!

BrunoBonacci commented 1 year ago

Hi @samwagg

yes it is supported, here is how you can log a map. Let's say you have a map like {:foo :bar, :num 1234} and you want to log it as is here is how you can do it:

(µ/log ::my-event-name  :value {:foo :bar, :num 1234} )

does this help?

samwagg commented 1 year ago

Thanks for the quick reply! Well, this is what I was doing. But I want to avoid that extra (and somewhat arbitrary) layer of nesting, especially because my map is already a bit nested. In fact, I think I would rather call log* directly or write a custom macro.

BrunoBonacci commented 1 year ago

fine, you can surely call log* but maybe it would be better to write your own macro on top of the log macro of mulog something like the following should do the job

(defmacro mylog
  [event map]
  `(u/log ~event ~@(mapcat seq map)))

(mylog  ::my-event-name {:foo :bar, :num 1234})
;; expands to
;; (u/log :user/my-event-name :foo :bar :num 1234)

the advantage is that if I change the log macro, you inherit the behaviours automatically

samwagg commented 1 year ago

Thanks, that seems like a good approach and will probably suffice.

I do want to push just a little bit on my suggestion for an API addition. I understand that authors/maintainers of open source projects get lots of requests to add features and have to be very conservative in adding stuff to avoid cruft. So I knew going in that this was unlikely result in API changes, and I get it.

That said, is there a reason in principle that we shouldn't be able to define events as maps away from the log call site? It seems to me that the current API design imposes an arbitrary constraint that the pairs be enumerated at the call site, but it didn't have to be that way.

BrunoBonacci commented 1 year ago

One of my top design goals was to be able to add logging and metering to projects without impacting their performance characteristics. I had cases in which I had to remove logs from source code because it was severely impacting the runtime performances (like Log4j v1) and also some other metering tools. So the entire design of mulog is based on reducing the runtime impact as much as possible.

Constructing a map is much slower than just a list of pairs, and it seems to me too constrictive to force the user to construct a map when in many cases the values are readily available as local in your function. So I decided to accept inline pairs and defer the construction of the map to a background thread.

If you already have the map (because you received already built) then it is easy enough to publish its value as I suggested above (https://github.com/BrunoBonacci/mulog/issues/109#issuecomment-1479712621)

samwagg commented 1 year ago

@BrunoBonacci thanks for taking the time to explain. That's helpful and interesting.

cch1 commented 1 year ago

I found this issue when faced with the exact same need as @samwagg. Would it not be preferable to have an opt-in variant of log (maybe logm) as part of the mulog library instead of having everybody roll their own alternative roughly equivalent to what @BrunoBonacci shows here?

Because mulog requires that logged events have a name (an excellent decision IMO), adding a key to log an entity or record is sometimes completely artificial. Consider (log ::SQSMessageReceived :message message) versus (logm ::SQSMessageReceived message). Not only is the invocation less noisy, but the output is shallower with logm.

With such a macro, when logging entities or records one could use logm and when logging discrete values one could use the existing log macro.

Ideally, the existing log macro would adapt to logging an odd number of "pair" arguments by treating the last one as an entity. It would minimize the surface area but cost a few nanoseconds for a count and conditional (but only at macro expansion time).

Regardless, I agree that rolling my own macro (per the author's suggestion) will suffice, and I thank the author for giving us this tool.