Skip to content

Conversation

@AndresArcones
Copy link

@AndresArcones AndresArcones commented Jul 10, 2024

In the current implementation of the log4j ScalyrAppender, the getLayout() method will always return null because the layout is not being read. This occurs because the requiresLayout() method is set to return false in ScalyrAppender. As a result, even if a layout is configured for this appender in the logger configuration, it will not be utilized. To fix this issue, the requiresLayout() method should return true, allowing the layout to be read and applied properly.

For reference, see the Appender class in Apache Log4j where requiresLayout() is used to determine if a layout is required.

@AndresArcones
Copy link
Author

AndresArcones commented Apr 23, 2025

is there any maintainers on this repository that have the right to merge this pull request? @steve-scalyr @soboko @moriarty-s3a @rvangsgaard @moriarty-s3a @ArthurKamalov

@rvangsgaard
Copy link
Contributor

It has been a while that I have been using this repository with log4j, so I am curious what happens when requiresLayout() returns true, and no configuration information is at its disposal?

Can a test show that both true and false will work as expected?

Documentation copied from log4j:

  /**
     Configurators call this method to determine if the appender
    requires a layout. If this method returns <code>true</code>,
    meaning that layout is required, then the configurator will
    configure an layout using the configuration information at its
    disposal.  If this method returns <code>false</code>, meaning that
    a layout is not required, then layout configuration will be
    skipped even if there is available layout configuration
    information at the disposal of the configurator..

     <p>In the rather exceptional case, where the appender
     implementation admits a layout but can also work without it, then
     the appender should return <code>true</code>.
     
     @since 0.8.4 */
  public
  boolean requiresLayout();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants