From 2b92515455761d1f46e927434e215fe54fd797f1 Mon Sep 17 00:00:00 2001 From: Andrew Galante Date: Wed, 27 Nov 2024 16:14:00 -0800 Subject: [PATCH 1/3] Attempt to dynamically load ManagementFactory class; Fail gracefully if not found. This can be the case on Android. --- .../kafka/common/utils/AppInfoParser.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java index f9ebd82ea11cf..3779d53e119fd 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java @@ -25,10 +25,9 @@ import org.slf4j.LoggerFactory; import java.io.InputStream; -import java.lang.management.ManagementFactory; +import java.lang.reflect.Method; import java.util.Properties; -import javax.management.JMException; import javax.management.MBeanServer; import javax.management.ObjectName; @@ -36,6 +35,7 @@ public class AppInfoParser { private static final Logger log = LoggerFactory.getLogger(AppInfoParser.class); private static final String VERSION; private static final String COMMIT_ID; + private static final Method GET_MBEAN_SERVER; protected static final String DEFAULT_VALUE = "unknown"; @@ -48,6 +48,15 @@ public class AppInfoParser { } VERSION = props.getProperty("version", DEFAULT_VALUE).trim(); COMMIT_ID = props.getProperty("commitId", DEFAULT_VALUE).trim(); + + Method getMBeanServer; + try { + Class managementFactory = Class.forName("java.lang.management.ManagementFactory"); + getMBeanServer = managementFactory.getMethod("getPlatformMBeanServer"); + } catch (ReflectiveOperationException e) { + getMBeanServer = null; + } + GET_MBEAN_SERVER = getMBeanServer; } public static String getVersion() { @@ -59,9 +68,11 @@ public static String getCommitId() { } public static synchronized void registerAppInfo(String prefix, String id, Metrics metrics, long nowMs) { + if (GET_MBEAN_SERVER == null) + return; try { ObjectName name = new ObjectName(prefix + ":type=app-info,id=" + Sanitizer.jmxSanitize(id)); - MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + MBeanServer server = (MBeanServer) GET_MBEAN_SERVER.invoke(null); if (server.isRegistered(name)) { log.info("The mbean of App info: [{}], id: [{}] already exists, so skipping a new mbean creation.", prefix, id); return; @@ -70,20 +81,22 @@ public static synchronized void registerAppInfo(String prefix, String id, Metric server.registerMBean(mBean, name); registerMetrics(metrics, mBean); // prefix will be added later by JmxReporter - } catch (JMException e) { + } catch (Throwable e) { log.warn("Error registering AppInfo mbean", e); } } public static synchronized void unregisterAppInfo(String prefix, String id, Metrics metrics) { - MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + if (GET_MBEAN_SERVER == null) + return; try { + MBeanServer server = (MBeanServer) GET_MBEAN_SERVER.invoke(null); ObjectName name = new ObjectName(prefix + ":type=app-info,id=" + Sanitizer.jmxSanitize(id)); if (server.isRegistered(name)) server.unregisterMBean(name); unregisterMetrics(metrics); - } catch (JMException e) { + } catch (Throwable e) { log.warn("Error unregistering AppInfo mbean", e); } finally { log.info("App info {} for {} unregistered", prefix, id); From b8da06db3c1f415620def037cb98062f3975c19c Mon Sep 17 00:00:00 2001 From: Andrew Galante Date: Wed, 27 Nov 2024 16:25:25 -0800 Subject: [PATCH 2/3] Restore caught exception types --- .../java/org/apache/kafka/common/utils/AppInfoParser.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java index 3779d53e119fd..aa8f005d440b1 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java @@ -28,6 +28,7 @@ import java.lang.reflect.Method; import java.util.Properties; +import javax.management.JMException; import javax.management.MBeanServer; import javax.management.ObjectName; @@ -81,7 +82,7 @@ public static synchronized void registerAppInfo(String prefix, String id, Metric server.registerMBean(mBean, name); registerMetrics(metrics, mBean); // prefix will be added later by JmxReporter - } catch (Throwable e) { + } catch (JMException | ReflectiveOperationException e) { log.warn("Error registering AppInfo mbean", e); } } @@ -96,7 +97,7 @@ public static synchronized void unregisterAppInfo(String prefix, String id, Metr server.unregisterMBean(name); unregisterMetrics(metrics); - } catch (Throwable e) { + } catch (JMException | ReflectiveOperationException e) { log.warn("Error unregistering AppInfo mbean", e); } finally { log.info("App info {} for {} unregistered", prefix, id); From d699271ce403e634493bbddc7576d82d6d1e7baf Mon Sep 17 00:00:00 2001 From: Andrew Galante Date: Mon, 19 May 2025 10:38:43 -0700 Subject: [PATCH 3/3] Encapsulate getMBeanServer for testing --- .../kafka/common/utils/AppInfoParser.java | 31 +++++++++++-------- .../kafka/common/utils/AppInfoParserTest.java | 18 +++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java index aa8f005d440b1..1dbacc35aeca6 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java @@ -25,7 +25,6 @@ import org.slf4j.LoggerFactory; import java.io.InputStream; -import java.lang.reflect.Method; import java.util.Properties; import javax.management.JMException; @@ -36,7 +35,6 @@ public class AppInfoParser { private static final Logger log = LoggerFactory.getLogger(AppInfoParser.class); private static final String VERSION; private static final String COMMIT_ID; - private static final Method GET_MBEAN_SERVER; protected static final String DEFAULT_VALUE = "unknown"; @@ -49,15 +47,20 @@ public class AppInfoParser { } VERSION = props.getProperty("version", DEFAULT_VALUE).trim(); COMMIT_ID = props.getProperty("commitId", DEFAULT_VALUE).trim(); + } - Method getMBeanServer; + static MBeanServer getPlatformMBeanServer(ClassLoader loader) { try { - Class managementFactory = Class.forName("java.lang.management.ManagementFactory"); - getMBeanServer = managementFactory.getMethod("getPlatformMBeanServer"); + // Not all platforms (*cough*Android*cough*) have MBeans, query using reflection + Class managementFactory = Class.forName("java.lang.management.ManagementFactory", true, loader); + return (MBeanServer) managementFactory.getMethod("getPlatformMBeanServer").invoke(null); } catch (ReflectiveOperationException e) { - getMBeanServer = null; + return null; } - GET_MBEAN_SERVER = getMBeanServer; + } + + static MBeanServer getPlatformMBeanServer() { + return getPlatformMBeanServer(AppInfoParser.class.getClassLoader()); } public static String getVersion() { @@ -69,11 +72,12 @@ public static String getCommitId() { } public static synchronized void registerAppInfo(String prefix, String id, Metrics metrics, long nowMs) { - if (GET_MBEAN_SERVER == null) + MBeanServer server = getPlatformMBeanServer(); + if (server == null) { return; + } try { ObjectName name = new ObjectName(prefix + ":type=app-info,id=" + Sanitizer.jmxSanitize(id)); - MBeanServer server = (MBeanServer) GET_MBEAN_SERVER.invoke(null); if (server.isRegistered(name)) { log.info("The mbean of App info: [{}], id: [{}] already exists, so skipping a new mbean creation.", prefix, id); return; @@ -82,22 +86,23 @@ public static synchronized void registerAppInfo(String prefix, String id, Metric server.registerMBean(mBean, name); registerMetrics(metrics, mBean); // prefix will be added later by JmxReporter - } catch (JMException | ReflectiveOperationException e) { + } catch (JMException e) { log.warn("Error registering AppInfo mbean", e); } } public static synchronized void unregisterAppInfo(String prefix, String id, Metrics metrics) { - if (GET_MBEAN_SERVER == null) + MBeanServer server = getPlatformMBeanServer(); + if (server == null) { return; + } try { - MBeanServer server = (MBeanServer) GET_MBEAN_SERVER.invoke(null); ObjectName name = new ObjectName(prefix + ":type=app-info,id=" + Sanitizer.jmxSanitize(id)); if (server.isRegistered(name)) server.unregisterMBean(name); unregisterMetrics(metrics); - } catch (JMException | ReflectiveOperationException e) { + } catch (JMException e) { log.warn("Error unregistering AppInfo mbean", e); } finally { log.info("App info {} for {} unregistered", prefix, id); diff --git a/clients/src/test/java/org/apache/kafka/common/utils/AppInfoParserTest.java b/clients/src/test/java/org/apache/kafka/common/utils/AppInfoParserTest.java index aac13f299fe2d..b72e5441788bd 100644 --- a/clients/src/test/java/org/apache/kafka/common/utils/AppInfoParserTest.java +++ b/clients/src/test/java/org/apache/kafka/common/utils/AppInfoParserTest.java @@ -31,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -55,6 +56,23 @@ public void tearDown() { metrics.close(); } + @Test + public void testGetPlatformMBeanServer() { + assertNotNull(AppInfoParser.getPlatformMBeanServer(AppInfoParser.class.getClassLoader())); + } + + @Test + public void testFailGetPlatformMBeanServer() { + ClassLoader nullLoader = new ClassLoader(null) { + @Override + public Class loadClass(String name) throws ClassNotFoundException { + throw new ClassNotFoundException(name); + } + }; + + assertNull(AppInfoParser.getPlatformMBeanServer(nullLoader)); + } + @Test public void testRegisterAppInfoRegistersMetrics() throws JMException { registerAppInfo();