strimzi / strimzi-kafka-operator

Apache Kafka® running on Kubernetes
https://strimzi.io/
Apache License 2.0
4.86k stars 1.3k forks source link

[Enhancement] Allow configuring arbitrary loggers #1848

Closed tombentley closed 5 years ago

tombentley commented 5 years ago

Is your feature request related to a problem? Please describe. I cannot configure individual loggers unless they're one of the ones in the default logger config for that particular component (e.g. the entityTopicOperatorDefaultLoggingProperties resource). This means that I can't, for example, configure strimzi loggers at DEBUG and loggers of dependencies at INFO or WARN.

Describe the solution you'd like I'd like to be able to configure arbitrary individual loggers using standard log4j config.

scholzj commented 5 years ago

Can't you do this by supplying your own log4j.properties file?

tombentley commented 5 years ago

Do you mean using external logging?

scholzj commented 5 years ago

Yes.

tombentley commented 5 years ago

Why should I have to configure logging in a completely different way to achieve this, though?

scholzj commented 5 years ago

Well, I think the philosophy when we did this was to have one simple way and one way to give you the flexibility to do whatever you want. Anyway, the reason I asked is because it is always good to know where there are workarounds which people can use and where there is no way how to do things.

ppatierno commented 5 years ago

I have just done a test with the bridge. Started with the default logging configuration so the healthy and ready are the only loggers in the log4j.properties file in the created ConfigMap. I hit the bridge on the /openapi endpoint and I could see the log. At some point I edited the KafkaBridge resource adding an inline logger related to openapi setting it to WARN. The configuration was changed and hitting the endpoint one more time no log was visible in the bridge pod. What's the difference with your test Tom?

tombentley commented 5 years ago

I was trying to configure the TO so that loggers in the io.strimzi category were DEBUG but everything else was INFO. I didn't manage to do that using inline, only external logging. One difference is that TO uses log4j2, whereas it seems that the bridge uses log4j. I suppose the CO's "validation" understands log4j.properties but not log4j2.properties.

There's two parts to addressing this:

  1. Improve the CO validation so it works (or at least works better) with log4j2. Better error messages when the config cannot be parsed, for example would help. But ideally we'd remove unnecessary limitations.
  2. Improve the documentation:
    • Explain the necessary limitations of inline logging (if, by that point, there are any).
    • For each component note whether it takes a log4j config or a log4j2 config.
    • For each component give an example with log4j config or a log4j2 config including actual logger names (and for external show the contents of the ConfigMap).
sknot-rh commented 5 years ago

The "validation" takes a {whatever}DefaultLoggingProperties file and its content is considered as the list of all loggers which can be set using inline type of logging. If the logger, you trying to set, is not the one from the default loggers, you should get a WARN message about that (https://github.com/strimzi/strimzi-kafka-operator/blob/master/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/AbstractModel.java#L359). So if you want to add other loggers, they have to be in {whatever}DefaultLoggingProperties file or we should remove the check from the code mentioned above. What do you think @scholzj @ppatierno @tombentley ?

tombentley commented 5 years ago

What are we gaining from having this check? AFAICS we shouldn't care about how the user wants to configure logging except we don't really want to allow logging anything other than stdout.