From e7ead178931091c00059c86019d68f164daf1783 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 1 Aug 2018 09:17:48 -0400 Subject: [PATCH] Core: Minor size reduction for AbstractComponent (#32509) This removes a constructor from `AbstractComponent` and `AbstractLifecycleComponent` that we weren't using and it switches the logger creation away from one of the `Settings` flavored methods which are no longer needed. --- .../common/component/AbstractComponent.java | 9 +-------- .../component/AbstractLifecycleComponent.java | 4 ---- .../common/logging/ESLoggerFactory.java | 10 +++++++--- .../service/ClusterApplierServiceTests.java | 10 +++++----- .../cluster/service/MasterServiceTests.java | 14 +++++++------- 5 files changed, 20 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java b/server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java index 62d6e7e311d..167ffdb7bea 100644 --- a/server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java +++ b/server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java @@ -23,7 +23,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.node.Node; @@ -34,13 +33,7 @@ public abstract class AbstractComponent { protected final Settings settings; public AbstractComponent(Settings settings) { - this.logger = Loggers.getLogger(getClass(), settings); - this.deprecationLogger = new DeprecationLogger(logger); - this.settings = settings; - } - - public AbstractComponent(Settings settings, Class customClass) { - this.logger = LogManager.getLogger(customClass); + this.logger = LogManager.getLogger(getClass()); this.deprecationLogger = new DeprecationLogger(logger); this.settings = settings; } diff --git a/server/src/main/java/org/elasticsearch/common/component/AbstractLifecycleComponent.java b/server/src/main/java/org/elasticsearch/common/component/AbstractLifecycleComponent.java index 3c4b35d5c34..8a472954ab4 100644 --- a/server/src/main/java/org/elasticsearch/common/component/AbstractLifecycleComponent.java +++ b/server/src/main/java/org/elasticsearch/common/component/AbstractLifecycleComponent.java @@ -35,10 +35,6 @@ public abstract class AbstractLifecycleComponent extends AbstractComponent imple super(settings); } - protected AbstractLifecycleComponent(Settings settings, Class customClass) { - super(settings, customClass); - } - @Override public Lifecycle.State lifecycleState() { return this.lifecycle.state(); diff --git a/server/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java b/server/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java index caa922b92cf..b45b55609f5 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java +++ b/server/src/main/java/org/elasticsearch/common/logging/ESLoggerFactory.java @@ -38,9 +38,13 @@ public final class ESLoggerFactory { public static Logger getLogger(String prefix, Class clazz) { /* - * Do not use LogManager#getLogger(Class) as this now uses Class#getCanonicalName under the hood; as this returns null for local and - * anonymous classes, any place we create, for example, an abstract component defined as an anonymous class (e.g., in tests) will - * result in a logger with a null name which will blow up in a lookup inside of Log4j. + * At one point we didn't use LogManager.getLogger(clazz) because + * of a bug in log4j that has since been fixed: + * https://github.com/apache/logging-log4j2/commit/ae33698a1846a5e10684ec3e52a99223f06047af + * + * For now we continue to use LogManager.getLogger(clazz.getName()) + * because we expect to eventually migrate away from needing this + * method entirely. */ return getLogger(prefix, LogManager.getLogger(clazz.getName())); } diff --git a/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java index f4c2090895b..cbf8a7eda2b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java @@ -118,13 +118,13 @@ public class ClusterApplierServiceTests extends ESTestCase { mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test1", - clusterApplierService.getClass().getName(), + clusterApplierService.getClass().getCanonicalName(), Level.DEBUG, "*processing [test1]: took [1s] no change in cluster state")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", - clusterApplierService.getClass().getName(), + clusterApplierService.getClass().getCanonicalName(), Level.TRACE, "*failed to execute cluster state applier in [2s]*")); @@ -192,19 +192,19 @@ public class ClusterApplierServiceTests extends ESTestCase { mockAppender.addExpectation( new MockLogAppender.UnseenEventExpectation( "test1 shouldn't see because setting is too low", - clusterApplierService.getClass().getName(), + clusterApplierService.getClass().getCanonicalName(), Level.WARN, "*cluster state applier task [test1] took [*] above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", - clusterApplierService.getClass().getName(), + clusterApplierService.getClass().getCanonicalName(), Level.WARN, "*cluster state applier task [test2] took [32s] above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test4", - clusterApplierService.getClass().getName(), + clusterApplierService.getClass().getCanonicalName(), Level.WARN, "*cluster state applier task [test3] took [34s] above the warn threshold of *")); diff --git a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java index 1ef548bd681..1168e1034fe 100644 --- a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java @@ -309,19 +309,19 @@ public class MasterServiceTests extends ESTestCase { mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test1", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.DEBUG, "*processing [test1]: took [1s] no change in cluster state")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.TRACE, "*failed to execute cluster state update in [2s]*")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test3", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.DEBUG, "*processing [test3]: took [3s] done publishing updated cluster state (version: *, uuid: *)")); @@ -650,25 +650,25 @@ public class MasterServiceTests extends ESTestCase { mockAppender.addExpectation( new MockLogAppender.UnseenEventExpectation( "test1 shouldn't see because setting is too low", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.WARN, "*cluster state update task [test1] took [*] above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.WARN, "*cluster state update task [test2] took [32s] above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test3", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.WARN, "*cluster state update task [test3] took [33s] above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test4", - masterService.getClass().getName(), + masterService.getClass().getCanonicalName(), Level.WARN, "*cluster state update task [test4] took [34s] above the warn threshold of *"));