search

31z4 / zookeeper-docker

Docker image packaging for Apache Zookeeper
MIT License
285 stars 244 forks source link

Zookeeper switched from log4j to Logback in v3.8.2 #156

Closed janhoy closed 1 year ago

janhoy commented 1 year ago

Describe the bug

This is mainly a documentation bug. The README says that you can configure logging to a file by setting ZOO_LOG4J_PROP. This no longer works as of v3.8.0.

To Reproduce

Try the ZOO_LOG4J_PROP var and see it won't work.

Proposal

A proposed solution could be to add our own logback.cfg file to this repository that is copied in by Dockerfile. This config could then allow selecting between a number of different log appenders (including potentially the JSON one mentioned in #154), so that we could document it as follows

$ docker run --name some-zookeeper --restart always -e ZOO_LOG_APPENDER=ROLLINGFILE -e ZOO_LOG_LEVEL=INFO zookeeper
31z4 commented 1 year ago

Thanks for the heads up! I've updated the docs with actual info on logging configuration. See https://github.com/docker-library/docs/pull/2380

As for shipping a custom logback config with the image I'm hesitant to do so. Because it would require additional maintenance efforts to keep this config in sync with the upstream (e.g., changes in default Zookeeper config or logback version upgrades). I could not afford such additional maintenance costs at this moment unfortunately. I think having a simple way to override the default config by mounting a volume (described in the docs PR) should be good enough.

janhoy commented 1 year ago

That's a fair tradeoff.

31z4 commented 1 year ago

Closing this because https://github.com/docker-library/docs/pull/2380 is merged.