taoensso / telemere

Structured telemetry library for Clojure/Script
https://www.taoensso.com/telemere
Eclipse Public License 1.0
199 stars 5 forks source link

Consider merging maps via `with-ctx+` #30

Closed rschmukler closed 1 month ago

rschmukler commented 1 month ago

If you have a context that has a map and you extend it with a with-ctx+ it will wipe the existing context. It could be nice if telemere did merge-with merge on the contexts

Eg.

(tel/with-ctx+ {:foo {:bar 5}}
  (tel/with-ctx+ {:foo {:baz 6}}
    (:ctx (tel/with-signal (tel/log! "Hello world!"))))
 ;; => {:foo {:baz 6}}, could be better as {:foo {:baz 6 :bar 5}
ptaoussanis commented 1 month ago

Hi Ryan, thanks for the clear example!

with-ctx+ already supports an arbitrary update fn, so you can do something like this:

(tel/with-ctx+ {:foo {:bar 5}}
  (tel/with-ctx+ #(taoensso.encore/nested-merge % {:foo {:baz 6}})
    tel/*ctx*)) ; => {:foo {:bar 5, :baz 6}}

The reason this (nested merging) isn't the default is because it's not always what one will want. And even when doing nested merging, one might want to customize the merging behaviour.

So the idea is to keep the default behaviour simple, but provide easy support for a custom fn for arbitrary control.

Hope that helps?

rschmukler commented 1 month ago

Nice! I didn't realize it could take a function. Thank you!