Add label to LogHandler

[tkrajacic]

Similar to DispatchQueue this adds a label property on LogHandler. I am not sure though if it makes more sense in the context of logging to make this property non-optional.

[hartbit]

@tkrajacic Just curious, what do you think this will be used for? It makes the examples using the multiplexer a bit more complicated than is entered necessary IMHO.

[tkrajacic]

@hartbit I agree about the use of Multiplexer, but for me there is something off as a user, when I write

let subsysLogger = Logger(label: "com.app.processor")

and the label is possibly discarded by the implementation. That would be very surprising as a user.

IMHO is makes total sense to label a Logger for debugging purposes, or to make sure that there is one special metadata field, that every LogHandler knows about.

As it was, the library clearly had the Logger accept a label, the included StdoutLogger handler accepted it and discarded it, and other handlers didn't handle it consistently (one put it in the output iirc).

Now when we write the documentation for Logger, what would it say about the label? It can only say that this is a String that may or may not be used by the logging system - may not even be stored at all. This sounds wrong to me. Why do we introduce the label explicitly in the framework's API if we don't make it a part of what we expect from a LogHandler? A user could just write it to metadata then, no?

Furthermore, if I write a LogHandler, I can't really use the label consistently, because there is no defined way how I would get to it. Right now I can of course provide some static String like com.swifty-beaver or com.fabric if I am a vendor, but that is also surprising for a user, if they added their own label to the logger, and it is discarded.

So I am really not sure what the label in the framework API is. Why is it part of the API? My PR doesn't fix that of course. That's why I tried asking in the forums as well.

Does that make sense?

[tkrajacic]

Thinking about it more, maybe we should put the label property just on Logger? That would actually make the most sense for debugging. and if a handler accepts a label, the bootstrap method can pass it along just like it always has (we just need to document, that the passed in String is the label of the logger)

Yeah… I'd go with that 🤔

[hartbit]

Thanks for the detailed explanation, I understand what you mean now, and I think it makes total sense! Actually, I think that label should be passed to the LogHandler.log function. For example, I'd like to use the label as a way to "categorise" logs coming from different sub-systems (db, http, cache, etc...), and I'd like those labels to appear in the formatting of the logs. But perhaps that's a job for Metadata: perhaps the categorisation label should be put in there.

[tkrajacic]

Yeah I was thinking the same about "why not just use metadata then", but the fact that label is non-optional makes it extra valuable imho.

[hartbit]

Disregard what I said earlier about passing level in the log function, it was silly: I was still stuck in my head in a world of reference-semantics LogHandlers. I think it's okay if a LogHandler decides not to do anything with the level, so we shouldn't force it to store it I think.

[tkrajacic]

Would you agree then that putting it as a stored property on Logger is a reasonable first approach?

The factory should provide documentation that the logger's label is passed in, and all should be fine right?

[hartbit]

I don't mind having it as a readonly property on Logger, sounds reasonable.

[tkrajacic]

I reverted all my previous commits and added the label to Logger. I like this quite a lot now. Thx @weissi and @hartbit for your feedback!

I'll squash and rebase before merging if desired.

[weissi]

@tkrajacic sorry, conflicting now :|