From 6ea54281fb46a8d0da6b21a383d79128ae08499d Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 4 Jul 2024 22:41:24 +0200 Subject: [PATCH] review --- .../util/JeeStatisticsAttributes.java | 65 ++++++++++++------- .../jmxfetch/SimpleTestJavaAppMBean.java | 8 +++ 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/util/JeeStatisticsAttributes.java b/src/main/java/org/datadog/jmxfetch/util/JeeStatisticsAttributes.java index d5741432..e3338bbc 100644 --- a/src/main/java/org/datadog/jmxfetch/util/JeeStatisticsAttributes.java +++ b/src/main/java/org/datadog/jmxfetch/util/JeeStatisticsAttributes.java @@ -5,9 +5,9 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; +import java.lang.ref.SoftReference; import java.security.AccessController; import java.security.PrivilegedAction; -import java.sql.Ref; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -41,8 +41,8 @@ public class JeeStatisticsAttributes { Arrays.asList("upperBound", "lowerBound", "highWaterMark", "lowWaterMark", "current")); private static final Logger LOGGER = LoggerFactory.getLogger(JeeStatisticsAttributes.class); - private static final WeakHashMap REFLECTION_CACHE = - new WeakHashMap<>(); + private static final WeakHashMap> + REFLECTION_CACHE = new WeakHashMap<>(); private static class ReflectionHolder { @@ -60,22 +60,39 @@ private static class ReflectionHolder { ReflectionHolder(final ClassLoader classLoader) { classStat = maybeLookupClass("javax.management.j2ee.statistics.Stats", classLoader); - mhStatGetStatisticNames = maybeFindMethodHandleFor(classStat, "getStatisticNames"); - mhStatGetStatistic = maybeFindMethodHandleFor(classStat, "getStatistic", String.class); - classStatistic = maybeLookupClass("javax.management.j2ee.statistics.Statistic", - classLoader); - classCountStatistic = - maybeLookupClass("javax.management.j2ee.statistics.CountStatistic", classLoader); - classTimeStatistic = - maybeLookupClass("javax.management.j2ee.statistics.TimeStatistic", classLoader); - classRangeStatistic = - maybeLookupClass("javax.management.j2ee.statistics.RangeStatistic", classLoader); - classBoundaryStatistic = - maybeLookupClass("javax.management.j2ee.statistics.BoundaryStatistic", classLoader); - classBoundedRangeStatistic = - maybeLookupClass("javax.management.j2ee.statistics.BoundedRangeStatistic", + if (classStat != null) { + mhStatGetStatisticNames = maybeFindMethodHandleFor(classStat, "getStatisticNames"); + mhStatGetStatistic = maybeFindMethodHandleFor(classStat, "getStatistic", + String.class); + classStatistic = maybeLookupClass("javax.management.j2ee.statistics.Statistic", classLoader); - methodCache = buildMethodCache(); + classCountStatistic = + maybeLookupClass("javax.management.j2ee.statistics.CountStatistic", + classLoader); + classTimeStatistic = + maybeLookupClass("javax.management.j2ee.statistics.TimeStatistic", + classLoader); + classRangeStatistic = + maybeLookupClass("javax.management.j2ee.statistics.RangeStatistic", + classLoader); + classBoundaryStatistic = + maybeLookupClass("javax.management.j2ee.statistics.BoundaryStatistic", + classLoader); + classBoundedRangeStatistic = + maybeLookupClass("javax.management.j2ee.statistics.BoundedRangeStatistic", + classLoader); + methodCache = buildMethodCache(); + } else { + mhStatGetStatisticNames = null; + mhStatGetStatistic = null; + classStatistic = null; + classCountStatistic = null; + classTimeStatistic = null; + classRangeStatistic = null; + classBoundaryStatistic = null; + classBoundedRangeStatistic = null; + methodCache = new WeakHashMap<>(); + } } private Map, Map> buildMethodCache() { @@ -118,7 +135,7 @@ public MethodHandle run() { return MethodHandles.lookup() .unreflect(cls.getMethod(name, parameterTypes)); } catch (Throwable t) { - LOGGER.warn("Unable to find method {} for class {}: {}", name, cls, + LOGGER.debug("Unable to find method {} for class {}: {}", name, cls, t.getMessage()); } return null; @@ -160,12 +177,12 @@ private static String getterMethodName(String attribute) { private static ReflectionHolder getOrCreateReflectionHolder(final ClassLoader classLoader) { // no need to lock here. At worst, we'll do it more time if there is contention. - ReflectionHolder holder = REFLECTION_CACHE.get(classLoader); - if (holder != null) { - return holder; + SoftReference ref = REFLECTION_CACHE.get(classLoader); + if (ref != null && ref.get() != null) { + return ref.get(); } - holder = new ReflectionHolder(classLoader); - REFLECTION_CACHE.put(classLoader, holder); + final ReflectionHolder holder = new ReflectionHolder(classLoader); + REFLECTION_CACHE.put(classLoader, new SoftReference<>(holder)); return holder; } diff --git a/src/test/java/org/datadog/jmxfetch/SimpleTestJavaAppMBean.java b/src/test/java/org/datadog/jmxfetch/SimpleTestJavaAppMBean.java index f9d8ea1a..6c156bb1 100644 --- a/src/test/java/org/datadog/jmxfetch/SimpleTestJavaAppMBean.java +++ b/src/test/java/org/datadog/jmxfetch/SimpleTestJavaAppMBean.java @@ -44,14 +44,22 @@ public interface SimpleTestJavaAppMBean { Float getInstanceFloat(); TabularData getTabulardata(); + TabularDataSupport getTabularDataSupport(); CompositeData getNestedCompositeData(); + Statistic getJeeCounter(); + Statistic getJeeRange(); + Statistic getJeeTime(); + Statistic getJeeBoundary(); + Statistic getJeeBoundedRange(); + Statistic getJeeUnsupported(); + Stats getJeeStat(); }