Add errorprone to prevent Log4j direct usage #3759
Conversation
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Diego López León <dieguitoll@gmail.com>
| @@ -25,6 +25,7 @@ | |||
|
|
|||
| import java.util.Arrays; | |||
| import java.util.Optional; | |||
| import java.util.Set; | |||
|
|
|||
| import org.apache.logging.log4j.Level; | |||
There was a problem hiding this comment.
Can this use of Log4J be removed? How needed is it?
There was a problem hiding this comment.
Yes. We can make o.h.b.util.Log4j2ConfiguratorUtil#setAllLevels to receive a string instead of the o.a.l.log4j.Level. That's a good point, it would encapsulate Log4j2 even further
| "Do not use junit assertions. Use assertj assertions instead."); | ||
| "Do not use junit assertions. Use assertj assertions instead.", | ||
| staticMethod().onClass("org.apache.logging.log4j.LogManager"), | ||
| "Do not use org.apache.logging.log4j.LogManager, use org.slf4j.LoggerFactory methods instead."); |
There was a problem hiding this comment.
Should this include the other log4j methods that were removed in this PR? static methods on org.apache.logging.log4j.core.config.Configurator, org.apache.logging.log4j.ThreadContext, instance methods on import org.apache.logging.log4j.core.LoggerContext, org.apache.logging.log4j.core.config.Configuration,
org.apache.logging.log4j.core.config.LoggerConfig? Or is there a way to go full Ender-Wiggin on it and have it fail for any class from the log4j packages?
There was a problem hiding this comment.
No, we cannot rip off the whole package because we still have hard dependencies on it. The most visible one is o.h.b.util.Log4j2ConfiguratorUtil but we also depend on o.a.l.log4j.Level for the CLI. I went on banning just o.a.l.log4j.LogManager b/c it's the most common entry point to the Log4j framework from the day to day developer perspective
Signed-off-by: Diego López León <dieguitoll@gmail.com>
|
Kudos, SonarCloud Quality Gate passed! |
* Replace Log4j2 API with SLF4j API * Replace explicit Log4J2 with util call * Replace ThreadContext with Slf4J's MDC in test * Inspect raw request parameter for admin_changeLogLevel * Add errorprone rule to prevent the creation Log4j2 loggers Signed-off-by: Diego López León <dieguitoll@gmail.com> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
PR description
This pull request contains some leftovers from #3285 and, most importantly, adds an error prone check for preventing Log4j usage.
Most of this commits were part of #3425 but they mostly abstract stuff from Log4j and are not Logback specific so I thought it would make sense to introduce them here as well.
Here is the full list of changes:
Log4j2ConfiguratorUtil(this is for simplifying the last diff)ThreadContextusage with Slf4j'sMDCo.a.l.log4j.LogManagerusagesDocumentation
doc-change-requiredlabel to this PR ifupdates are required.
Changelog