Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Minor size reduction for AbstractComponent #32509

Merged
merged 6 commits into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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;

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ protected AbstractLifecycleComponent(Settings settings) {
super(settings);
}

protected AbstractLifecycleComponent(Settings settings, Class<?> customClass) {
super(settings, customClass);
}

@Override
public Lifecycle.State lifecycleState() {
return this.lifecycle.state();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ public static Logger getLogger(String prefix, String name) {

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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@ public void testClusterStateUpdateLogging() throws Exception {
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]*"));

Expand Down Expand Up @@ -192,19 +192,19 @@ public void testLongClusterStateUpdateLogging() throws Exception {
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 *"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,19 +309,19 @@ public void testClusterStateUpdateLogging() throws Exception {
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: *)"));

Expand Down Expand Up @@ -650,25 +650,25 @@ public void testLongClusterStateUpdateLogging() throws Exception {
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 *"));

Expand Down