Can a `Logger.with_metadata/2` be useful?

Chat & Discussions Proposals: Ideas
1

Hello :waving_hand:

I’ve been looking for a way to do, put simply:

# Set metadata
Logger.metadata(meta1: :one)

# Execute code

# Remove metadata
Logger.metadata(meta1: nil)

I could have used this multiple times now. This method works fine, but when the `# Execute code` starts getting bigger, it’s easy to forget resetting the metadata at the end of it. Also, if an exception arises in the code and is caught, the metadata won’t get reset.

So I was wondering if a function `Logger.with_metadata([meta1: :one], fn → … end)` could be useful to integrate in `Logger`. Any thought about it? :blush:

2

I do not think that it would be useful to have in core, but as it is not that hard to implement on your own, then I do not think that it would be much of a problem.

I never had need for such thing, so I may be biased there.

2 Likes
3

I basically rolled this out for myself during three previous consulting engagements. I suggest you do the same. No point having such a surface-level helper upstreamed. Do it like f.ex. Repo.transaction – you pass a function / MFA to your helper and then do the cleanup after it is executed.

2 Likes
4

As others already said, it’s a well-known functionality which is not very hard to implement. It’s very common in Common Lisp and it’s usually implemented with macros. If you want you could do the same and, in the end, have the following:

with_metadata meta1: :one do
  # execute code
end
5

I must say I’m a bit surprised :smile: I quite expected the answer “nobody needs that, just do your own helper”, but if the answer is “yes, I do that a lot with my own implementation”, why not make it generic? :thinking: Indeed, the implementation is not really complicated, but I don’t think that makes the proposal invalid?

6

A standard library’s API surface must be as small as possible by necessity. We don’t know for sure how it will evolve; maybe a next version will deprecate Logger.metadata and replace it with something better. That would then lead to deprecating not one, but two functions. It complicates things. For reference, you can research why is Python such a Frankenstein monster of APIs. They added too much and too liberally back in the day and are now stuck with that legacy.

A stdlib’s job is to give you building blocks, not ergonomic helpers.

1 Like
7

The problem I see there is that there is no clear semantic of how it should work. That is why it should be left up to the user.

Imagine situation like:

# metadata = %{foo: :bar}
Logger.with_metadata([foo: :baz], fn ->
  Logger.metadata(foo: :quux)
end)
# What should be the value of `:foo` key in metadata?

You say that

But if exception is raised, then system is in broken state anyway. So caller should know whether they need to reset the state to known form, not the callee.

That is why there is no clear way forward with that, as there is too many corner cases and the usability of such function is IMHO low, and implementing what you need is quite simple:

%{foo: old_foo} = Logger.metadata()
Logger.metadata(foo: :new_foo)
try do
  # Code
after
  Logger.metadata(foo: old_foo)
end
2 Likes
8

Thanks for your answers, I’ll stick with our custom doing then :smile:

Have a nice day!