Logger: Merge ESLoggerFactory into Loggers

client/rest-high-level/src/main/resources/forbidden/rest-high-level-signatures.txt

@@ -22,7 +22,6 @@ org.apache.http.entity.ContentType#create(java.lang.String,org.apache.http.NameV
 
 @defaultMessage ES's logging infrastructure uses log4j2 which we don't want to force on high level rest client users
 org.elasticsearch.common.logging.DeprecationLogger
-org.elasticsearch.common.logging.ESLoggerFactory
 org.elasticsearch.common.logging.LogConfigurator
 org.elasticsearch.common.logging.LoggerMessageFormat
 org.elasticsearch.common.logging.Loggers

qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java

@@ -27,7 +27,6 @@
 import org.apache.logging.log4j.core.appender.ConsoleAppender;
 import org.apache.logging.log4j.core.appender.CountingNoOpAppender;
 import org.apache.logging.log4j.core.config.Configurator;
-import org.apache.logging.log4j.spi.ExtendedLogger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.cli.UserException;
@@ -301,7 +300,7 @@ public void testPrefixLogger() throws IOException, IllegalAccessException, UserE
         setupLogging("prefix");
 
         final String prefix = randomAlphaOfLength(16);
-        final Logger logger = new PrefixLogger((ExtendedLogger) LogManager.getLogger("prefix_test"), "prefix_test", prefix);
+        final Logger logger = new PrefixLogger(LogManager.getLogger("prefix_test"), prefix);
         logger.info("test");
         logger.info("{}", "test");
         final Exception e = new Exception("exception");
@@ -332,7 +331,7 @@ public void testPrefixLoggerMarkersCanBeCollected() throws IOException, UserExce
         final int prefixes = 1 << 19; // to ensure enough markers that the GC should collect some when we force a GC below
         for (int i = 0; i < prefixes; i++) {
             // this has the side effect of caching a marker with this prefix
-            new PrefixLogger((ExtendedLogger) LogManager.getLogger("prefix" + i), "prefix" + i, "prefix" + i);
+            new PrefixLogger(LogManager.getLogger("logger" + i), "prefix" + i);
         }
 
         System.gc(); // this will free the weakly referenced keys in the marker cache

server/src/main/java/org/elasticsearch/common/logging/Loggers.java

@@ -57,23 +57,24 @@ public static Logger getLogger(Class<?> clazz, ShardId shardId, String... prefix
      * Class and no extra prefixes.
      */
     public static Logger getLogger(String loggerName, ShardId shardId) {
-        return ESLoggerFactory.getLogger(formatPrefix(shardId.getIndexName(), Integer.toString(shardId.id())), loggerName);
+        String prefix = formatPrefix(shardId.getIndexName(), Integer.toString(shardId.id()));
+        return new PrefixLogger(LogManager.getLogger(loggerName), prefix);
     }
 
     public static Logger getLogger(Class<?> clazz, Index index, String... prefixes) {
         return getLogger(clazz, asArrayList(Loggers.SPACE, index.getName(), prefixes).toArray(new String[0]));
     }
 
     public static Logger getLogger(Class<?> clazz, String... prefixes) {
-        return ESLoggerFactory.getLogger(formatPrefix(prefixes), clazz);
+        return new PrefixLogger(LogManager.getLogger(clazz), formatPrefix(prefixes));
     }
 
     public static Logger getLogger(Logger parentLogger, String s) {
-        String prefix = null;
+        Logger inner = LogManager.getLogger(parentLogger.getName() + s);
         if (parentLogger instanceof PrefixLogger) {
-            prefix = ((PrefixLogger)parentLogger).prefix();
+            return new PrefixLogger(inner, ((PrefixLogger)parentLogger).prefix());
         }
-        return ESLoggerFactory.getLogger(prefix, parentLogger.getName() + s);
+        return inner;
     }
 
     private static String formatPrefix(String... prefixes) {

server/src/main/java/org/elasticsearch/common/logging/PrefixLogger.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.common.logging;
 
 import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.MarkerManager;
 import org.apache.logging.log4j.message.Message;
@@ -70,26 +71,27 @@ public String prefix() {
      * Construct a prefix logger with the specified name and prefix.
      *
      * @param logger the extended logger to wrap
-     * @param name   the name of this prefix logger
      * @param prefix the prefix for this prefix logger
      */
-    PrefixLogger(final ExtendedLogger logger, final String name, final String prefix) {
-        super(logger, name, null);
+    PrefixLogger(final Logger logger, final String prefix) {
+        super((ExtendedLogger) logger, logger.getName(), null);
 
-        final String actualPrefix = (prefix == null ? "" : prefix);
+        if (prefix == null || prefix.isEmpty()) {
+            throw new IllegalArgumentException("if you don't need a prefix then use a regular logger");
+        }
         final Marker actualMarker;
         // markers is not thread-safe, so we synchronize access
         synchronized (markers) {
-            final Marker maybeMarker = markers.get(actualPrefix);
+            final Marker maybeMarker = markers.get(prefix);
             if (maybeMarker == null) {
-                actualMarker = new MarkerManager.Log4jMarker(actualPrefix);
+                actualMarker = new MarkerManager.Log4jMarker(prefix);
                 /*
                  * We must create a new instance here as otherwise the marker will hold a reference to the key in the weak hash map; as
                  * those references are held strongly, this would give a strong reference back to the key preventing them from ever being
                  * collected. This also guarantees that no other strong reference can be held to the prefix anywhere.
                  */
                 // noinspection RedundantStringConstructorCall
-                markers.put(new String(actualPrefix), actualMarker);
+                markers.put(new String(prefix), actualMarker);
             } else {
                 actualMarker = maybeMarker;
             }

server/src/test/java/org/elasticsearch/common/logging/PrefixLoggerTests.java

@@ -0,0 +1,36 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.logging;
+
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.containsString;
+
+public class PrefixLoggerTests extends ESTestCase {
+    public void testNullPrefix() {
+        Exception e = expectThrows(IllegalArgumentException.class, () -> new PrefixLogger(logger, null));
+        assertThat(e.getMessage(), containsString("use a regular logger"));
+    }
+
+    public void testEmptyPrefix() {
+        Exception e = expectThrows(IllegalArgumentException.class, () -> new PrefixLogger(logger, ""));
+        assertThat(e.getMessage(), containsString("use a regular logger"));
+    }
+}