sarutak commented on a change in pull request #35080:
URL: https://github.com/apache/spark/pull/35080#discussion_r777138948
##########
File path: core/src/main/scala/org/apache/spark/internal/Logging.scala
##########
@@ -259,18 +259,25 @@ private[spark] object Logging {
} else if
(logEvent.getLevel.isMoreSpecificThan(Logging.sparkShellThresholdLevel)) {
Filter.Result.NEUTRAL
} else {
- var logger = LogManager.getLogger(logEvent.getLoggerName)
- .asInstanceOf[Log4jLogger]
- while (logger.getParent() != null) {
- if (logger.getLevel != null || !logger.getAppenders.isEmpty) {
+ val logger =
LogManager.getLogger(logEvent.getLoggerName).asInstanceOf[Log4jLogger]
+ if (loggerWithCustomConfig(logger)) {
return Filter.Result.NEUTRAL
- }
- logger = logger.getParent()
}
Filter.Result.DENY
}
}
+ // Return true if the logger has custom configuration. It depends on:
+ // 1. If the logger isn't attached with root logger config (i.e., with
custom configuration), or
+ // 2. the logger level is different to root config level (i.e., it is
changed programmingly).
+ //
+ // Note that if a logger is programmingly changed log level but set to
same level as root config
Review comment:
In this case, the instance of `LoggerConfig` is different between the
custom logger and root logger?
Then, can we tell them each other?
##########
File path: core/src/main/scala/org/apache/spark/internal/Logging.scala
##########
@@ -259,18 +259,25 @@ private[spark] object Logging {
} else if
(logEvent.getLevel.isMoreSpecificThan(Logging.sparkShellThresholdLevel)) {
Filter.Result.NEUTRAL
} else {
- var logger = LogManager.getLogger(logEvent.getLoggerName)
- .asInstanceOf[Log4jLogger]
- while (logger.getParent() != null) {
- if (logger.getLevel != null || !logger.getAppenders.isEmpty) {
+ val logger =
LogManager.getLogger(logEvent.getLoggerName).asInstanceOf[Log4jLogger]
+ if (loggerWithCustomConfig(logger)) {
return Filter.Result.NEUTRAL
- }
- logger = logger.getParent()
}
Filter.Result.DENY
}
}
+ // Return true if the logger has custom configuration. It depends on:
+ // 1. If the logger isn't attached with root logger config (i.e., with
custom configuration), or
+ // 2. the logger level is different to root config level (i.e., it is
changed programmingly).
Review comment:
`programmingly` -> `programmatically`?
##########
File path: core/src/main/scala/org/apache/spark/internal/Logging.scala
##########
@@ -259,18 +259,25 @@ private[spark] object Logging {
} else if
(logEvent.getLevel.isMoreSpecificThan(Logging.sparkShellThresholdLevel)) {
Filter.Result.NEUTRAL
} else {
- var logger = LogManager.getLogger(logEvent.getLoggerName)
- .asInstanceOf[Log4jLogger]
- while (logger.getParent() != null) {
- if (logger.getLevel != null || !logger.getAppenders.isEmpty) {
+ val logger =
LogManager.getLogger(logEvent.getLoggerName).asInstanceOf[Log4jLogger]
+ if (loggerWithCustomConfig(logger)) {
return Filter.Result.NEUTRAL
- }
- logger = logger.getParent()
}
Filter.Result.DENY
}
}
+ // Return true if the logger has custom configuration. It depends on:
+ // 1. If the logger isn't attached with root logger config (i.e., with
custom configuration), or
+ // 2. the logger level is different to root config level (i.e., it is
changed programmingly).
+ //
+ // Note that if a logger is programmingly changed log level but set to
same level as root config
+ // level, we cannot tell if it is with custom configuration.
+ private def loggerWithCustomConfig(logger: Log4jLogger): Boolean = {
+ val rootConfig = LogManager.getRootLogger.asInstanceOf[Log4jLogger].get()
+ logger.get() != rootConfig || logger.getLevel != rootConfig.getLevel()
Review comment:
Is it possible that `logger.get() != rootConfig` is `false` but
`logger.getLevel != rootConfig.getLevel()` is `true`?
Can we check with `logger.get ne rootConfig` instead ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
