Skip to content

Commit e7ead17

Browse files
authored
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.
1 parent 4c38853 commit e7ead17

File tree

5 files changed

+20
-27
lines changed

5 files changed

+20
-27
lines changed

server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.logging.log4j.Logger;
2424
import org.elasticsearch.common.Strings;
2525
import org.elasticsearch.common.logging.DeprecationLogger;
26-
import org.elasticsearch.common.logging.Loggers;
2726
import org.elasticsearch.common.settings.Settings;
2827
import org.elasticsearch.node.Node;
2928

@@ -34,13 +33,7 @@ public abstract class AbstractComponent {
3433
protected final Settings settings;
3534

3635
public AbstractComponent(Settings settings) {
37-
this.logger = Loggers.getLogger(getClass(), settings);
38-
this.deprecationLogger = new DeprecationLogger(logger);
39-
this.settings = settings;
40-
}
41-
42-
public AbstractComponent(Settings settings, Class<?> customClass) {
43-
this.logger = LogManager.getLogger(customClass);
36+
this.logger = LogManager.getLogger(getClass());
4437
this.deprecationLogger = new DeprecationLogger(logger);
4538
this.settings = settings;
4639
}

server/src/main/java/org/elasticsearch/common/component/AbstractLifecycleComponent.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ protected AbstractLifecycleComponent(Settings settings) {
3535
super(settings);
3636
}
3737

38-
protected AbstractLifecycleComponent(Settings settings, Class<?> customClass) {
39-
super(settings, customClass);
40-
}
41-
4238
@Override
4339
public Lifecycle.State lifecycleState() {
4440
return this.lifecycle.state();

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,13 @@ public static Logger getLogger(String prefix, String name) {
3838

3939
public static Logger getLogger(String prefix, Class<?> clazz) {
4040
/*
41-
* Do not use LogManager#getLogger(Class) as this now uses Class#getCanonicalName under the hood; as this returns null for local and
42-
* anonymous classes, any place we create, for example, an abstract component defined as an anonymous class (e.g., in tests) will
43-
* result in a logger with a null name which will blow up in a lookup inside of Log4j.
41+
* At one point we didn't use LogManager.getLogger(clazz) because
42+
* of a bug in log4j that has since been fixed:
43+
* https://github.com/apache/logging-log4j2/commit/ae33698a1846a5e10684ec3e52a99223f06047af
44+
*
45+
* For now we continue to use LogManager.getLogger(clazz.getName())
46+
* because we expect to eventually migrate away from needing this
47+
* method entirely.
4448
*/
4549
return getLogger(prefix, LogManager.getLogger(clazz.getName()));
4650
}

server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,13 @@ public void testClusterStateUpdateLogging() throws Exception {
118118
mockAppender.addExpectation(
119119
new MockLogAppender.SeenEventExpectation(
120120
"test1",
121-
clusterApplierService.getClass().getName(),
121+
clusterApplierService.getClass().getCanonicalName(),
122122
Level.DEBUG,
123123
"*processing [test1]: took [1s] no change in cluster state"));
124124
mockAppender.addExpectation(
125125
new MockLogAppender.SeenEventExpectation(
126126
"test2",
127-
clusterApplierService.getClass().getName(),
127+
clusterApplierService.getClass().getCanonicalName(),
128128
Level.TRACE,
129129
"*failed to execute cluster state applier in [2s]*"));
130130

@@ -192,19 +192,19 @@ public void testLongClusterStateUpdateLogging() throws Exception {
192192
mockAppender.addExpectation(
193193
new MockLogAppender.UnseenEventExpectation(
194194
"test1 shouldn't see because setting is too low",
195-
clusterApplierService.getClass().getName(),
195+
clusterApplierService.getClass().getCanonicalName(),
196196
Level.WARN,
197197
"*cluster state applier task [test1] took [*] above the warn threshold of *"));
198198
mockAppender.addExpectation(
199199
new MockLogAppender.SeenEventExpectation(
200200
"test2",
201-
clusterApplierService.getClass().getName(),
201+
clusterApplierService.getClass().getCanonicalName(),
202202
Level.WARN,
203203
"*cluster state applier task [test2] took [32s] above the warn threshold of *"));
204204
mockAppender.addExpectation(
205205
new MockLogAppender.SeenEventExpectation(
206206
"test4",
207-
clusterApplierService.getClass().getName(),
207+
clusterApplierService.getClass().getCanonicalName(),
208208
Level.WARN,
209209
"*cluster state applier task [test3] took [34s] above the warn threshold of *"));
210210

server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -309,19 +309,19 @@ public void testClusterStateUpdateLogging() throws Exception {
309309
mockAppender.addExpectation(
310310
new MockLogAppender.SeenEventExpectation(
311311
"test1",
312-
masterService.getClass().getName(),
312+
masterService.getClass().getCanonicalName(),
313313
Level.DEBUG,
314314
"*processing [test1]: took [1s] no change in cluster state"));
315315
mockAppender.addExpectation(
316316
new MockLogAppender.SeenEventExpectation(
317317
"test2",
318-
masterService.getClass().getName(),
318+
masterService.getClass().getCanonicalName(),
319319
Level.TRACE,
320320
"*failed to execute cluster state update in [2s]*"));
321321
mockAppender.addExpectation(
322322
new MockLogAppender.SeenEventExpectation(
323323
"test3",
324-
masterService.getClass().getName(),
324+
masterService.getClass().getCanonicalName(),
325325
Level.DEBUG,
326326
"*processing [test3]: took [3s] done publishing updated cluster state (version: *, uuid: *)"));
327327

@@ -650,25 +650,25 @@ public void testLongClusterStateUpdateLogging() throws Exception {
650650
mockAppender.addExpectation(
651651
new MockLogAppender.UnseenEventExpectation(
652652
"test1 shouldn't see because setting is too low",
653-
masterService.getClass().getName(),
653+
masterService.getClass().getCanonicalName(),
654654
Level.WARN,
655655
"*cluster state update task [test1] took [*] above the warn threshold of *"));
656656
mockAppender.addExpectation(
657657
new MockLogAppender.SeenEventExpectation(
658658
"test2",
659-
masterService.getClass().getName(),
659+
masterService.getClass().getCanonicalName(),
660660
Level.WARN,
661661
"*cluster state update task [test2] took [32s] above the warn threshold of *"));
662662
mockAppender.addExpectation(
663663
new MockLogAppender.SeenEventExpectation(
664664
"test3",
665-
masterService.getClass().getName(),
665+
masterService.getClass().getCanonicalName(),
666666
Level.WARN,
667667
"*cluster state update task [test3] took [33s] above the warn threshold of *"));
668668
mockAppender.addExpectation(
669669
new MockLogAppender.SeenEventExpectation(
670670
"test4",
671-
masterService.getClass().getName(),
671+
masterService.getClass().getCanonicalName(),
672672
Level.WARN,
673673
"*cluster state update task [test4] took [34s] above the warn threshold of *"));
674674

0 commit comments

Comments
 (0)