Skip to content

Commit

Permalink
KAFKA-18243 Fix compatibility of Loggers class between log4j and log4…
Browse files Browse the repository at this point in the history
…j2 (#18185)

Reviewers: Chia-Ping Tsai <[email protected]>
  • Loading branch information
frankvicky authored Dec 29, 2024
1 parent 3654bc4 commit 96527be
Show file tree
Hide file tree
Showing 6 changed files with 459 additions and 254 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public List<String> getMessages(String level) {
}

public List<String> getMessages() {
final LinkedList<String> result = new LinkedList<>();
final List<String> result = new LinkedList<>();
synchronized (events) {
for (final LogEvent event : events) {
result.add(event.getMessage().getFormattedMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.config.Configurator;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -48,16 +47,24 @@ public class Loggers {

private static final Logger log = LoggerFactory.getLogger(Loggers.class);

private static final String ROOT_LOGGER_NAME = "root";

/**
* Log4j uses "root" (case-insensitive) as name of the root logger.
* Note: In log4j, the root logger's name was "root" and Kafka also followed that name for dynamic logging control feature.
*
* The root logger's name is changed in log4j2 to empty string (see: [[LogManager.ROOT_LOGGER_NAME]]) but for backward-
* compatibility. Kafka keeps its original root logger name. It is why here is a dedicated definition for the root logger name.
* While log4j2 changed the root logger's name to empty string (see: [[LogManager.ROOT_LOGGER_NAME]]),
* for backward-compatibility purposes, we accept both empty string and "root" as valid root logger names.
* This is why we have a dedicated definition that includes both values.
*/
private static final String ROOT_LOGGER_NAME = "root";
private static final List<String> VALID_ROOT_LOGGER_NAMES = List.of(LogManager.ROOT_LOGGER_NAME, ROOT_LOGGER_NAME);

private final Time time;

/**
* Maps logger names to their last modification timestamps.
* Note: The logger name "root" refers to the actual root logger of log4j2.
*/
private final Map<String, Long> lastModifiedTimes;

public Loggers(Time time) {
Expand All @@ -75,10 +82,10 @@ public synchronized LoggerLevel level(String logger) {
Objects.requireNonNull(logger, "Logger may not be null");

org.apache.logging.log4j.Logger foundLogger = null;
if (ROOT_LOGGER_NAME.equalsIgnoreCase(logger)) {
if (isValidRootLoggerName(logger)) {
foundLogger = rootLogger();
} else {
List<org.apache.logging.log4j.Logger> currentLoggers = currentLoggers();
var currentLoggers = currentLoggers().values();
// search within existing loggers for the given name.
// using LogManger.getLogger() will create a logger if it doesn't exist
// (potential leak since these don't get cleaned up).
Expand All @@ -103,18 +110,16 @@ public synchronized LoggerLevel level(String logger) {
* @return the levels of all known loggers; may be empty, but never null
*/
public synchronized Map<String, LoggerLevel> allLevels() {
Map<String, LoggerLevel> result = new TreeMap<>();

currentLoggers().stream()
.filter(logger -> !logger.getLevel().equals(Level.OFF))
.forEach(logger -> result.put(logger.getName(), loggerLevel(logger)));

org.apache.logging.log4j.Logger root = rootLogger();
if (!root.getLevel().equals(Level.OFF)) {
result.put(ROOT_LOGGER_NAME, loggerLevel(root));
}

return result;
return currentLoggers()
.values()
.stream()
.filter(logger -> !logger.getLevel().equals(Level.OFF))
.collect(Collectors.toMap(
this::getLoggerName,
this::loggerLevel,
(existing, replacing) -> replacing,
TreeMap::new)
);
}

/**
Expand All @@ -127,14 +132,25 @@ public synchronized Map<String, LoggerLevel> allLevels() {
public synchronized List<String> setLevel(String namespace, Level level) {
Objects.requireNonNull(namespace, "Logging namespace may not be null");
Objects.requireNonNull(level, "Level may not be null");
String internalNameSpace = isValidRootLoggerName(namespace) ? LogManager.ROOT_LOGGER_NAME : namespace;

log.info("Setting level of namespace {} and children to {}", internalNameSpace, level);

log.info("Setting level of namespace {} and children to {}", namespace, level);
List<org.apache.logging.log4j.Logger> childLoggers = loggers(namespace);
var loggers = loggers(internalNameSpace);
var nameToLevel = allLevels();

List<String> result = new ArrayList<>();
for (org.apache.logging.log4j.Logger logger: childLoggers) {
setLevel(logger, level);
result.add(logger.getName());
Configurator.setAllLevels(internalNameSpace, level);
for (org.apache.logging.log4j.Logger logger : loggers) {
// We need to track level changes for each logger and record their update timestamps to ensure this method
// correctly returns only the loggers whose levels were actually modified.
String name = getLoggerName(logger);
String newLevel = logger.getLevel().name();
String oldLevel = nameToLevel.getOrDefault(name, new LoggerLevel("", time.milliseconds())).level();
if (!newLevel.equalsIgnoreCase(oldLevel)) {
lastModifiedTimes.put(name, time.milliseconds());
result.add(name);
}
}
Collections.sort(result);

Expand All @@ -148,18 +164,18 @@ public synchronized List<String> setLevel(String namespace, Level level) {
* @return all loggers that fall under the given namespace; never null, and will always contain
* at least one logger (the ancestor logger for the namespace)
*/
private synchronized List<org.apache.logging.log4j.Logger> loggers(String namespace) {
private synchronized Collection<org.apache.logging.log4j.Logger> loggers(String namespace) {
Objects.requireNonNull(namespace, "Logging namespace may not be null");

if (ROOT_LOGGER_NAME.equalsIgnoreCase(namespace)) {
List<org.apache.logging.log4j.Logger> result = currentLoggers();
result.add(rootLogger());
return result;
if (isValidRootLoggerName(namespace)) {
return currentLoggers().values();
}

List<org.apache.logging.log4j.Logger> result = new ArrayList<>();
org.apache.logging.log4j.Logger ancestorLogger = lookupLogger(namespace);
List<org.apache.logging.log4j.Logger> currentLoggers = currentLoggers();
var result = new ArrayList<org.apache.logging.log4j.Logger>();
var nameToLogger = currentLoggers();
var ancestorLogger = lookupLogger(namespace);
var currentLoggers = nameToLogger.values();

boolean present = false;
for (org.apache.logging.log4j.Logger currentLogger : currentLoggers) {
if (currentLogger.getName().startsWith(namespace)) {
Expand All @@ -179,45 +195,41 @@ private synchronized List<org.apache.logging.log4j.Logger> loggers(String namesp

// visible for testing
org.apache.logging.log4j.Logger lookupLogger(String logger) {
return LogManager.getLogger(logger);
return LogManager.getLogger(isValidRootLoggerName(logger) ? LogManager.ROOT_LOGGER_NAME : logger);
}

List<org.apache.logging.log4j.Logger> currentLoggers() {
Map<String, org.apache.logging.log4j.Logger> currentLoggers() {
LoggerContext context = (LoggerContext) LogManager.getContext(false);
Collection<LoggerConfig> loggerConfigs = context.getConfiguration().getLoggers().values();
return loggerConfigs.stream()
.map(LoggerConfig::getName)
.distinct()
.map(LogManager::getLogger)
.collect(Collectors.toCollection(ArrayList::new));
var results = new HashMap<String, org.apache.logging.log4j.Logger>();
context.getConfiguration().getLoggers().forEach((name, logger) -> results.put(name, LogManager.getLogger(name)));
context.getLoggerRegistry().getLoggers().forEach(logger -> results.put(logger.getName(), logger));
return results;
}

// visible for testing
org.apache.logging.log4j.Logger rootLogger() {
return LogManager.getRootLogger();
}

private void setLevel(org.apache.logging.log4j.Logger logger, Level level) {
String loggerName = logger.getName();
LoggerContext context = (LoggerContext) LogManager.getContext(false);
LoggerConfig loggerConfig = context.getConfiguration().getLoggerConfig(loggerName);
Level currentLevel = loggerConfig.getLevel();

if (level.equals(currentLevel)) {
log.debug("Skipping update for logger {} since its level is already {}", loggerName, level);
return;
}
private LoggerLevel loggerLevel(org.apache.logging.log4j.Logger logger) {
Long lastModified = lastModifiedTimes.get(getLoggerName(logger));
return new LoggerLevel(Objects.toString(logger.getLevel()), lastModified);
}

log.debug("Setting level of logger {} (excluding children) to {}", loggerName, level);
Configurator.setLevel(loggerName, level);
lastModifiedTimes.put(loggerName, time.milliseconds());
private boolean isValidRootLoggerName(String namespace) {
return VALID_ROOT_LOGGER_NAMES.stream()
.anyMatch(rootLoggerNames -> rootLoggerNames.equalsIgnoreCase(namespace));
}

private LoggerLevel loggerLevel(org.apache.logging.log4j.Logger logger) {
LoggerContext context = (LoggerContext) LogManager.getContext(false);
LoggerConfig loggerConfig = context.getConfiguration().getLoggerConfig(logger.getName());
Level level = loggerConfig.getLevel();
Long lastModified = lastModifiedTimes.get(logger.getName());
return new LoggerLevel(Objects.toString(level), lastModified);
/**
* Converts logger name to ensure backward compatibility between log4j and log4j2.
* If the logger name is empty (log4j2's root logger representation), converts it to "root" (log4j's style).
* Otherwise, returns the original logger name.
*
* @param logger The logger instance to get the name from
* @return The logger name - returns "root" for empty string, otherwise returns the original logger name
*/
private String getLoggerName(org.apache.logging.log4j.Logger logger) {
return logger.getName().equals(LogManager.ROOT_LOGGER_NAME) ? ROOT_LOGGER_NAME : logger.getName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ private Map<String, LoggerLevel> testSetLoggingLevel(
newLevels,
e -> hasNamespace(e, namespace)
&& (!level(e).equals(level)
|| !isModified(e)
|| lastModified(e) < requestTime
|| (isModified(e) && lastModified(e) < requestTime)
)
);
assertEquals(
Expand Down
Loading

0 comments on commit 96527be

Please sign in to comment.