From 37e736170697ebf3452dfc6cf6d74c464f3037f4 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 12 Jan 2021 14:25:37 -0600 Subject: [PATCH 1/3] Issue #5872 - JMX DynamicMBean for jetty-slf4j-impl Signed-off-by: Joakim Erdfelt --- jetty-jmx/src/main/config/etc/jetty-jmx.xml | 6 + .../src/main/java/module-info.java | 1 + .../jetty/logging/JettyLoggerFactory.java | 181 +++++++++++++++++- .../org/eclipse/jetty/logging/JMXTest.java | 23 ++- 4 files changed, 200 insertions(+), 11 deletions(-) diff --git a/jetty-jmx/src/main/config/etc/jetty-jmx.xml b/jetty-jmx/src/main/config/etc/jetty-jmx.xml index 052fa7cc506..5aadbb66a70 100644 --- a/jetty-jmx/src/main/config/etc/jetty-jmx.xml +++ b/jetty-jmx/src/main/config/etc/jetty-jmx.xml @@ -18,6 +18,12 @@ + + + + + + diff --git a/jetty-slf4j-impl/src/main/java/module-info.java b/jetty-slf4j-impl/src/main/java/module-info.java index ba708ec0910..c0952c8bcd0 100644 --- a/jetty-slf4j-impl/src/main/java/module-info.java +++ b/jetty-slf4j-impl/src/main/java/module-info.java @@ -18,6 +18,7 @@ module org.eclipse.jetty.logging { exports org.eclipse.jetty.logging; + requires transitive java.management; requires transitive org.slf4j; provides SLF4JServiceProvider with JettyLoggingServiceProvider; diff --git a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java b/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java index 646e517fbda..3b06a80c759 100644 --- a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java +++ b/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java @@ -19,15 +19,28 @@ import java.util.concurrent.ConcurrentHashMap; 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, JettyLoggerFactoryMBean { private final JettyLoggerConfiguration configuration; private final JettyLogger rootLogger; private final ConcurrentMap loggerMap; + private MBeanInfo mBeanInfo; public JettyLoggerFactory(JettyLoggerConfiguration config) { @@ -129,20 +142,17 @@ public class JettyLoggerFactory implements ILoggerFactory, JettyLoggerFactoryMBe return nameFunction.apply(Logger.ROOT_LOGGER_NAME); } - @Override public String[] getLoggerNames() { TreeSet 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 class JettyLoggerFactory implements ILoggerFactory, JettyLoggerFactoryMBe }); } - @Override public boolean setLoggerLevel(String loggerName, String levelName) { JettyLevel level = JettyLoggerConfiguration.toJettyLevel(loggerName, levelName); @@ -166,4 +175,166 @@ public class JettyLoggerFactory implements ILoggerFactory, JettyLoggerFactoryMBe 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; + } + + /* + setLoggerLevel(String loggerName, String levelName); + String getLoggerLevel(String loggerName); + */ + + @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; + } } diff --git a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java index c345c317995..acada2f6af8 100644 --- a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java +++ b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java @@ -18,13 +18,18 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Locale; -import java.util.Properties; +import java.util.stream.Stream; import javax.management.JMX; +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,18 +40,24 @@ public class JMXTest { 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)); + + MBeanAttributeInfo attr = Stream.of(attributeInfos).filter((a) -> a.getName().equals("LoggerNames")).findFirst().orElseThrow(); + assertThat("attr", attr.getDescription(), is("List of Registered Loggers by Name.")); JettyLoggerFactoryMBean mbean = JMX.newMBeanProxy(mbeanServer, objectName, JettyLoggerFactoryMBean.class); // Only the root logger. assertEquals(1, mbean.getLoggerCount()); + JettyLoggerFactory loggerFactory = (JettyLoggerFactory)LoggerFactory.getILoggerFactory(); JettyLogger child = loggerFactory.getJettyLogger("org.eclipse.jetty.logging"); JettyLogger parent = loggerFactory.getJettyLogger("org.eclipse.jetty"); assertEquals(3, mbean.getLoggerCount()); From 5f9ea7804a48bae16cdb3f6d00d5d1017d800296 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 22 Jan 2021 10:08:46 -0600 Subject: [PATCH 2/3] Issue #5872 - DynamicMBean for JettyLoggerFactory Signed-off-by: Joakim Erdfelt --- .../jetty/logging/JettyLoggerFactory.java | 7 +---- .../logging/JettyLoggerFactoryMBean.java | 25 --------------- .../org/eclipse/jetty/logging/JMXTest.java | 31 ++++++++++++++----- 3 files changed, 24 insertions(+), 39 deletions(-) delete mode 100644 jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactoryMBean.java diff --git a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java b/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java index 630976ad64d..6556a491a8a 100644 --- a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java +++ b/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java @@ -35,7 +35,7 @@ import javax.management.ReflectionException; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; -public class JettyLoggerFactory implements ILoggerFactory, DynamicMBean, JettyLoggerFactoryMBean +public class JettyLoggerFactory implements ILoggerFactory, DynamicMBean { private final JettyLoggerConfiguration configuration; private final JettyLogger rootLogger; @@ -254,11 +254,6 @@ public class JettyLoggerFactory implements ILoggerFactory, DynamicMBean, JettyLo return ret; } - /* - setLoggerLevel(String loggerName, String levelName); - String getLoggerLevel(String loggerName); - */ - @Override public Object invoke(String actionName, Object[] params, String[] signature) throws MBeanException, ReflectionException { diff --git a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactoryMBean.java b/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactoryMBean.java deleted file mode 100644 index 6e9bab31fbb..00000000000 --- a/jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactoryMBean.java +++ /dev/null @@ -1,25 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.logging; - -public interface JettyLoggerFactoryMBean -{ - int getLoggerCount(); - - String[] getLoggerNames(); - - boolean setLoggerLevel(String loggerName, String levelName); - - String getLoggerLevel(String loggerName); -} diff --git a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java index ffc29298adb..921beb59e55 100644 --- a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java +++ b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java @@ -19,7 +19,6 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.stream.Stream; -import javax.management.JMX; import javax.management.MBeanAttributeInfo; import javax.management.MBeanInfo; import javax.management.MBeanServer; @@ -52,28 +51,44 @@ public class JMXTest MBeanAttributeInfo attr = Stream.of(attributeInfos).filter((a) -> a.getName().equals("LoggerNames")).findFirst().orElseThrow(); assertThat("attr", attr.getDescription(), is("List of Registered Loggers by Name.")); - JettyLoggerFactoryMBean mbean = JMX.newMBeanProxy(mbeanServer, objectName, JettyLoggerFactoryMBean.class); + // 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 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 = "getLoggerName"; + 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 = "setLoggerName"; + 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()); } } From 81c2c233db475508fe31ba104a80b09d342ca472 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 22 Jan 2021 10:31:16 -0600 Subject: [PATCH 3/3] Issue #5872 - DynamicMBean for JettyLoggerFactory + Fixing typo in operation names Signed-off-by: Joakim Erdfelt --- .../src/test/java/org/eclipse/jetty/logging/JMXTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java index 921beb59e55..ab950e95e5f 100644 --- a/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java +++ b/jetty-slf4j-impl/src/test/java/org/eclipse/jetty/logging/JMXTest.java @@ -77,14 +77,14 @@ public class JMXTest // Setting the parent level should propagate to the children. parent.setLevel(JettyLevel.DEBUG); - operationName = "getLoggerName"; + 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. - operationName = "setLoggerName"; + 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);