You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "sharanggupta (via GitHub)" <gi...@apache.org> on 2026/10/22 05:14:36 UTC
[PR] Respect logger-name hierarchy boundary in Configurator.setAllLevels [logging-log4j2]
sharanggupta opened a new pull request, #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419
Fixes #4418.
`Configurator.setAllLevels(parentLogger, level)` selected the child `LoggerConfig`s with a plain `startsWith`, so `setAllLevels("com.foo", TRACE)` also re-leveled `com.foobar` and `com.foobar.child`. This adds the same hierarchy-boundary check that #4414 introduced for `LoggerNameLevelRewritePolicy` (the parent itself, or names continuing with `.`; the root logger `""` and a trailing-dot parent still cover everything below them), a `LoggerTest` case with a sibling logger that keeps its level, and a changelog entry.
`main` (3.x) carries the identical lines in `Configurator.setAllLevels`; happy to forward-port once this is merged.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "sharanggupta (via GitHub)" <gi...@apache.org>.
sharanggupta commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6529560179
The build jobs on 979372c were cancelled at 16:56 UTC. Could you re-run them?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "sharanggupta (via GitHub)" <gi...@apache.org>.
sharanggupta commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6396909462
As asked on #3155, this PR now also carries the `AbstractConfiguration` fix for that issue as a separate second commit (941a6a78ab, previously #4423, now closed). Description updated to cover both issues; the two commits stay independent.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect logger-name hierarchy boundary in Configurator.setAllLevels [logging-log4j2]
Posted by "sharanggupta (via GitHub)" <gi...@apache.org>.
sharanggupta commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4361122641
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerTest.java:
##########
@@ -215,6 +215,18 @@ void debugChangeLevelAllChildrenLoggers() {
assertEventCount(app.getEvents(), 5);
}
+ @Test
+ void setAllLevelsDoesNotChangeSiblingLoggers() {
+ // A logger whose name merely starts with the parent's name is a sibling, not a child, in the
+ // dot-separated hierarchy and must keep its own level
+ final String sibling = logger.getName() + "Sibling";
+ Configurator.setLevel(sibling, Level.INFO);
Review Comment:
Done in 05f9b0fb58: the child now gets its own level before the call, so the assertion checks the child's own `LoggerConfig` rather than inheritance. With the `charAt(...) == '.'` check removed this test now fails (verified).
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerTest.java:
##########
@@ -215,6 +215,18 @@ void debugChangeLevelAllChildrenLoggers() {
assertEventCount(app.getEvents(), 5);
}
+ @Test
+ void setAllLevelsDoesNotChangeSiblingLoggers() {
+ // A logger whose name merely starts with the parent's name is a sibling, not a child, in the
+ // dot-separated hierarchy and must keep its own level
+ final String sibling = logger.getName() + "Sibling";
+ Configurator.setLevel(sibling, Level.INFO);
+ Configurator.setAllLevels(logger.getName(), Level.OFF);
+ assertEquals(Level.OFF, logger.getLevel());
+ assertEquals(Level.OFF, loggerChild.getLevel());
+ assertEquals(Level.INFO, LogManager.getLogger(sibling).getLevel(), "sibling logger must keep its level");
+ }
Review Comment:
Added as suggested in 05f9b0fb58. Removing `isEmpty()` or `endsWith(".")` from `isChildLogger` fails it (`expected: <ERROR> but was: <WARN>` for the root case); verified both.
##########
src/changelog/.2.x.x/4418_Configurator-setAllLevels-boundary.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="4418" link="https://github.com/apache/logging-log4j2/issues/4418"/>
+ <description format="asciidoc">
+ Fix `Configurator.setAllLevels()` also changing the level of sibling loggers that only share a character prefix with the given parent logger name, instead of only its descendants.
Review Comment:
Shortened to the suggested line in 05f9b0fb58.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6520878976
Each `existing` branch now has its own test, and the #3155 changelog line is short.
Can both changelog entries also link this PR after the issue line, as in the inline suggestions?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "sharanggupta (via GitHub)" <gi...@apache.org>.
sharanggupta commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6531095939
Please ignore the re-run request: the cancelled builds look like a regression from this PR. A fix is coming.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4422823961
##########
src/changelog/.2.x.x/4418_Configurator-setAllLevels-boundary.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="4418" link="https://github.com/apache/logging-log4j2/issues/4418"/>
Review Comment:
Can the entry also link this PR, after the issue line?
```suggestion
<issue id="4418" link="https://github.com/apache/logging-log4j2/issues/4418"/>
<issue id="4419" link="https://github.com/apache/logging-log4j2/pull/4419"/>
```
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect logger-name hierarchy boundary in Configurator.setAllLevels [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4358372811
##########
src/changelog/.2.x.x/4418_Configurator-setAllLevels-boundary.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="4418" link="https://github.com/apache/logging-log4j2/issues/4418"/>
+ <description format="asciidoc">
+ Fix `Configurator.setAllLevels()` also changing the level of sibling loggers that only share a character prefix with the given parent logger name, instead of only its descendants.
Review Comment:
Can the entry be one short line with what the user sees?
```suggestion
Fix `Configurator.setAllLevels()` changing sibling loggers.
```
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4409326636
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java:
##########
@@ -959,17 +959,30 @@ public synchronized void addLoggerAppender(
return;
}
final String loggerName = logger.getName();
- appenders.putIfAbsent(appender.getName(), appender);
+ final Appender registered = appenders.putIfAbsent(appender.getName(), appender);
+ // The configuration owns a single appender per name: when another instance is already registered under
+ // this name, attach that one instead of an instance the configuration would never start or stop.
+ final Appender effective = registered == null ? appender : registered;
+ if (effective != appender) {
+ LOGGER.warn(
+ "An appender named '{}' is already registered; attaching the registered instance to logger '{}' instead of the given one",
+ appender.getName(),
+ loggerName);
+ }
final LoggerConfig lc = getLoggerConfig(loggerName);
if (lc.getName().equals(loggerName)) {
- lc.addAppender(appender, null, null);
+ lc.addAppender(effective, null, null);
} else {
final LoggerConfig nlc = new LoggerConfig(loggerName, lc.getLevel(), lc.isAdditive());
- nlc.addAppender(appender, null, null);
+ nlc.addAppender(effective, null, null);
nlc.setParent(lc);
- loggerConfigs.putIfAbsent(loggerName, nlc);
- setParents();
- logger.getContext().updateLoggers();
+ final LoggerConfig existing = loggerConfigs.putIfAbsent(loggerName, nlc);
+ if (existing == null) {
+ setParents();
+ logger.getContext().updateLoggers();
+ } else {
+ existing.addAppender(effective, null, null);
Review Comment:
This branch and the `existing.addFilter(filter)` and `existing.setAdditive(additive)` branches below have no test. Removing any of them keeps every test green. A `LoggerConfig` added with `addLogger` under a key that differs from its name reaches them, for example:
```java
final LoggerConfig other = new LoggerConfig("other", Level.INFO, true);
config.addLogger("mismatch", other);
config.addLoggerFilter(context.getLogger("mismatch"), filter);
assertThat(other.hasFilter()).isTrue();
```
##########
src/changelog/.2.x.x/3155_AbstractConfiguration_logger_mutators.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="3155" link="https://github.com/apache/logging-log4j2/issues/3155"/>
+ <description format="asciidoc">
+ Fix `addLoggerAppender()` attaching an unregistered appender when its name is already registered, and `addLoggerFilter()`/`setLoggerAdditive()` failing on `null` arguments.
Review Comment:
Can the entry be one short line with what the user sees?
```suggestion
Fix `Logger.addAppender()` attaching an unregistered appender.
```
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect logger-name hierarchy boundary in Configurator.setAllLevels [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4358370443
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerTest.java:
##########
@@ -215,6 +215,18 @@ void debugChangeLevelAllChildrenLoggers() {
assertEventCount(app.getEvents(), 5);
}
+ @Test
+ void setAllLevelsDoesNotChangeSiblingLoggers() {
+ // A logger whose name merely starts with the parent's name is a sibling, not a child, in the
+ // dot-separated hierarchy and must keep its own level
+ final String sibling = logger.getName() + "Sibling";
+ Configurator.setLevel(sibling, Level.INFO);
Review Comment:
Give the child its own `LoggerConfig`. Without it the child inherits `OFF` from the parent, and removing the `charAt(...) == '.'` check keeps the test green.
```suggestion
Configurator.setLevel(loggerChild.getName(), Level.INFO);
Configurator.setLevel(sibling, Level.INFO);
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerTest.java:
##########
@@ -215,6 +215,18 @@ void debugChangeLevelAllChildrenLoggers() {
assertEventCount(app.getEvents(), 5);
}
+ @Test
+ void setAllLevelsDoesNotChangeSiblingLoggers() {
+ // A logger whose name merely starts with the parent's name is a sibling, not a child, in the
+ // dot-separated hierarchy and must keep its own level
+ final String sibling = logger.getName() + "Sibling";
+ Configurator.setLevel(sibling, Level.INFO);
+ Configurator.setAllLevels(logger.getName(), Level.OFF);
+ assertEquals(Level.OFF, logger.getLevel());
+ assertEquals(Level.OFF, loggerChild.getLevel());
+ assertEquals(Level.INFO, LogManager.getLogger(sibling).getLevel(), "sibling logger must keep its level");
+ }
Review Comment:
The root and trailing-dot parents have no test. Removing `isEmpty()` or `endsWith(".")` from `isChildLogger` keeps every test green.
```suggestion
}
@Test
void setAllLevelsWithRootOrTrailingDotChangesDescendants() {
Configurator.setLevel(loggerChild.getName(), Level.INFO);
Configurator.setAllLevels(logger.getName() + ".", Level.WARN);
assertEquals(Level.WARN, loggerChild.getLevel());
Configurator.setAllLevels("", Level.ERROR);
assertEquals(Level.ERROR, loggerChild.getLevel());
}
```
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on code in PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#discussion_r4422827025
##########
src/changelog/.2.x.x/3155_AbstractConfiguration_logger_mutators.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="3155" link="https://github.com/apache/logging-log4j2/issues/3155"/>
Review Comment:
Can the entry also link this PR, after the issue line?
```suggestion
<issue id="3155" link="https://github.com/apache/logging-log4j2/issues/3155"/>
<issue id="4419" link="https://github.com/apache/logging-log4j2/pull/4419"/>
```
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6489615583
The setAllLevels tests now fail for each removed condition, and each new `AbstractConfigurationTest` case fails without its fix.
The three `existing` branches in `AbstractConfiguration` stay green when removed, and a `LoggerConfig` added with `addLogger` under a different key reaches them. Can each get a test, and can the #3155 changelog be one short line?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect logger-name hierarchy boundary in Configurator.setAllLevels [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6362427918
The boundary check matches #4414, and the sibling case fails without the fix.
Removing `charAt(...) == '.'`, `isEmpty()` or `endsWith(".")` from `Configurator.isChildLogger` keeps every test green. Can the test give `LoggerTest.child` its own level and cover the root and trailing-dot parents, as in the inline suggestions?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "sharanggupta (via GitHub) via notifications" <no...@logging.apache.org>.
sharanggupta commented on PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419#issuecomment-6588109001
The workflow runs on 3245e87 are waiting for approval. Could you approve them?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Respect the logger-name hierarchy boundary in Configurator.setAllLevels and putIfAbsent results in AbstractConfiguration [logging-log4j2]
Posted by "ramanathan1504 (via GitHub) via notifications" <no...@logging.apache.org>.
ramanathan1504 merged PR #4419:
URL: https://github.com/apache/logging-log4j2/pull/4419
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org