You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/02/25 21:09:55 UTC

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1270: ZOOKEEPER-3737: Detect log4j 1.2 jmx support better

eolivelli commented on a change in pull request #1270: ZOOKEEPER-3737: Detect log4j 1.2 jmx support better
URL: https://github.com/apache/zookeeper/pull/1270#discussion_r384126953
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/jmx/ManagedUtil.java
 ##########
 @@ -35,27 +35,25 @@
     private static boolean isLog4jJmxEnabled() {
         boolean enabled = false;
 
-        try {
-            Class.forName("org.apache.log4j.spi.LoggerRepository");
-
-            if (Boolean.getBoolean("zookeeper.jmx.log4j.disable")) {
-                LOG.info("Log4j found but jmx support is disabled.");
-            } else {
+        if (Boolean.getBoolean("zookeeper.jmx.log4j.disable")) {
+            LOG.info("Log4j 1.2 jmx support is disabled by property.");
+        } else {
+            try {
+                Class.forName("org.apache.log4j.jmx.HierarchyDynamicMBean");
                 enabled = true;
-                LOG.info("Log4j found with jmx enabled.");
+                LOG.info("Log4j 1.2 jmx support found and enabled.");
+            } catch (ClassNotFoundException e) {
+                LOG.info("Log4j 1.2 jmx support not found; jmx disabled.");
             }
-
-        } catch (ClassNotFoundException e) {
-            LOG.info("Log4j not found.");
         }
 
         return enabled;
     }
 
     /**
-     * Register the log4j JMX mbeans. Set environment variable
+     * Register the log4j JMX mbeans. Set system property
      * "zookeeper.jmx.log4j.disable" to true to disable registration.
-     * See http://logging.apache.org/log4j/1.2/apidocs/index.html?org/apache/log4j/jmx/package-summary.html
+     * @see http://logging.apache.org/log4j/1.2/apidocs/index.html?org/apache/log4j/jmx/package-summary.html
 
 Review comment:
   Just for curiosity:
   I am not sure it is a good thing to link to an external website using @see 
   It is not so relevant this link and javadoc tools may want to validate this link or to download?
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services