SOLR-7825: Forbid all usages of log4j and java.util.logging classes in Solr

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1692624 13f79535-47bb-0310-9956-ffa450edef68

lucene/tools/forbiddenApis/solr.txt

@@ -26,4 +26,8 @@ java.util.concurrent.Executors#newCachedThreadPool(java.util.concurrent.ThreadFa
 java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory,java.util.concurrent.RejectedExecutionHandler)
 java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue)
 java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.ThreadFactory)
-java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.RejectedExecutionHandler)
+java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent.TimeUnit,java.util.concurrent.BlockingQueue,java.util.concurrent.RejectedExecutionHandler)
+
+@defaultMessage Use slf4j classes instead
+org.apache.log4j.**
+java.util.logging.**

solr/CHANGES.txt

@@ -349,6 +349,12 @@ Other Changes
   formatting and parsing out of TrieDateField and move to static utility class
   DateFormatUtil.  (Markus Heiden, Uwe Schindler)
 
+* SOLR-7825: Forbid all usages of log4j and java.util.logging classes in Solr except
+  classes which are specific to logging implementations. Remove accidental usage of log4j
+  logger from a few places. The default log level for org.apache.zookeeper is changed from
+  ERROR to WARN for zkcli.{sh,cmd} only.
+  (Oliver Schrenk, Tim Potter, Uwe Schindler, shalin)
+
 ==================  5.2.1 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release

solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/MapReduceIndexerTool.java

@@ -74,7 +74,6 @@ import org.apache.hadoop.mapreduce.lib.output.TextOutputFormat;
 import org.apache.hadoop.util.GenericOptionsParser;
 import org.apache.hadoop.util.Tool;
 import org.apache.hadoop.util.ToolRunner;
-import org.apache.log4j.PropertyConfigurator;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.hadoop.dedup.RetainMostRecentUpdateConflictResolver;
 import org.apache.solr.hadoop.morphline.MorphlineMapRunner;
@@ -483,7 +482,7 @@ public class MapReduceIndexerTool extends Configured implements Tool {
       
       opts.log4jConfigFile = (File) ns.get(log4jConfigFileArg.getDest());
       if (opts.log4jConfigFile != null) {
-        PropertyConfigurator.configure(opts.log4jConfigFile.getPath());        
+        Utils.configureLog4jProperties(opts.log4jConfigFile.getPath());
       }
       LOG.debug("Parsed command line args: {}", ns);
       

solr/contrib/map-reduce/src/java/org/apache/solr/hadoop/Utils.java

@@ -22,6 +22,7 @@ import org.apache.hadoop.conf.Configuration;
 import org.apache.log4j.PropertyConfigurator;
 
 import com.google.common.annotations.Beta;
+import org.apache.solr.common.util.SuppressForbidden;
 
 
 @Beta
@@ -35,6 +36,11 @@ public final class Utils {
 
   public static void getLogConfigFile(Configuration conf) {
     String log4jPropertiesFile = conf.get(LOG_CONFIG_FILE);
+    configureLog4jProperties(log4jPropertiesFile);
+  }
+
+  @SuppressForbidden(reason = "method is specific to log4j")
+  public static void configureLog4jProperties(String log4jPropertiesFile) {
     if (log4jPropertiesFile != null) {
       PropertyConfigurator.configure(log4jPropertiesFile);
     }

solr/core/src/java/org/apache/solr/core/ZkContainer.java

@@ -25,10 +25,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.TimeoutException;
 
-import org.apache.log4j.MDC;
 import org.apache.solr.cloud.CurrentCoreDescriptorProvider;
 import org.apache.solr.cloud.SolrZkServer;
 import org.apache.solr.cloud.ZkController;
@@ -43,8 +41,6 @@ import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.solr.common.cloud.ZkStateReader.CORE_NAME_PROP;
-
 public class ZkContainer {
   protected static Logger log = LoggerFactory.getLogger(ZkContainer.class);
   
@@ -201,7 +197,7 @@ public class ZkContainer {
             SolrException.log(log, "", e);
           }
         } finally {
-          MDC.clear();
+          MDCLoggingContext.clear();
         }
       }
       
@@ -215,7 +211,7 @@ public class ZkContainer {
         try {
           thread.run();
         } finally {
-          MDC.clear();
+          MDCLoggingContext.clear();
         }
       }
     }

solr/core/src/java/org/apache/solr/handler/component/RangeFacetProcessor.java

@@ -23,7 +23,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
-import org.apache.log4j.Logger;
 import org.apache.lucene.search.Query;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.FacetParams.FacetRangeMethod;
@@ -41,11 +40,14 @@ import org.apache.solr.schema.TrieField;
 import org.apache.solr.search.DocSet;
 import org.apache.solr.search.SyntaxError;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * Processor for Range Facets
  */
 public class RangeFacetProcessor extends SimpleFacets {
-  private final static Logger log = Logger.getLogger(RangeFacetProcessor.class);
+  private final static Logger log = LoggerFactory.getLogger(RangeFacetProcessor.class);
 
   public RangeFacetProcessor(SolrQueryRequest req, DocSet docs, SolrParams params, ResponseBuilder rb) {
     super(req, docs, params, rb);

solr/core/src/java/org/apache/solr/handler/component/RangeFacetRequest.java

@@ -24,7 +24,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.log4j.Logger;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.FacetParams;
 import org.apache.solr.common.params.GroupParams;
@@ -41,12 +40,15 @@ import org.apache.solr.schema.TrieField;
 import org.apache.solr.util.DateMathParser;
 import org.apache.solr.util.DateFormatUtil;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * Encapsulates a single facet.range request along with all its parameters. This class
  * calculates all the ranges (gaps) required to be counted.
  */
 public class RangeFacetRequest extends FacetComponent.FacetBase {
-  private final static Logger log = Logger.getLogger(RangeFacetRequest.class);
+  private final static Logger log = LoggerFactory.getLogger(RangeFacetRequest.class);
 
   protected final SchemaField schemaField;
   protected final String start;

solr/core/src/java/org/apache/solr/logging/jul/JulInfo.java

@@ -19,8 +19,10 @@ package org.apache.solr.logging.jul;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.LoggerInfo;
 
+@SuppressForbidden(reason = "class is specific to java.util.logging")
 public class JulInfo extends LoggerInfo {
   private static final Level[] LEVELS = {
       null, // aka unset

solr/core/src/java/org/apache/solr/logging/jul/JulWatcher.java

@@ -18,6 +18,7 @@ package org.apache.solr.logging.jul;
 
 import com.google.common.base.Throwables;
 import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.CircularList;
 import org.apache.solr.logging.ListenerConfig;
 import org.apache.solr.logging.LogWatcher;
@@ -35,6 +36,7 @@ import java.util.logging.LogManager;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 
+@SuppressForbidden(reason = "class is specific to java.util.logging")
 public class JulWatcher extends LogWatcher<LogRecord> {
 
   final String name;

solr/core/src/java/org/apache/solr/logging/jul/RecordHandler.java

@@ -19,8 +19,10 @@ package org.apache.solr.logging.jul;
 
 import java.util.logging.LogRecord;
 
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.LogWatcher;
 
+@SuppressForbidden(reason = "class is specific to java.util.logging")
 public final class RecordHandler extends java.util.logging.Handler {
   final LogWatcher<LogRecord> framework;
   

solr/core/src/java/org/apache/solr/logging/log4j/EventAppender.java

@@ -18,9 +18,10 @@ package org.apache.solr.logging.log4j;
 
 import org.apache.log4j.AppenderSkeleton;
 import org.apache.log4j.spi.LoggingEvent;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.LogWatcher;
 
-
+@SuppressForbidden(reason = "class is specific to log4j")
 public final class EventAppender extends AppenderSkeleton {
 
   final LogWatcher<LoggingEvent> watcher;

solr/core/src/java/org/apache/solr/logging/log4j/Log4jInfo.java

@@ -16,8 +16,10 @@
  */
 package org.apache.solr.logging.log4j;
 
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.LoggerInfo;
 
+@SuppressForbidden(reason = "class is specific to log4j")
 public class Log4jInfo extends LoggerInfo {
   final org.apache.log4j.Logger logger;
 

solr/core/src/java/org/apache/solr/logging/log4j/Log4jWatcher.java

@@ -31,6 +31,7 @@ import org.apache.log4j.spi.LoggingEvent;
 import org.apache.log4j.spi.ThrowableInformation;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.CircularList;
 import org.apache.solr.logging.ListenerConfig;
 import org.apache.solr.logging.LogWatcher;
@@ -38,6 +39,7 @@ import org.apache.solr.logging.LoggerInfo;
 
 import com.google.common.base.Throwables;
 
+@SuppressForbidden(reason = "class is specific to log4j")
 public class Log4jWatcher extends LogWatcher<LoggingEvent> {
 
   final String name;

solr/core/src/java/org/apache/solr/util/SolrCLI.java

@@ -58,9 +58,6 @@ import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.conn.ConnectTimeoutException;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.util.EntityUtils;
-import org.apache.log4j.Level;
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServerException;
@@ -84,6 +81,8 @@ import org.noggit.CharArr;
 import org.noggit.JSONParser;
 import org.noggit.JSONWriter;
 import org.noggit.ObjectBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import static org.apache.solr.common.params.CommonParams.NAME;
 
@@ -113,10 +112,6 @@ public class SolrCLI {
     
     public int runTool(CommandLine cli) throws Exception {
       
-      // quiet down the ZK logging for cli tools
-      LogManager.getLogger("org.apache.zookeeper").setLevel(Level.ERROR);
-      LogManager.getLogger("org.apache.solr.common.cloud").setLevel(Level.WARN);
-      
       String zkHost = cli.getOptionValue("zkHost", ZK_HOST);
       
       log.debug("Connecting to Solr cluster: " + zkHost);
@@ -150,7 +145,7 @@ public class SolrCLI {
         throws Exception;
   }
   
-  public static Logger log = Logger.getLogger(SolrCLI.class);    
+  public static Logger log = LoggerFactory.getLogger(SolrCLI.class);
   public static final String DEFAULT_SOLR_URL = "http://localhost:8983/solr";  
   public static final String ZK_HOST = "localhost:9983";
   
@@ -1116,8 +1111,6 @@ public class SolrCLI {
       if (zkHost == null)
         throw new IllegalStateException("Must provide either the '-solrUrl' or '-zkHost' parameters!");
 
-      LogManager.getLogger("org.apache.zookeeper").setLevel(Level.ERROR);
-      LogManager.getLogger("org.apache.solr.common.cloud").setLevel(Level.WARN);
       try (CloudSolrClient cloudSolrClient = new CloudSolrClient(zkHost)) {
         cloudSolrClient.connect();
         Set<String> liveNodes = cloudSolrClient.getZkStateReader().getClusterState().getLiveNodes();
@@ -1203,10 +1196,6 @@ public class SolrCLI {
 
     public int runTool(CommandLine cli) throws Exception {
 
-      // quiet down the ZK logging for cli tools
-      LogManager.getLogger("org.apache.zookeeper").setLevel(Level.ERROR);
-      LogManager.getLogger("org.apache.solr.common.cloud").setLevel(Level.WARN);
-
       String zkHost = getZkHost(cli);
       if (zkHost == null) {
         System.err.println("\nERROR: Solr at "+cli.getOptionValue("solrUrl")+
@@ -1607,10 +1596,6 @@ public class SolrCLI {
     @Override
     public int runTool(CommandLine cli) throws Exception {
 
-      // quiet down the ZK logging for cli tools
-      LogManager.getLogger("org.apache.zookeeper").setLevel(Level.ERROR);
-      LogManager.getLogger("org.apache.solr.common.cloud").setLevel(Level.WARN);
-
       String solrUrl = cli.getOptionValue("solrUrl", DEFAULT_SOLR_URL);
       if (!solrUrl.endsWith("/"))
         solrUrl += "/";

solr/core/src/java/org/apache/solr/util/SolrLogLayout.java

@@ -13,6 +13,7 @@ import org.apache.solr.cloud.ZkController;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.StringUtils;
 import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestInfo;
@@ -41,6 +42,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
  * limitations under the License.
  */
 
+@SuppressForbidden(reason = "class is specific to log4j")
 public class SolrLogLayout extends Layout {
   /**
    * Add this interface to a thread group and the string returned by getTag()

solr/core/src/test/org/apache/solr/cloud/ConcurrentDeleteAndCreateCollectionTest.java

@@ -22,8 +22,6 @@ import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
-import org.apache.log4j.Level;
-import org.apache.log4j.Logger;
 import org.apache.lucene.util.LuceneTestCase.Nightly;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.SolrClient;
@@ -75,37 +73,28 @@ public class ConcurrentDeleteAndCreateCollectionTest extends SolrTestCaseJ4 {
   }
   
   public void testConcurrentCreateAndDeleteOverTheSameConfig() {
-    // TODO: no idea what this test needs to override the level, but regardless of reason it should
-    // reset when it's done.
-    final Logger logger = Logger.getLogger("org.apache.solr");
-    final Level SAVED_LEVEL = logger.getLevel();
+    final String configName = "testconfig";
+    final File configDir = getFile("solr").toPath().resolve("configsets/configset-2/conf").toFile();
+    uploadConfig(configDir, configName); // upload config once, to be used by all collections
+    final SolrClient solrClient = new HttpSolrClient(solrCluster.getJettySolrRunners().get(0).getBaseUrl().toString());
+    final AtomicReference<Exception> failure = new AtomicReference<>();
+    final int timeToRunSec = 30;
+    final Thread[] threads = new Thread[2];
+    for (int i = 0; i < threads.length; i++) {
+      final String collectionName = "collection" + i;
+      threads[i] = new CreateDeleteCollectionThread("create-delete-" + i, collectionName, configName,
+                                                    timeToRunSec, solrClient, failure);
+    }
+
+    startAll(threads);
+    joinAll(threads);
+
+    assertNull("concurrent create and delete collection failed: " + failure.get(), failure.get());
+
     try {
-      logger.setLevel(Level.WARN);
-      final String configName = "testconfig";
-      final File configDir = getFile("solr").toPath().resolve("configsets/configset-2/conf").toFile();
-      uploadConfig(configDir, configName); // upload config once, to be used by all collections
-      final SolrClient solrClient = new HttpSolrClient(solrCluster.getJettySolrRunners().get(0).getBaseUrl().toString());
-      final AtomicReference<Exception> failure = new AtomicReference<>();
-      final int timeToRunSec = 30;
-      final Thread[] threads = new Thread[2];
-      for (int i = 0; i < threads.length; i++) {
-        final String collectionName = "collection" + i;
-        threads[i] = new CreateDeleteCollectionThread("create-delete-" + i, collectionName, configName, 
-                                                      timeToRunSec, solrClient, failure);
-      }
-    
-      startAll(threads);
-      joinAll(threads);
-    
-      assertNull("concurrent create and delete collection failed: " + failure.get(), failure.get());
-      
-      try {
-        solrClient.close();
-      } catch (IOException e) {
-        throw new RuntimeException(e);
-      }
-    } finally {
-      logger.setLevel(SAVED_LEVEL);
+      solrClient.close();
+    } catch (IOException e) {
+      throw new RuntimeException(e);
     }
   }
   

solr/core/src/test/org/apache/solr/handler/RequestLoggingTest.java

@@ -27,11 +27,13 @@ import org.apache.log4j.Logger;
 import org.apache.log4j.SimpleLayout;
 import org.apache.log4j.WriterAppender;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.core.SolrCore;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+@SuppressForbidden(reason = "test is specific to log4j")
 public class RequestLoggingTest extends SolrTestCaseJ4 {
   private StringWriter writer;
   private Appender appender;

solr/core/src/test/org/apache/solr/handler/admin/LoggingHandlerTest.java

@@ -21,11 +21,13 @@ import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.log4j.Log4jInfo;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 
+@SuppressForbidden(reason = "test uses log4j because it tests output at a specific level")
 public class LoggingHandlerTest extends SolrTestCaseJ4 {
 
   // TODO: This only tests Log4j at the moment, as that's what's defined

solr/server/scripts/cloud-scripts/log4j.properties

@@ -6,3 +6,7 @@ log4j.appender.stderr = org.apache.log4j.ConsoleAppender
 log4j.appender.stderr.Target = System.err
 log4j.appender.stderr.layout = org.apache.log4j.PatternLayout
 log4j.appender.stderr.layout.ConversionPattern=%-5p - %d{yyyy-MM-dd HH:mm:ss.SSS}; %C; %m\n
+
+# quiet down the ZK logging for cli tools
+log4j.logger.org.apache.zookeeper=WARN
+log4j.logger.org.apache.solr.common.cloud=WARN

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -78,6 +78,7 @@ import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ObjectReleaseTracker;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.common.util.XML;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoreDescriptor;
@@ -391,6 +392,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     super.tearDown();
   }
 
+  @SuppressForbidden(reason = "method is specific to java.util.logging and highly suspect!")
   public static void setLoggingLevel(Level level) {
     java.util.logging.Logger logger = java.util.logging.Logger.getLogger("");
     logger.setLevel(level);