You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "eldwrjwt (via GitHub)" <gi...@apache.org> on 2024/11/29 17:00:23 UTC

[I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

eldwrjwt opened a new issue, #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257

   ## Description
   
   I was using a failover appender with an AsyncLoggerContextSelector and wanted to print the line number. I set includeLocation to true in the root logger, but there were still no line numbers in the logs. We encountered the same problem with the async appender, even with includeLocation set to true.
   
   I believe this is because the requiresLocation function in FailoverAppender is the default implementation in AbstractAppender, which relies on the layout inside the appender. However, for FailoverAppender, we do not have a layout, and thus fail to generate the location field in the log event object. Additionally, the Async logger uses a RingBufferLogEvent, which directly reads the location field, so there's no location to print. This issue is also true for AsyncAppender.
   
   ## Configuration
   
   **Version:** 2.24.2
   
   **Operating system:** Windows11
   
   **JDK:** OpenJDK23
   
   ## Reproduction
   
   Log4j2.xml
   `
   <Configuration>
       <Appenders>
           <Console name="CONSOLE">
               <PatternLayout pattern="%m%L%n"/>
           </Console>
   
           <Console name="CONSOLE2">
               <PatternLayout pattern="%m%L%n"/>
           </Console>
   
           <Failover name="Failover" primary="CONSOLE">
               <Failovers>
                   <AppenderRef ref="CONSOLE2"/>
               </Failovers>
           </Failover>
       </Appenders>
       <Loggers>
           <Root level="INFO" includeLocation="true">
               <AppenderRef ref="Failover"/>
           </Root>
       </Loggers>
   </Configuration>
   `
   
   log4j2.component.properties
   `
   Log4jContextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector
   `
   


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2508300272

   Hi @eldwrjwt,
   
   > I believe this is because the requiresLocation function in FailoverAppender is the default implementation in AbstractAppender, which relies on the layout inside the appender. However, for FailoverAppender, we do not have a layout, and thus fail to generate the location field in the log event object.
   
   Nice catch! :100: Can you submit a PR that fixes it?
   
   > Additionally, the Async logger uses a RingBufferLogEvent, which directly reads the location field, so there's no location to print. 
   
   The **shouldn't** be a problem:
   
   - Log4j Core 2 uses the `o.a.l.l.spi.AbstractLogger` class from Log4j API, which populates the value of the location **early**:
   
   https://github.com/apache/logging-log4j2/blob/e1715dc9f3132c70f1a87a6a75058d02a0b69d63/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java#L2861
   
   https://github.com/apache/logging-log4j2/blob/e1715dc9f3132c70f1a87a6a75058d02a0b69d63/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java#L2918-L2920
   
   - Log4j Core 3 populates the location **just before** the asynchronous barrier to give filters a chance to discard it:
   
   https://github.com/apache/logging-log4j2/blob/7fd3a1303dec3ede639dd573851143c5ac0890e1/log4j-async-logger/src/main/java/org/apache/logging/log4j/async/logger/RingBufferLogEventTranslator.java#L61-L87
   
   In both cases the `location` field is populated if `requiresLocation()` returns `true`.
   
   > This issue is also true for AsyncAppender.
   
   I agree, we have the same problem as in `Failover` here: the `AsyncAppender.requiresLocation()` implementation is incorrect.
   Could you submit a separate PR for this one?
   
   Note that `AsyncAppender` has an additional twist to make things interesting: it has an [`includeLocation` configuration attribute](https://logging.apache.org/log4j/2.x/manual/appenders/delegating.html#AsyncAppender-attr-includeLocation) that must be turned on to compute location. The implementation of `requiresLocation()` for the Async appender should probably be: `includeLocation ` is true and any of the downstream appenders has `requiresLocation() == true`.
   
   **Remark**: If location information is important to you, since last year we have a [Log4j Transform Maven plugin](https://logging.apache.org/log4j/transform/log4j-transform-maven-plugin.html), which precomputes the location of your log statements at build time.
   If the location is precomputed, it will be available regardless of any `includeLocation` setting: these settings prevent the **computation** of location information at runtime, they don't delete the information that is already there.


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "eldwrjwt (via GitHub)" <gi...@apache.org>.
eldwrjwt commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2509081129

   PR Submitted in #3259 for FailoverAppender


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "eldwrjwt (via GitHub)" <gi...@apache.org>.
eldwrjwt commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2509818683

   Another PR #3260 for AsyncAppender


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "eldwrjwt (via GitHub)" <gi...@apache.org>.
eldwrjwt commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2508466527

   Hi @ppkarwasz,
   I can submit a PR to fix the issue later on. If my understanding is correct, I think I can write a correct implementation of `requiresLocation()` for `FailoverAppender` (and `AsyncAppender` in another PR), and this should fix the bug.


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2573165646

   @eldwrjwt,
   
   Thanks for the PRs. Your fixes have been included in the latest `2.25.0-SNAPSHOT`.
   See the [download page](https://logging.apache.org/download.html) for the address of our snapshot Nexus repository.


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz closed issue #3257: File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector
URL: https://github.com/apache/logging-log4j2/issues/3257


-- 
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: [I] File line numbers are not printed for the failover appender when using AsyncLoggerContextSelector (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3257:
URL: https://github.com/apache/logging-log4j2/issues/3257#issuecomment-2508676006

   > Hi @ppkarwasz, I can submit a PR to fix the issue later on. If my understanding is correct, I think I can write a correct implementation of `requiresLocation()` for `FailoverAppender` (and `AsyncAppender` in another PR), and this should fix the bug.
   
   That's great! Yes, `requiresLocation()` is the method that needs fixing. Please add some unit tests too.


-- 
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