You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/03/02 03:43:02 UTC
[PR] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
panbingkun opened a new pull request, #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340
Follow up for https://github.com/apache/logging-log4j2/pull/2338,
Fixes https://github.com/apache/logging-log4j2/issues/2337 and makes StatusLogger take debug mode into account.
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1974785178
The local verification results are as follows:
```
...
2024-03-02T12:30:05.718720Z main DEBUG Registering MBean org.apache.logging.log4j2:type=5ffd2b27
2024-03-02T12:30:05.719599Z main DEBUG Registering MBean org.apache.logging.log4j2:type=5ffd2b27,component=StatusLogger
2024-03-02T12:30:05.719958Z main DEBUG Registering MBean org.apache.logging.log4j2:type=5ffd2b27,component=ContextSelector
2024-03-02T12:30:05.720390Z main DEBUG Registering MBean org.apache.logging.log4j2:type=5ffd2b27,component=Loggers,name=
2024-03-02T12:30:05.720736Z main DEBUG Registering MBean org.apache.logging.log4j2:type=5ffd2b27,component=Appenders,name=console
2024-03-02T12:30:05.721543Z main TRACE Using default SystemClock for timestamps.
2024-03-02T12:30:05.721726Z main DEBUG org.apache.logging.log4j.core.util.SystemClock supports precise timestamps.
2024-03-02T12:30:05.721839Z main TRACE Using DummyNanoClock for nanosecond timestamps.
2024-03-02T12:30:05.721929Z main DEBUG Reconfiguration complete for context[name=5ffd2b27] at URI /Users/panbingkun/Developer/spark/spark-client/spark-4.0.0-SNAPSHOT-bin-3.3.6/conf/log4j2.properties (org.apache.logging.log4j.core.LoggerContext@3857f613) with optional ClassLoader: null
2024-03-02T12:30:05.721979Z main DEBUG Shutdown hook enabled. Registering a new one.
2024-03-02T12:30:05.722241Z main DEBUG LoggerContext[name=5ffd2b27, org.apache.logging.log4j.core.LoggerContext@3857f613] started OK.
2024-03-02T12:30:05.769692Z main TRACE Log4jLoggerFactory.getContext() found anchor interface org.apache.spark.internal.Logging
2024-03-02T12:30:05.849111Z main TRACE Log4jLoggerFactory.getContext() found anchor class org.apache.spark.network.util.JavaUtils
2024-03-02T12:30:05.951557Z main TRACE Log4jLoggerFactory.getContext() found anchor class org.apache.hadoop.conf.Configuration
2024-03-02T12:30:05.951733Z main TRACE Log4jLoggerFactory.getContext() found anchor class org.apache.hadoop.conf.Configuration
2024-03-02T12:30:05.957285Z main TRACE Log4jLoggerFactory.getContext() found anchor class org.apache.hadoop.util.Preconditions
2024-03-02T12:30:05.998250Z main TRACE Log4jLoggerFactory.getContext() found anchor interface org.apache.spark.internal.Logging
2024-03-02T12:30:05.999710Z main DEBUG AsyncLogger.ThreadNameStrategy=UNCACHED (user specified null, default is UNCACHED)
2024-03-02T12:30:05.999769Z main TRACE Using default SystemClock for timestamps.
2024-03-02T12:30:05.999811Z main DEBUG org.apache.logging.log4j.core.util.SystemClock supports precise timestamps.
...
```
It's okay.
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1975010214
May I friendly ask a question, do we have a planned `release date` for the official version `2.23.1`?
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1976011256
> > do we have a planned `release date` for the official version `2.23.1`?
>
> Yes, it was last week, until you told us `StatusLogger` doesn't work. 😝 Putting joke aside, I think we will try again this week.
Haha.., thank you very much! ❤️
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1974257766
cc @vy @ppkarwasz
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#discussion_r1509954940
##########
log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java:
##########
@@ -134,7 +165,7 @@ public Level getStatusLevel() {
@Override
public void log(final StatusData data) {
requireNonNull(data, "data");
- if (level.isLessSpecificThan(data.getLevel())) {
+ if (debugEnabled || level.isLessSpecificThan(data.getLevel())) {
Review Comment:
Sure, let me try making it.
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#discussion_r1509919282
##########
log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java:
##########
@@ -134,7 +165,7 @@ public Level getStatusLevel() {
@Override
public void log(final StatusData data) {
requireNonNull(data, "data");
- if (level.isLessSpecificThan(data.getLevel())) {
+ if (debugEnabled || level.isLessSpecificThan(data.getLevel())) {
Review Comment:
I think the correct fix is to remove the filtering on level here. `StatusLogger` is already taking care of the filtering, listeners should not (incorrectly!) duplicate that work. Hence, instead of leaking `debugEnabled` to `StatusConsoleListener`, could you remove this `if` block, please?
As a result of this change, `StatusConsoleListenerTest#level_and_stream_should_be_honored` test will be broken. Could you rename it to `stream_should_be_honored` and fix it too, please? There you won't need multiple messages with different levels anymore. Checking the dump of a single message should be good enough.
I can share some development tips to quickly iterate over your changes:
* Compile your `log4j-api` changes: `./mvnw clean install -Dmaven.test.skip -Dspotless.skip -Dcyclonedx.skip -Dspotbugs.skip -Drat.skip -Dbnd.baseline.skip -pl :log4j-api`
* Make sure you fixed `StatusConsoleListenerTest`: `./mvnw test -Dspotless.skip -Dcyclonedx.skip -Dspotbugs.skip -Drat.skip -Dbnd.baseline.skip -pl :log4j-api-test -Dtest=StatusConsoleListenerTest`
* Fix formatting: `./mvnw spotless:apply -pl :log4j-api,:log4j-api-test`
* Make sure all is in good shape: `./mvnw clean install -pl :log4j-api,:log4j-api-test`
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1974874685
Great job! :100:
Thanks @panbingkun! :bow:
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy merged PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340
--
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] [FOLLOWUP] Fix StatusLogger log level filtering when debug mode is enabled (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #2340:
URL: https://github.com/apache/logging-log4j2/pull/2340#issuecomment-1976000836
> do we have a planned `release date` for the official version `2.23.1`?
Yes, it was last week, until you told us `StatusLogger` doesn't work. :stuck_out_tongue_closed_eyes: Putting joke aside, I think we will try again this week.
--
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