> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the code. > > Tomás Fernández Löbbe wrote: > Not entierly sure. We **don't** do log4j calls in the code in general, we > have that explicitly forbidden in forbidden APIS today, and code that does > something with log4j has to supress that. Developers must instead use slf4j > APIs. I don't believe that's changing now with log4j2, or does it? > > Shawn Heisey wrote: > ``` > Not entierly sure. We don't do log4j calls in the code in general, we > have that explicitly forbidden in forbidden APIS today, and code that does > something with log4j has to supress that. Developers must instead use slf4j > APIs. I don't believe that's changing now with log4j2, or does it? > ``` > > Solr only uses log4j code in one place -- our log watcher for the admin > UI. This probably has a notation to disable the forbidden API check. Lucene > doesn't use any logging at all as far as I know. > > The reason that we need the 1.2 compatibility shim is not for Solr or > Lucene. It's for dependencies. One dependency that I know for sure uses it > is zookeeper. There might be others, but I haven't done an exhaustive check > to find out. > > Tomás Fernández Löbbe wrote: > That is exactly my point. Solr doesn't use log4j in the code and it's > forbidden. We need to include the log4j2 packages in the list of forbidden so > that developers don't try to use it (unless they know what they are doing and > explicitly remove the forbidden check)
Sounds like we are in violent agreement, then. :) Yes, we should make a general rule of forbidding log4j usage, both 1.2 and 2.x, and provide an exception for the one place where we use it intentionally. Am I correct in thinking that forbidden APIs won't complain about usage in dependencies, just the class files that our code builds? - Shawn ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/#review198580 ----------------------------------------------------------- On March 3, 2018, 2:51 a.m., Varun Thacker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65888/ > ----------------------------------------------------------- > > (Updated March 3, 2018, 2:51 a.m.) > > > Review request for lucene. > > > Repository: lucene-solr > > > Description > ------- > > Upgrade Solr to use Log4J2 > > > Diffs > ----- > > lucene/CHANGES.txt e3799bc9d5 > lucene/common-build.xml 4fa59ac936 > lucene/ivy-versions.properties 5ab36ddfa2 > solr/CHANGES.txt 05f4f560bd > solr/bin/install_solr_service.sh b82957144d > solr/bin/solr 4e178de945 > solr/bin/solr.cmd dcff0c6af7 > solr/bin/solr.in.cmd bfb33e0e9d > solr/bin/solr.in.sh e7478cdf5c > solr/contrib/clustering/src/test-files/log4j.properties b5216db8b2 > solr/contrib/clustering/src/test-files/log4j2.xml PRE-CREATION > solr/contrib/dataimporthandler/src/test-files/log4j.properties d3ea4deafc > solr/contrib/dataimporthandler/src/test-files/log4j2.xml PRE-CREATION > solr/contrib/ltr/src/test-files/log4j.properties d86c6988d5 > solr/contrib/ltr/src/test-files/log4j2.xml PRE-CREATION > solr/core/ivy.xml ff4fa48679 > > solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java > 23a8dc1eb3 > solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java > 122d2cbf8b > solr/core/src/java/org/apache/solr/logging/LogWatcher.java c510590282 > solr/core/src/java/org/apache/solr/logging/log4j/EventAppender.java > ff2876fb2f > solr/core/src/java/org/apache/solr/logging/log4j/Log4jInfo.java dfd3dde74a > solr/core/src/java/org/apache/solr/logging/log4j/Log4jWatcher.java > 04fa5fb1d8 > solr/core/src/java/org/apache/solr/logging/log4j/package-info.java > f78953385c > solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java > PRE-CREATION > solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java > 4d944d239b > solr/core/src/java/org/apache/solr/util/SolrCLI.java 266ef3a28a > solr/core/src/java/org/apache/solr/util/SolrLogLayout.java a60ada828b > solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java c582eff4c0 > solr/core/src/test-files/log4j.properties 969439a228 > solr/core/src/test-files/log4j2.xml PRE-CREATION > solr/core/src/test/org/apache/solr/handler/RequestLoggingTest.java > 4c780ccda4 > solr/core/src/test/org/apache/solr/handler/admin/LoggingHandlerTest.java > 555c1376a5 > solr/core/src/test/org/apache/solr/logging/log4j2/Log4j2WatcherTest.java > PRE-CREATION > solr/core/src/test/org/apache/solr/util/TestSolrCLIRunExample.java > 89008517f8 > solr/example/README.txt 562c256377 > solr/example/example-DIH/solr/db/conf/solrconfig.xml 1ffbbe817f > solr/example/example-DIH/solr/mail/conf/solrconfig.xml 770b0fd870 > solr/example/example-DIH/solr/solr/conf/solrconfig.xml 3f00141340 > solr/example/resources/log4j.properties 02f91c5dae > solr/example/resources/log4j2.xml PRE-CREATION > solr/licenses/audience-annotations-0.7.0.jar.sha1 PRE-CREATION > solr/licenses/audience-annotations-LICENSE-ASL.txt PRE-CREATION > solr/licenses/audience-annotations-NOTICE.txt PRE-CREATION > solr/licenses/disruptor-3.4.0.jar.sha1 PRE-CREATION > solr/licenses/disruptor-LICENSE-ASL.txt PRE-CREATION > solr/licenses/disruptor-NOTICE.txt PRE-CREATION > solr/licenses/log4j-1.2-api-2.10.0.jar.sha1 PRE-CREATION > solr/licenses/log4j-1.2.17.jar.sha1 383110e29f > solr/licenses/log4j-api-2.10.0.jar.sha1 PRE-CREATION > solr/licenses/log4j-api-LICENSE-ASL.txt PRE-CREATION > solr/licenses/log4j-api-NOTICE.txt PRE-CREATION > solr/licenses/log4j-core-2.10.0.jar.sha1 PRE-CREATION > solr/licenses/log4j-core-LICENSE-ASL.txt PRE-CREATION > solr/licenses/log4j-core-NOTICE.txt PRE-CREATION > solr/licenses/log4j-slf4j-LICENSE-ASL.txt PRE-CREATION > solr/licenses/log4j-slf4j-NOTICE.txt PRE-CREATION > solr/licenses/log4j-slf4j-impl-2.10.0.jar.sha1 PRE-CREATION > solr/licenses/slf4j-log4j12-1.7.24.jar.sha1 b8ec050172 > solr/server/README.txt 228f4d467b > solr/server/ivy.xml c9b3a73014 > solr/server/resources/log4j.properties 9f9c4a0f74 > solr/server/resources/log4j2.xml PRE-CREATION > solr/server/scripts/cloud-scripts/log4j.properties 5f2ae18574 > solr/server/scripts/cloud-scripts/log4j2.xml PRE-CREATION > solr/server/scripts/cloud-scripts/snapshotscli.sh f885721f66 > solr/server/scripts/cloud-scripts/zkcli.bat c5d7b72948 > solr/server/scripts/cloud-scripts/zkcli.sh bd971e9ee4 > solr/server/solr/configsets/sample_techproducts_configs/conf/solrconfig.xml > 1b2563662e > solr/solr-ref-guide/ivy.xml adefe2ced5 > solr/solr-ref-guide/src/configuring-logging.adoc 8984744e55 > solr/solr-ref-guide/src/solr-control-script-reference.adoc 8ca14ea476 > solr/solr-ref-guide/src/taking-solr-to-production.adoc ca0f4ebe76 > solr/solrj/ivy.xml 3637bc34bd > solr/solrj/src/test-files/log4j.properties dae4f6f418 > solr/solrj/src/test-files/log4j2.xml PRE-CREATION > solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java 706f1ebe8b > solr/test-framework/src/java/org/apache/solr/util/LogLevel.java 7542694441 > solr/test-framework/src/test-files/log4j.properties f6fedb6ea2 > solr/test-framework/src/test-files/log4j2.xml PRE-CREATION > solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java > 2ede874e50 > > > Diff: https://reviews.apache.org/r/65888/diff/2/ > > > Testing > ------- > > > Thanks, > > Varun Thacker > >
