Directory ApacheDS
  1. Directory ApacheDS
  2. DIRSERVER-1804

[patch] Fix ApacheDS code to allow control and reduce number of outputted logs

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-M10
    • Fix Version/s: 2.0.0-M11
    • Component/s: None

      Description

      I am using Maven artifact apacheds-all version 2.0.0-M10 (http://mvnrepository.com/artifact/org.apache.directory.server/apacheds-all/2.0.0-M10).

      I want to start and stop the server from my Java code (in a way similar to the one described in the official 'embedded sample': http://svn.apache.org/repos/asf/directory/documentation/samples/trunk/embedded-sample/src/main/java/org/apache/directory/seserver/EmbeddedADSVer157.java).

      The problem is: ApacheDS start takes 7 minutes and recuces to 3 seconds when standard output is redirected to /dev/null. Reason: you make lots of debug-level logging which take 99.9% of the startup time.

      I would like to restrict logging from ApacheDS to ERROR level. However, it is not possible because of the flawed ApacheDS code.

      You use SLF4J loggers this way:
      private static final Logger LOG = LoggerFactory.getLogger( JdbmIndex.class.getSimpleName() );

      This is absolutely unacceptable. The result of this code that the logger would be created for name 'JdbmIndex' and there is no way to use logger inheritance to control all logging from ApacheDS code, without affecting my own code. I don't want to look through ApacheDS code, find classes that fail to use logging correctly and silence each of these classes one by one in my own logging configuration.

      The correct way for using logging in Java is to use:
      private static final Logger LOG = LoggerFactory.getLogger( JdbmIndex.class );

      Then the logger would be created for name 'org.apache.directory.server.core.partition.impl.btree.jdbm.JdbmIndex' and I would be able to silence the whole 'org.apache.directory' or just 'org.apache.directory.server.core.partition' - or any part I like.


      I created a patch based on the today version of trunk. It introduces a single correct way of creating loggers across all ApacheDS code. I have also updated your logging configuration in log4j.properties to make sure the application behaviour won't change after my fixes.

      1. slf4j-logging.patch
        33 kB
        Piotr Kubowicz

        Activity

        Hide
        Piotr Kubowicz added a comment - - edited

        Attached a patch: slf4j-logging.patch.

        Show
        Piotr Kubowicz added a comment - - edited Attached a patch: slf4j-logging.patch.
        Hide
        Emmanuel Lecharny added a comment -

        You are absolutely right. I think we made a mistake by using xxx.class.getSimpleName() in a few classes.

        Note that we have dedicated loggers (in capital letters, like ATTRIBUTE_TYPE etc) that need to be documented (there is a JIRA for that).

        Path applied, tests running.

        Show
        Emmanuel Lecharny added a comment - You are absolutely right. I think we made a mistake by using xxx.class.getSimpleName() in a few classes. Note that we have dedicated loggers (in capital letters, like ATTRIBUTE_TYPE etc) that need to be documented (there is a JIRA for that). Path applied, tests running.
        Hide
        Piotr Kubowicz added a comment -

        Thank you.

        Show
        Piotr Kubowicz added a comment - Thank you.
        Hide
        Emmanuel Lecharny added a comment -

        Fixed with http://svn.apache.org/r1446546

        Thanks for the patch !

        Show
        Emmanuel Lecharny added a comment - Fixed with http://svn.apache.org/r1446546 Thanks for the patch !
        Hide
        Emmanuel Lecharny added a comment -

        One more thing : you can reduce the logs by simply changing the log4j.rootCategory to ERROR, and by removing all the log4j.logger categories.

        Something as simple a :
        log4j.rootCategory=ERROR, stdout

        log4j.appender.stdout=org.apache.log4j.ConsoleAppender
        log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
        log4j.appender.stdout.layout.ConversionPattern=[%d

        {HH:mm:ss}

        ] %p [%c-%X

        {Replica}

        ] - %m%n

        will reduce the logs to a minimum.

        Show
        Emmanuel Lecharny added a comment - One more thing : you can reduce the logs by simply changing the log4j.rootCategory to ERROR, and by removing all the log4j.logger categories. Something as simple a : log4j.rootCategory=ERROR, stdout log4j.appender.stdout=org.apache.log4j.ConsoleAppender log4j.appender.stdout.layout=org.apache.log4j.PatternLayout log4j.appender.stdout.layout.ConversionPattern=[%d {HH:mm:ss} ] %p [%c-%X {Replica} ] - %m%n will reduce the logs to a minimum.
        Hide
        Piotr Kubowicz added a comment -

        This would affect logging of my own code - I still want to see WARN messages from my classes.

        Show
        Piotr Kubowicz added a comment - This would affect logging of my own code - I still want to see WARN messages from my classes.
        Hide
        Emmanuel Lecharny added a comment -

        I understand. Still if you limit the log level to WARN, and remove all the Apacheds log4j.logger, you should have a very limited amount of logs.

        Show
        Emmanuel Lecharny added a comment - I understand. Still if you limit the log level to WARN, and remove all the Apacheds log4j.logger, you should have a very limited amount of logs.
        Hide
        Piotr Kubowicz added a comment - - edited

        Maybe it would also be good if you move 'dedicated' loggers to a namespace, for example rename 'ATTRIBUTE_TYPE' to 'org.apache.directory.server.ATTRIBUTE_TYPE' (or something more nested, like .server.dedicated.ATTRIBUTE_TYPE). This way those loggers would fit nicely into logger hierarchy, i.e. you would be able to see enable debug logs everywhere or restrict visible messages only to ERROR with a single command like:
        log4j.logger.org.apache.directory.server=DEBUG

        Your code would benefit from making all loggers being part of some hierarchy, for example ldap-client-test/src/test/resources/log4j.properties might become something as simple as
        log4j.logger.net=FATAL
        log4j.logger.org.apache.directory=FATAL

        Note that currently you need to modify all your log4j.properties when you introduce a new 'dedicated' logger. If those were all part of a hierarchy, adding a new dedicated logger wouldn't require any changes in log4j.properties, because you would be able to cover all with a parent logger rule.

        Show
        Piotr Kubowicz added a comment - - edited Maybe it would also be good if you move 'dedicated' loggers to a namespace, for example rename 'ATTRIBUTE_TYPE' to 'org.apache.directory.server.ATTRIBUTE_TYPE' (or something more nested, like .server.dedicated.ATTRIBUTE_TYPE). This way those loggers would fit nicely into logger hierarchy, i.e. you would be able to see enable debug logs everywhere or restrict visible messages only to ERROR with a single command like: log4j.logger.org.apache.directory.server=DEBUG Your code would benefit from making all loggers being part of some hierarchy, for example ldap-client-test/src/test/resources/log4j.properties might become something as simple as log4j.logger.net=FATAL log4j.logger.org.apache.directory=FATAL Note that currently you need to modify all your log4j.properties when you introduce a new 'dedicated' logger. If those were all part of a hierarchy, adding a new dedicated logger wouldn't require any changes in log4j.properties, because you would be able to cover all with a parent logger rule.
        Hide
        Emmanuel Lecharny added a comment -

        Smart ! I'll do that for dedicated logger.

        Thanks !

        Show
        Emmanuel Lecharny added a comment - Smart ! I'll do that for dedicated logger. Thanks !
        Hide
        Emmanuel Lecharny added a comment -

        Modification done : the dedicated loggers are now prefixed with org.apache.directory

        Show
        Emmanuel Lecharny added a comment - Modification done : the dedicated loggers are now prefixed with org.apache.directory
        Hide
        Emmanuel Lecharny added a comment -

        Closed all the resolved issues

        Show
        Emmanuel Lecharny added a comment - Closed all the resolved issues

          People

          • Assignee:
            Unassigned
            Reporter:
            Piotr Kubowicz
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development