mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-03-24 17:09:48 +00:00
Logger: Merge ESLoggerFactory into Loggers (#35146)
`ESLoggerFactory` is now not particularly interesting and simple enought to fold entirely into `Loggers. So let's do that. Closes #32174
This commit is contained in:
parent
3ee37b425b
commit
348c28d1d1
@ -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
|
||||
|
@ -27,7 +27,6 @@ import org.apache.logging.log4j.core.LoggerContext;
|
||||
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 class EvilLoggerTests extends ESTestCase {
|
||||
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 class EvilLoggerTests extends ESTestCase {
|
||||
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
|
||||
|
@ -1,64 +0,0 @@
|
||||
/*
|
||||
* 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.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.logging.log4j.spi.ExtendedLogger;
|
||||
|
||||
/**
|
||||
* Factory to get {@link Logger}s
|
||||
*/
|
||||
final class ESLoggerFactory {
|
||||
|
||||
private ESLoggerFactory() {
|
||||
|
||||
}
|
||||
|
||||
static Logger getLogger(String prefix, String name) {
|
||||
return getLogger(prefix, LogManager.getLogger(name));
|
||||
}
|
||||
|
||||
static Logger getLogger(String prefix, Class<?> clazz) {
|
||||
/*
|
||||
* 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()));
|
||||
}
|
||||
|
||||
static Logger getLogger(String prefix, Logger logger) {
|
||||
/*
|
||||
* In a followup we'll throw an exception if prefix is null or empty
|
||||
* redirecting folks to LogManager.getLogger.
|
||||
*
|
||||
* This and more is tracked in https://github.com/elastic/elasticsearch/issues/32174
|
||||
*/
|
||||
if (prefix == null || prefix.length() == 0) {
|
||||
return logger;
|
||||
}
|
||||
return new PrefixLogger((ExtendedLogger)logger, logger.getName(), prefix);
|
||||
}
|
||||
}
|
@ -57,7 +57,8 @@ public class Loggers {
|
||||
* 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) {
|
||||
@ -65,15 +66,15 @@ public class Loggers {
|
||||
}
|
||||
|
||||
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) {
|
||||
|
@ -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 @@ class PrefixLogger extends ExtendedLoggerWrapper {
|
||||
* 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;
|
||||
}
|
||||
|
@ -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"));
|
||||
}
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user