You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/02/20 16:31:07 UTC
[GitHub] [logging-log4j2] ppkarwasz opened a new pull request #764: Adds Log4j 1.x global threshold
ppkarwasz opened a new pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764
The `log4j.threshold` configuration key was not translated into its Log4j 2.x equivalent global filter.
--
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
[GitHub] [logging-log4j2] garydgregory merged pull request #764: Adds Log4j 1.x global threshold
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764
--
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
[GitHub] [logging-log4j2] garydgregory commented on pull request #764: Log4j 1.2 bridge supports global threshold
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764#issuecomment-1047183989
@ppkarwasz
Thank you for your PR.
If you are looking for more fixes to investigate, please have a look at the disabled tests in `org.apache.log4j.xml.DOMTestCase`.
--
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
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #764: Adds Log4j 1.x global threshold
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764#discussion_r810672064
##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
##########
@@ -103,6 +107,18 @@
if (Boolean.parseBoolean(debugValue)) {
builder.setStatusLevel(Level.DEBUG);
}
+ // if log4j.reset=true then reset hierarchy
+ final String reset = getLog4jValue("reset");
+ if (reset != null && OptionConverter.toBoolean(reset, false)) {
Review comment:
I do not see this reset setting tested in this PR. Did I miss it? Also, elsewhere we call the reset API from a doConfigure() method, it seems odd to call it here. Is it then called twice or should this call be moved to the site when the configuration is actually applied? @rgoers WDYT?
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on a change in pull request #764: Adds Log4j 1.x global threshold
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764#discussion_r810676890
##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
##########
@@ -103,6 +107,18 @@
if (Boolean.parseBoolean(debugValue)) {
builder.setStatusLevel(Level.DEBUG);
}
+ // if log4j.reset=true then reset hierarchy
+ final String reset = getLog4jValue("reset");
+ if (reset != null && OptionConverter.toBoolean(reset, false)) {
Review comment:
Yes, you are right. I copy-pasted the "reset" part from the `PropertiesConfiguration`, without checking exactly what it does. I'll remove it for now (besides the `XmlConfiguration` does not recognize this option).
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on a change in pull request #764: Adds Log4j 1.x global threshold
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #764:
URL: https://github.com/apache/logging-log4j2/pull/764#discussion_r810680613
##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1ConfigurationParser.java
##########
@@ -103,6 +107,18 @@
if (Boolean.parseBoolean(debugValue)) {
builder.setStatusLevel(Level.DEBUG);
}
+ // if log4j.reset=true then reset hierarchy
+ final String reset = getLog4jValue("reset");
+ if (reset != null && OptionConverter.toBoolean(reset, false)) {
Review comment:
Looking at the Log4j 1.2 code, neither `Log4j1ConfigurationParser` nor any `Configuration` class should check the _"log4j.reset"_ key. This should be done in `PropertyConfigurator#configure` to decide whether to replace the old configuration or to make a `CompositeConfiguration` out of them.
Looking at [this SO question](https://stackoverflow.com/q/71087714/11748454) I have the impression that calling `PropertyConfigurator#configure` multiple times without resetting the old config, was a common practice.
--
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