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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Log4j v1 support completely, even with log4j-1.2-api bridge #11925

Merged
merged 1 commit into from Dec 16, 2021

Conversation

vorburger
Copy link
Contributor

@vorburger vorburger commented Dec 14, 2021

I would like to propose now fully removing support for Log4j v1 entirely, and not even have a log4j-1.2-api dependency.

Having found past discussions about this in related previous #11263 #11264 #11265, and accepting that this PR may get rejected if you consider that matter settled for good, I would like to make an attempt and retry to convince you otherwise anyway, with a new rationale:

As I've attempted to clarify through https://github.com/apache/logging-log4j2/pull/629/files, I would argue that https://logging.apache.org/log4j/2.x/manual/migration.html is primarly useful and intended for "applications". I'm not so sure all this added extra complexity and delegation and layering is really in any of our interest for "libraries" (frameworks), such as Netty. The only reason this a library/framework project would want to do this is that you wish allow an application using Netty to keep Log4j v1 as their main logging framework. Except with log4j-1.2-api you anyway aren't really - because such applications have to change their configuration files from Log2j v1 to v2 format! I do understand that one COULD use Netty in an application and not use this log4j-1.2-api bridge to Log4j v2 but add a depenency to the old Log4j v1 Core (not Bridge). That would technically works, sure, but this enables the use of a version of a logging library that has been officially unmainainted for many years, which is partially broken on Java 9+, and which has at least one known security vulnernability which will never be patched. Perhaps some would say "But this is not the concern of libarires/frameworks!" I personally may even agreed until a few days ago... 馃槇 But given the fun we've all had over the last few days, I posit that "eternal API backwards compatibility" is overrated for something like this, and what's why I'm (re)proposing to fully remove support for Log4j v1 entirely, and not even have a log4j-1.2-api dependency in Netty anymore. It's simpler, clearer, more secure. The only drawback is for anyone insisting on using Netty an unmaintained version v1 of the Log4j logging library. It seems to me that an actively maintained project has no reason to offer that kind of backwards compability guarantee.

This PR will fail to build as-is initally submitted, but I couldn't for the life of me figure out how to correctly configure the revapi-maven-plugin (https://revapi.org) to accept the removal of Log4JLoggerFactory and Log4JLogger (which are in .internal. packages anyway - don't you want to ignore all changes to those anyway?) - I've tried a bunch of things, and somehow none of them worked; if this PR is acceptable in principle, I'm sure someone could figure that detail out.

@normanmaurer @Stwissel @chrisvest

Sorry if this was long, and TX for reading.

@vorburger
Copy link
Contributor Author

@normanmaurer
Copy link
Member

I think we can do this for main but not for 4.1 as it would be a "breaking change". Can you rebase against main ?

@vorburger vorburger changed the base branch from 4.1 to main December 15, 2021 14:50
@vorburger
Copy link
Contributor Author

I think we can do this for main but not for 4.1 as it would be a "breaking change". Can you rebase against main ?

Cool. Done! Merge? (It technically wasn't a rebase but a cherry-pick - and base branch changed accordingly.)

Original proposal for 4.1 kept on 4.1...vorburger:rm-log4j-1.2-api_4.1, just FTR.

@normanmaurer normanmaurer added this to the 4.1.72.Final milestone Dec 15, 2021
@normanmaurer normanmaurer removed this from the 4.1.72.Final milestone Dec 15, 2021
@normanmaurer normanmaurer merged commit 6b36175 into netty:main Dec 16, 2021
@normanmaurer
Copy link
Member

@vorburger thanks a lot!

@normanmaurer normanmaurer added this to the 5.0.0.Final milestone Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants