Issue #5872 - JMX DynamicMBean for jetty-slf4j-impl

jetty-jmx/src/main/config/etc/jetty-jmx.xml

@@ -18,6 +18,12 @@
         <Arg>
           <Ref refid="MBeanServer" />
         </Arg>
+        <Call name="beanAdded">
+          <Arg/>
+          <Arg>
+            <Get name="ILoggerFactory" class="org.slf4j.LoggerFactory"/>
+          </Arg>
+        </Call>
       </New>
     </Arg>
   </Call>

jetty-slf4j-impl/src/main/java/module-info.java

@@ -18,6 +18,7 @@
 {
     exports org.eclipse.jetty.logging;
 
+    requires transitive java.management;
     requires transitive org.slf4j;
 
     provides SLF4JServiceProvider with JettyLoggingServiceProvider;

jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java

@@ -19,15 +19,28 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import javax.management.Attribute;
+import javax.management.AttributeList;
+import javax.management.AttributeNotFoundException;
+import javax.management.DynamicMBean;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanConstructorInfo;
+import javax.management.MBeanException;
+import javax.management.MBeanInfo;
+import javax.management.MBeanNotificationInfo;
+import javax.management.MBeanOperationInfo;
+import javax.management.MBeanParameterInfo;
+import javax.management.ReflectionException;
 
 import org.slf4j.ILoggerFactory;
 import org.slf4j.Logger;
 
-public class JettyLoggerFactory implements ILoggerFactory, JettyLoggerFactoryMBean
+public class JettyLoggerFactory implements ILoggerFactory, DynamicMBean
 {
     private final JettyLoggerConfiguration configuration;
     private final JettyLogger rootLogger;
     private final ConcurrentMap<String, JettyLogger> loggerMap;
+    private MBeanInfo mBeanInfo;
 
     public JettyLoggerFactory(JettyLoggerConfiguration config)
     {
@@ -129,20 +142,17 @@ static <T> T walkParentLoggerNames(String startName, Function<String, T> nameFun
         return nameFunction.apply(Logger.ROOT_LOGGER_NAME);
     }
 
-    @Override
     public String[] getLoggerNames()
     {
         TreeSet<String> names = new TreeSet<>(loggerMap.keySet());
         return names.toArray(new String[0]);
     }
 
-    @Override
     public int getLoggerCount()
     {
         return loggerMap.size();
     }
 
-    @Override
     public String getLoggerLevel(String loggerName)
     {
         return walkParentLoggerNames(loggerName, key ->
@@ -154,7 +164,6 @@ public String getLoggerLevel(String loggerName)
         });
     }
 
-    @Override
     public boolean setLoggerLevel(String loggerName, String levelName)
     {
         JettyLevel level = JettyLoggerConfiguration.toJettyLevel(loggerName, levelName);
@@ -166,4 +175,161 @@ public boolean setLoggerLevel(String loggerName, String levelName)
         jettyLogger.setLevel(level);
         return true;
     }
+
+    @Override
+    public Object getAttribute(String name) throws AttributeNotFoundException
+    {
+        Objects.requireNonNull(name, "Attribute Name");
+
+        switch (name)
+        {
+            case "LoggerNames":
+                return getLoggerNames();
+            case "LoggerCount":
+                return getLoggerCount();
+            default:
+                throw new AttributeNotFoundException("Cannot find " + name + " attribute in " + this.getClass().getName());
+        }
+    }
+
+    @Override
+    public void setAttribute(Attribute attribute) throws AttributeNotFoundException
+    {
+        Objects.requireNonNull(attribute, "attribute");
+        String name = attribute.getName();
+        // No attributes are writable
+        throw new AttributeNotFoundException("Cannot set attribute " + name + " because it is read-only");
+    }
+
+    @Override
+    public AttributeList getAttributes(String[] attributeNames)
+    {
+        Objects.requireNonNull(attributeNames, "attributeNames[]");
+
+        AttributeList ret = new AttributeList();
+        if (attributeNames.length == 0)
+            return ret;
+
+        for (String name : attributeNames)
+        {
+            try
+            {
+                Object value = getAttribute(name);
+                ret.add(new Attribute(name, value));
+            }
+            catch (Exception e)
+            {
+                // nothing much we can do, this method has no throwables, and we cannot use logging here.
+                e.printStackTrace();
+            }
+        }
+        return ret;
+    }
+
+    @Override
+    public AttributeList setAttributes(AttributeList attributes)
+    {
+        Objects.requireNonNull(attributes, "attributes");
+
+        AttributeList ret = new AttributeList();
+
+        if (attributes.isEmpty())
+            return ret;
+
+        for (Attribute attr : attributes.asList())
+        {
+            try
+            {
+                setAttribute(attr);
+                String name = attr.getName();
+                Object value = getAttribute(name);
+                ret.add(new Attribute(name, value));
+            }
+            catch (Exception e)
+            {
+                // nothing much we can do, this method has no throwables, and we cannot use logging here.
+                e.printStackTrace();
+            }
+        }
+        return ret;
+    }
+
+    @Override
+    public Object invoke(String actionName, Object[] params, String[] signature) throws MBeanException, ReflectionException
+    {
+        Objects.requireNonNull(actionName, "Action Name");
+
+        switch (actionName)
+        {
+            case "setLoggerLevel":
+            {
+                String loggerName = (String)params[0];
+                String level = (String)params[1];
+                return setLoggerLevel(loggerName, level);
+            }
+            case "getLoggerLevel":
+            {
+                String loggerName = (String)params[0];
+                return getLoggerLevel(loggerName);
+            }
+            default:
+                throw new ReflectionException(
+                    new NoSuchMethodException(actionName),
+                    "Cannot find the operation " + actionName + " in " + this.getClass().getName());
+        }
+    }
+
+    @Override
+    public MBeanInfo getMBeanInfo()
+    {
+        if (mBeanInfo == null)
+        {
+            MBeanAttributeInfo[] attrs = new MBeanAttributeInfo[2];
+
+            attrs[0] = new MBeanAttributeInfo(
+                "LoggerCount",
+                "java.lang.Integer",
+                "Count of Registered Loggers by Name.",
+                true,
+                false,
+                false);
+            attrs[1] = new MBeanAttributeInfo(
+                "LoggerNames",
+                "java.lang.String[]",
+                "List of Registered Loggers by Name.",
+                true,
+                false,
+                false);
+
+            MBeanOperationInfo[] operations = new MBeanOperationInfo[]{
+                new MBeanOperationInfo(
+                    "setLoggerLevel",
+                    "Set the logging level at the named logger",
+                    new MBeanParameterInfo[]{
+                        new MBeanParameterInfo("loggerName", "java.lang.String", "The name of the logger"),
+                        new MBeanParameterInfo("level", "java.lang.String", "The name of the level [DEBUG, INFO, WARN, ERROR]")
+                    },
+                    "boolean",
+                    MBeanOperationInfo.ACTION
+                ),
+                new MBeanOperationInfo(
+                    "getLoggerLevel",
+                    "Get the logging level at the named logger",
+                    new MBeanParameterInfo[]{
+                        new MBeanParameterInfo("loggerName", "java.lang.String", "The name of the logger")
+                    },
+                    "java.lang.String",
+                    MBeanOperationInfo.INFO
+                )
+            };
+
+            mBeanInfo = new MBeanInfo(this.getClass().getName(),
+                "Jetty Slf4J Logger Factory",
+                attrs,
+                new MBeanConstructorInfo[0],
+                operations,
+                new MBeanNotificationInfo[0]);
+        }
+        return mBeanInfo;
+    }
 }

jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java

@@ -18,13 +18,17 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
-import java.util.Properties;
-import javax.management.JMX;
+import java.util.stream.Stream;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 
 import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -35,34 +39,56 @@ public void testJMX() throws Exception
     {
         MBeanServer mbeanServer = ManagementFactory.getPlatformMBeanServer();
 
-        Properties props = new Properties();
-        JettyLoggerConfiguration config = new JettyLoggerConfiguration(props);
-        JettyLoggerFactory loggerFactory = new JettyLoggerFactory(config);
-
         ObjectName objectName = ObjectName.getInstance("org.eclipse.jetty.logging", "type", JettyLoggerFactory.class.getSimpleName().toLowerCase(Locale.ENGLISH));
-        mbeanServer.registerMBean(loggerFactory, objectName);
+        mbeanServer.registerMBean(LoggerFactory.getILoggerFactory(), objectName);
+
+        // Verify MBeanInfo
+        MBeanInfo beanInfo = mbeanServer.getMBeanInfo(objectName);
+
+        MBeanAttributeInfo[] attributeInfos = beanInfo.getAttributes();
+        assertThat("MBeanAttributeInfo count", attributeInfos.length, is(2));
 
-        JettyLoggerFactoryMBean mbean = JMX.newMBeanProxy(mbeanServer, objectName, JettyLoggerFactoryMBean.class);
+        MBeanAttributeInfo attr = Stream.of(attributeInfos).filter((a) -> a.getName().equals("LoggerNames")).findFirst().orElseThrow();
+        assertThat("attr", attr.getDescription(), is("List of Registered Loggers by Name."));
+
+        // Do some MBean attribute testing
+        int loggerCount;
 
         // Only the root logger.
-        assertEquals(1, mbean.getLoggerCount());
+        loggerCount = (int)mbeanServer.getAttribute(objectName, "LoggerCount");
+        assertEquals(1, loggerCount);
 
+        JettyLoggerFactory loggerFactory = (JettyLoggerFactory)LoggerFactory.getILoggerFactory();
         JettyLogger child = loggerFactory.getJettyLogger("org.eclipse.jetty.logging");
         JettyLogger parent = loggerFactory.getJettyLogger("org.eclipse.jetty");
-        assertEquals(3, mbean.getLoggerCount());
+        loggerCount = (int)mbeanServer.getAttribute(objectName, "LoggerCount");
+        assertEquals(3, loggerCount);
 
-        // Names are sorted.
+        // Names from JMX are sorted, so lets sort our expected list too.
         List<String> expected = new ArrayList<>(Arrays.asList(JettyLogger.ROOT_LOGGER_NAME, parent.getName(), child.getName()));
         expected.sort(String::compareTo);
-        String[] loggerNames = mbean.getLoggerNames();
+        String[] loggerNames = (String[])mbeanServer.getAttribute(objectName, "LoggerNames");
         assertEquals(expected, Arrays.asList(loggerNames));
 
+        // Do some MBean invoker testing
+        String operationName;
+        String[] signature;
+        Object[] params;
+
         // Setting the parent level should propagate to the children.
         parent.setLevel(JettyLevel.DEBUG);
-        assertEquals(parent.getLevel().toString(), mbean.getLoggerLevel(child.getName()));
+        operationName = "getLoggerLevel";
+        signature = new String[]{String.class.getName()};
+        params = new Object[]{child.getName()};
+        String levelName = (String)mbeanServer.invoke(objectName, operationName, params, signature);
+        assertEquals(parent.getLevel().toString(), levelName);
 
         // Setting the level via JMX affects the logger.
-        assertTrue(mbean.setLoggerLevel(child.getName(), "INFO"));
+        operationName = "setLoggerLevel";
+        signature = new String[]{String.class.getName(), String.class.getName()};
+        params = new Object[]{child.getName(), "INFO"};
+        boolean result = (boolean)mbeanServer.invoke(objectName, operationName, params, signature);
+        assertTrue(result);
         assertEquals(JettyLevel.INFO, child.getLevel());
     }
 }