Skip to content

Commit

Permalink
Logger: Merge ESLoggerFactory into Loggers
Browse files Browse the repository at this point in the history
`ESLoggerFactory` is now not particularly interesting and simple enought
to fold entirely into `Loggers. So let's do that.

Closes #32174
  • Loading branch information
nik9000 committed Oct 31, 2018
1 parent ca620ff commit e3763b4
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -70,26 +71,25 @@ 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);
assert prefix != null && false == prefix.isEmpty() : "don't use a prefix logger without a prefix";
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;
}
Expand Down

0 comments on commit e3763b4

Please sign in to comment.