Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

log4j 1.2.17 has reached End of life. Plans of upgrade? #17697

Closed
SKumarMN opened this issue Apr 13, 2016 · 14 comments · Fixed by #20235
Closed

log4j 1.2.17 has reached End of life. Plans of upgrade? #17697

SKumarMN opened this issue Apr 13, 2016 · 14 comments · Fixed by #20235
Assignees
Labels

Comments

@SKumarMN
Copy link

Elasticsearch version: 2.0.2/2.1.0

As the version of log4j 1.2.17 lib used by ES has reached end of life as on Aug ,2015(https://logging.apache.org/log4j/1.2/index.html), and is recommend to migrate to log4j 2.X version what are the plans of ES team to use new versions of log4j.
In what releases of ES can we expect new versions of log4j. If any security vulnerability are reported on log4j 1.2.17 lib in the future, what are the plans of mitigating the risks so that affects would be minimal.

@clintongormley clintongormley added discuss :Core/Infra/Logging Log management and logging utilities labels Apr 13, 2016
@jpountz
Copy link
Contributor

jpountz commented Apr 15, 2016

Discussed in FixitFriday: we need to upgrade indeed. Since the configuration format will likely have some minor differences, this is probably a good opportunity to remove the layer we have on top of log4j to use a yaml configuration file. This would be more user friendly as the log4j documentation would be directly applicable.

@nik9000
Copy link
Member

nik9000 commented Apr 15, 2016

If we upgrade do we go to log4j 2 or something like that?
On Apr 15, 2016 5:53 AM, "Adrien Grand" notifications@github.com wrote:

Discussed in FixitFriday: we need to upgrade indeed. Since the
configuration format will likely have some minor differences, this is
probably a good opportunity to remove the layer we have on top of log4j to
use a yaml configuration file. This would be more user friendly as the
log4j documentation would be directly applicable.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#17697 (comment)

@jasontedor
Copy link
Member

jasontedor commented Apr 15, 2016

If we upgrade do we go to log4j 2 or something like that?

Yeah, the proposal is:

  • upgrade to log4j 2
  • drop our custom log4j configuration and just enable regular log4j configuration
  • target a major release, probably not 5.0
  • fork log4j 1 if the need arises

@dadoonet
Copy link
Member

I spoke some years ago with Log4J (v1) author about this and he told me that we should consider logback instead. Do we consider benchmarking one or the other solutions around? May be there are no difference and it's a no brainer though?

@rjernst
Copy link
Member

rjernst commented Apr 15, 2016

drop our custom log4j configuration and just enable regular log4j configuration

I don't think we should do that. It would force us to always stay with log4j, and would have to reimplement log4j configuration in order to eg try using java logging.

@nik9000
Copy link
Member

nik9000 commented Apr 15, 2016

I don't think we should do that. It would force us to always stay with log4j, and would have to reimplement log4j configuration in order to eg try using java logging.

We have that problem already. Our logging configuration now is just a thin wrapper around log4j so we'd have to reimplement it if we wanted continuity. I think we're better off telling users "the logging configuration is log4j 2.x" or "the logging configuration is jdk built in" and then telling them about the 6 or 8 interesting loggers. Then they can use the massive wealth of knowledge on the internet about configuring logging in those environments. They'll still need to know a bit about customizing it for elasticsearch but it won't be as specific as what we have now.

@jmoney
Copy link

jmoney commented Apr 17, 2016

Instead of the "custom" logging layer why isn't slf4j used. Logback and log4j2 both support the api. There are bridges too that allow log4j 1.x to delegate to the slf4j api which can then be backed by log4j2 to "bridge" a migration.

If a logging refactor is too happen might I suggest that be part of it to decouple future logging implementation upgrades.

@jasontedor
Copy link
Member

The simplest argument against is that means we add a dependency (slf4j plus a binding that we have to ship with for a net addition of a dependency), and we don't achieve our goal of keeping the number of logging configurations that we support to a minimum. We are going to keep it simple here.

Relates #16585

@ppf2
Copy link
Member

ppf2 commented Jun 30, 2016

One popular request is to provide an appender that will compress the ES logs and have MaxFileSize + MaxBackUpIndex. Currently, we have a commented out appender users can potentially use for gzipping of the logs. With log4j2, there may be an opportunity here to update the extrasRollingFile appender example commented out in the logging.yml so that it will not just compress the logs, but allow users to set a file size limit + number of backups. Thx!

@rmuir
Copy link
Contributor

rmuir commented Jul 14, 2016

Its not just about security releases, for example some features of log4j 1.x are broken with java 9, we have a hack around that: https://github.com/elastic/elasticsearch/blob/f01f15d3b8592db4210f725f0c37baff4a554a35/core/src/main/java/org/apache/log4j/Java9Hack.java

I think the should elevate the priority of this. This is not the place to bikeshed loggers (which are some of the largest bikesheds on the planet, by definition), instead we should simply move to something that is actively supported, and bikeshed on a separate issue.

Or we can use System.out.println for all logging, I am fine with that, it is supported method by the JDK.

@s1monw
Copy link
Contributor

s1monw commented Jul 18, 2016

I think we should prioritize this even for 5.0?

target a major release, probably not 5.0

@jasontedor any reasons?

@clintongormley I wonder if this should be a blocker?

@jasontedor
Copy link
Member

any reasons?

That comment was made in April and is out of date now. We are now intending to target 5.0.0.

@s1monw
Copy link
Contributor

s1monw commented Jul 18, 2016

thanks @jasontedor

@bosinm
Copy link

bosinm commented Aug 5, 2016

Another common request is to have the option of combining MaxBackUPIndex from RollingFileAppender with DailyRollingFileAppender. The idea is to have automatic deletion of the rolled files after X number of days.
So where the logger finally selected should be able to combine both functionalities . Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.