ppkarwasz commented on code in PR #18496:
URL: https://github.com/apache/kafka/pull/18496#discussion_r1931146785
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Loggers.java:
##########
@@ -52,184 +53,274 @@ public class Loggers {
/**
* 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.
- *
+ * <p>
* 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.
+ * </p>
*/
private static final List<String> VALID_ROOT_LOGGER_NAMES =
List.of(LogManager.ROOT_LOGGER_NAME, ROOT_LOGGER_NAME);
- private final Time time;
+ 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;
+ final Map<String, Long> lastModifiedTimes;
- public Loggers(Time time) {
+ /**
+ * Creates a {@link Loggers} instance appropriate for the current
environment.
+ *
+ * @param time A time source.
+ * @return A new {@link Loggers} instance, never {@link null}.
+ */
+ public static Loggers newInstance(Time time) {
+ Objects.requireNonNull(time);
+ try {
+ return new Log4jLoggers(time);
+ } catch (ClassCastException | LinkageError e) {
+ log.info("No supported logging implementation found. Logging
configuration endpoint will be disabled.");
+ return new NoOpLoggers(time);
+ }
+ }
+
+ private Loggers(Time time) {
this.time = time;
- this.lastModifiedTimes = new HashMap<>();
+ this.lastModifiedTimes = new ConcurrentHashMap<>();
}
/**
* Retrieve the current level for a single logger.
- * @param logger the name of the logger to retrieve the level for; may not
be null
+ *
+ * @param loggerName the name of the logger to retrieve the level for; may
not be null
* @return the current level (falling back on the effective level if
necessary) of the logger,
* or null if no logger with the specified name exists
*/
- public synchronized LoggerLevel level(String logger) {
- Objects.requireNonNull(logger, "Logger may not be null");
-
- org.apache.logging.log4j.Logger foundLogger = null;
- if (isValidRootLoggerName(logger)) {
- foundLogger = rootLogger();
- } else {
- 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).
- for (org.apache.logging.log4j.Logger currentLogger :
currentLoggers) {
- if (logger.equals(currentLogger.getName())) {
- foundLogger = currentLogger;
- break;
- }
- }
- }
-
- if (foundLogger == null) {
- log.warn("Unable to find level for logger {}", logger);
- return null;
- }
-
- return loggerLevel(foundLogger);
- }
+ public abstract LoggerLevel level(String loggerName);
/**
* Retrieve the current levels of all known loggers
+ *
* @return the levels of all known loggers; may be empty, but never null
*/
- public synchronized Map<String, LoggerLevel> allLevels() {
- return currentLoggers()
- .values()
- .stream()
- .filter(logger -> !logger.getLevel().equals(Level.OFF))
- .collect(Collectors.toMap(
- this::getLoggerName,
- this::loggerLevel,
- (existing, replacing) -> replacing,
- TreeMap::new)
- );
- }
+ public abstract Map<String, LoggerLevel> allLevels();
/**
* Set the level for the specified logger and all of its children
+ *
* @param namespace the name of the logger to adjust along with its
children; may not be null
- * @param level the level to set for the logger and its children; may not
be null
+ * @param level the level to set for the logger and its children; may
not be null
* @return all loggers that were affected by this action, sorted by their
natural ordering;
* may be empty, but never null
*/
- 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);
-
- var loggers = loggers(internalNameSpace);
- var nameToLevel = allLevels();
-
- List<String> result = new ArrayList<>();
- 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);
+ public abstract List<String> setLevel(String namespace, String level);
+
+ public abstract boolean isValidLevel(String level);
+
+ static class Log4jLoggers extends Loggers {
Review Comment:
In the long term I would use
[apache/logging-admin](https://github.com/apache/logging-admin) (as soon as we
publish it), which abstracts the way loggers are configured and provides a
limited access to the capabilities of the logging implementations.
For now the main goal is not to expose any of these details (classes,
methods, etc.) as public API, so that they could be removed in a minor version
of Kafka 4.x, without breaking changes.
Checkstyle is not bulletproof, but it should be enough for now. Fixed in
https://github.com/apache/kafka/pull/18496/commits/db05e49b49836e9392aaf1f3c0d6a27124ec4445
##########
core/src/main/scala/kafka/utils/LoggingController.scala:
##########
@@ -27,22 +28,55 @@ import java.util.Locale
import scala.jdk.CollectionConverters._
-object Log4jController {
+object LoggingController {
/**
- * Note: In log4j, the root logger's name was "root" and Kafka also followed
that name for dynamic logging control feature.
+ * Note: In Log4j 1, 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.
*/
val ROOT_LOGGER = "root"
+ private[this] val delegate: LoggingControllerDelegate = {
+ try {
+ new Log4jCoreController
+ } catch {
+ case _: ClassCastException | _: LinkageError => new NoOpController
+ }
+ }
+
/**
* Returns a map of the log4j loggers and their assigned log level.
* If a logger does not have a log level assigned, we return the log level
of the first ancestor with a level configured.
*/
- def loggers: Map[String, String] = {
- val logContext = LogManager.getContext(false).asInstanceOf[LoggerContext]
+ def loggers: Map[String, String] = delegate.loggers
+
+ /**
+ * Sets the log level of a particular logger. If the given logLevel is not
an available level
+ * (i.e., one of OFF, FATAL, ERROR, WARN, INFO, DEBUG, TRACE, ALL) it falls
back to DEBUG.
+ *
+ * @see [[Level.toLevel]]
+ */
+ def logLevel(loggerName: String, logLevel: String): Boolean =
delegate.logLevel(loggerName, logLevel)
+
+ def unsetLogLevel(loggerName: String): Boolean =
delegate.unsetLogLevel(loggerName)
+
+ def loggerExists(loggerName: String): Boolean =
delegate.loggerExists(loggerName)
+}
+
+private class NoOpController extends LoggingControllerDelegate {
+ override def loggers: Map[String, String] = Map.empty
+
+ override def logLevel(loggerName: String, logLevel: String): Boolean = false
+
+ override def unsetLogLevel(loggerName: String): Boolean = false
+}
+
+private class Log4jCoreController extends LoggingControllerDelegate {
Review Comment:
Fixed in
https://github.com/apache/kafka/pull/18496/commits/db05e49b49836e9392aaf1f3c0d6a27124ec4445
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
