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:32:31 UTC

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

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

 ##########
 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:
   I'm not aware of any built-in link validation in javadoc. This just cleans it up slightly to use proper javadoc syntax to reference a resource.

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