SOLR-17855: Cleanup Dropwizard metric code and tests

solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java

@@ -165,7 +165,7 @@ public SolrMetricsContext getSolrMetricsContext() {
   @Override
   public void initializeMetrics(SolrMetricsContext parentContext, Attributes attributes) {
     if (aggregateNodeLevelMetricsEnabled) {
-      //       NOCOMMIT: SOLR-17865
+      //      NOCOMMIT: SOLR-17865
       //      this.solrMetricsContext =
       //          new SolrDelegateRegistryMetricsContext(
       //              parentContext.getMetricManager(),

solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java

@@ -33,6 +33,7 @@
 import java.util.function.BiConsumer;
 import java.util.regex.Pattern;
 import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.CommonTestInjection;
 import org.apache.solr.common.util.StrUtils;
@@ -108,6 +109,18 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
           SolrException.ErrorCode.INVALID_STATE, "SolrMetricManager instance not initialized");
     }
 
+    SolrParams params = req.getParams();
+    String format = params.get(CommonParams.WT);
+
+    if (format == null) {
+      req.setParams(SolrParams.wrapDefaults(params, SolrParams.of("wt", "prometheus")));
+    } else if (!PROMETHEUS_METRICS_WT.equals(format) && !OPEN_METRICS_WT.equals(format)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Only Prometheus and OpenMetrics metric formats supported. Unsupported format requested: "
+              + format);
+    }
+
     if (cc != null && AdminHandlersProxy.maybeProxyToNodes(req, rsp, cc)) {
       return; // Request was proxied to other node
     }
@@ -125,10 +138,6 @@ private void handleRequest(SolrParams params, BiConsumer<String, Object> consume
       return;
     }
 
-    handlePrometheusRequest(params, consumer);
-  }
-
-  private void handlePrometheusRequest(SolrParams params, BiConsumer<String, Object> consumer) {
     Set<String> metricNames = readParamsAsSet(params, METRIC_NAME_PARAM);
     SortedMap<String, Set<String>> labelFilters = labelFilters(params);
 

solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java

@@ -18,11 +18,17 @@
 
 import static org.apache.solr.common.params.CommonParams.NAME;
 
+import java.beans.BeanInfo;
+import java.beans.IntrospectionException;
+import java.beans.Introspector;
+import java.beans.PropertyDescriptor;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.lang.management.ManagementFactory;
 import java.lang.management.OperatingSystemMXBean;
+import java.lang.management.PlatformManagedObject;
 import java.lang.management.RuntimeMXBean;
+import java.lang.reflect.Method;
 import java.net.InetAddress;
 import java.nio.file.Path;
 import java.text.DecimalFormat;
@@ -33,6 +39,9 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.apache.lucene.util.Version;
@@ -79,6 +88,13 @@ public class SystemInfoHandler extends RequestHandlerBase {
   private static final String REVERSE_DNS_OF_LOCALHOST_SYSPROP =
       "solr.admin.handler.systeminfo.dns.reverse.lookup.enabled";
 
+  /**
+   * Local cache for BeanInfo instances that are created to scan for system metrics. List of
+   * properties is not supposed to change for the JVM lifespan, so we can keep already create
+   * BeanInfo instance for future calls.
+   */
+  private static final ConcurrentMap<Class<?>, BeanInfo> beanInfos = new ConcurrentHashMap<>();
+
   // on some platforms, resolving canonical hostname can cause the thread
   // to block for several seconds if nameservices aren't available
   // so resolve this once per handler instance
@@ -97,6 +113,76 @@ public SystemInfoHandler(CoreContainer cc) {
     initHostname();
   }
 
+  /**
+   * Iterates over properties of the given MXBean and invokes the provided consumer with each
+   * property name and its current value.
+   *
+   * @param obj an instance of MXBean
+   * @param interfaces interfaces that it may implement. Each interface will be tried in turn, and
+   *     only if it exists and if it contains unique properties then they will be added as metrics.
+   * @param consumer consumer for each property name and value
+   * @param <T> formal type
+   */
+  public static <T extends PlatformManagedObject> void forEachGetterValue(
+      T obj, String[] interfaces, BiConsumer<String, Object> consumer) {
+    for (String clazz : interfaces) {
+      try {
+        final Class<? extends PlatformManagedObject> intf =
+            Class.forName(clazz).asSubclass(PlatformManagedObject.class);
+        forEachGetterValue(obj, intf, consumer);
+      } catch (ClassNotFoundException e) {
+        // ignore
+      }
+    }
+  }
+
+  /**
+   * Iterates over properties of the given MXBean and invokes the provided consumer with each
+   * property name and its current value.
+   *
+   * @param obj an instance of MXBean
+   * @param intf MXBean interface, one of {@link PlatformManagedObject}-s
+   * @param consumer consumer for each property name and value
+   * @param <T> formal type
+   */
+  public static <T extends PlatformManagedObject> void forEachGetterValue(
+      T obj, Class<? extends T> intf, BiConsumer<String, Object> consumer) {
+    if (intf.isInstance(obj)) {
+      BeanInfo beanInfo =
+          beanInfos.computeIfAbsent(
+              intf,
+              clazz -> {
+                try {
+                  return Introspector.getBeanInfo(
+                      clazz, clazz.getSuperclass(), Introspector.IGNORE_ALL_BEANINFO);
+
+                } catch (IntrospectionException e) {
+                  log.warn("Unable to fetch properties of MXBean {}", obj.getClass().getName());
+                  return null;
+                }
+              });
+
+      // if BeanInfo retrieval failed, return early
+      if (beanInfo == null) {
+        return;
+      }
+      for (final PropertyDescriptor desc : beanInfo.getPropertyDescriptors()) {
+        try {
+          Method readMethod = desc.getReadMethod();
+          if (readMethod == null) {
+            continue; // skip properties without a read method
+          }
+
+          final String name = desc.getName();
+          Object value = readMethod.invoke(obj);
+          consumer.accept(name, value);
+        } catch (Exception e) {
+          // didn't work, skip it...
+        }
+      }
+    }
+  }
+
   private void initHostname() {
     if (!EnvUtils.getPropertyAsBool(REVERSE_DNS_OF_LOCALHOST_SYSPROP, true)) {
       log.info(
@@ -219,7 +305,7 @@ public static SimpleOrderedMap<Object> getSystemInfo() {
 
     // add remaining ones dynamically using Java Beans API
     // also those from JVM implementation-specific classes
-    MetricUtils.addMXBeanMetrics(
+    forEachGetterValue(
         os,
         MetricUtils.OS_MXBEAN_CLASSES,
         (name, value) -> {

solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java

@@ -18,19 +18,10 @@
 
 import com.codahale.metrics.Snapshot;
 import com.codahale.metrics.Timer;
-import java.beans.BeanInfo;
-import java.beans.IntrospectionException;
-import java.beans.Introspector;
-import java.beans.PropertyDescriptor;
 import java.lang.invoke.MethodHandles;
 import java.lang.management.OperatingSystemMXBean;
-import java.lang.management.PlatformManagedObject;
-import java.lang.reflect.Method;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
-import java.util.function.BiConsumer;
 import java.util.function.Predicate;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.SolrInfoBean;
@@ -70,32 +61,6 @@ public class MetricUtils {
 
   public static final Predicate<CharSequence> ALL_PROPERTIES = (name) -> true;
 
-  /**
-   * Local cache for BeanInfo instances that are created to scan for system metrics. List of
-   * properties is not supposed to change for the JVM lifespan, so we can keep already create
-   * BeanInfo instance for future calls.
-   */
-  private static final ConcurrentMap<Class<?>, BeanInfo> beanInfos = new ConcurrentHashMap<>();
-
-  /**
-   * Converts a double representing nanoseconds to a double representing milliseconds.
-   *
-   * @param ns the amount of time in nanoseconds
-   * @return the amount of time in milliseconds
-   */
-  public static double nsToMs(double ns) {
-    return ns / TimeUnit.MILLISECONDS.toNanos(1);
-  }
-
-  // optionally convert ns to ms
-  static double nsToMs(boolean convert, double value) {
-    if (convert) {
-      return nsToMs(value);
-    } else {
-      return value;
-    }
-  }
-
   /**
    * Adds metrics from a Timer to a NamedList, using well-known back-compat names.
    *
@@ -116,49 +81,21 @@ public static void addMetrics(NamedList<Object> lst, Timer timer) {
   }
 
   /**
-   * Creates a set of metrics (gauges) that correspond to available bean properties for the provided
-   * MXBean.
+   * Converts a double representing nanoseconds to a double representing milliseconds.
    *
-   * @param obj an instance of MXBean
-   * @param intf MXBean interface, one of {@link PlatformManagedObject}-s
-   * @param consumer consumer for created names and metrics
-   * @param <T> formal type
+   * @param ns the amount of time in nanoseconds
+   * @return the amount of time in milliseconds
    */
-  public static <T extends PlatformManagedObject> void addMXBeanMetrics(
-      T obj, Class<? extends T> intf, BiConsumer<String, Object> consumer) {
-    if (intf.isInstance(obj)) {
-      BeanInfo beanInfo =
-          beanInfos.computeIfAbsent(
-              intf,
-              clazz -> {
-                try {
-                  return Introspector.getBeanInfo(
-                      clazz, clazz.getSuperclass(), Introspector.IGNORE_ALL_BEANINFO);
-
-                } catch (IntrospectionException e) {
-                  log.warn("Unable to fetch properties of MXBean {}", obj.getClass().getName());
-                  return null;
-                }
-              });
-
-      // if BeanInfo retrieval failed, return early
-      if (beanInfo == null) {
-        return;
-      }
-      for (final PropertyDescriptor desc : beanInfo.getPropertyDescriptors()) {
-        try {
-          Method readMethod = desc.getReadMethod();
-          if (readMethod == null) {
-            continue; // skip properties without a read method
-          }
+  public static double nsToMs(double ns) {
+    return ns / TimeUnit.MILLISECONDS.toNanos(1);
+  }
 
-          final String name = desc.getName();
-          Object value = readMethod.invoke(obj);
-          consumer.accept(name, value);
-        } catch (Exception e) {
-          // didn't work, skip it...
-        }
-      }
+  // optionally convert ns to ms
+  static double nsToMs(boolean convert, double value) {
+    if (convert) {
+      return nsToMs(value);
+    } else {
+      return value;
     }
   }
 
@@ -182,27 +119,4 @@ public static ExecutorService instrumentedExecutorService(
       String name) {
     return new OtelInstrumentedExecutorService(delegate, ctx, category, name);
   }
-
-  /**
-   * Creates a set of metrics (gauges) that correspond to available bean properties for the provided
-   * MXBean.
-   *
-   * @param obj an instance of MXBean
-   * @param interfaces interfaces that it may implement. Each interface will be tried in turn, and
-   *     only if it exists and if it contains unique properties then they will be added as metrics.
-   * @param consumer consumer for created names and metrics
-   * @param <T> formal type
-   */
-  public static <T extends PlatformManagedObject> void addMXBeanMetrics(
-      T obj, String[] interfaces, BiConsumer<String, Object> consumer) {
-    for (String clazz : interfaces) {
-      try {
-        final Class<? extends PlatformManagedObject> intf =
-            Class.forName(clazz).asSubclass(PlatformManagedObject.class);
-        MetricUtils.addMXBeanMetrics(obj, intf, consumer);
-      } catch (ClassNotFoundException e) {
-        // ignore
-      }
-    }
-  }
 }

solr/core/src/test/org/apache/solr/handler/admin/AdminHandlersProxyTest.java

@@ -71,22 +71,6 @@ public void proxySystemInfoHandlerAllNodes() throws IOException, SolrServerExcep
     assertEquals(nl.getName(2), ((NamedList) nl.get(nl.getName(2))).get("node"));
   }
 
-  // NOCOMMIT: The nodes view might be broken because of this
-  @Test
-  @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-8207")
-  public void proxyMetricsHandlerAllNodes() throws IOException, SolrServerException {
-    MapSolrParams params = new MapSolrParams(Collections.singletonMap("nodes", "all"));
-    GenericSolrRequest req =
-        new GenericSolrRequest(
-            SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
-    SimpleSolrResponse rsp = req.process(solrClient, null);
-    NamedList<Object> nl = rsp.getResponse();
-    assertEquals(3, nl.size());
-    assertTrue(nl.getName(1).endsWith("_solr"));
-    assertTrue(nl.getName(2).endsWith("_solr"));
-    assertNotNull(((NamedList) nl.get(nl.getName(1))).get("metrics"));
-  }
-
   @Test(expected = SolrException.class)
   public void proxySystemInfoHandlerNonExistingNode() throws IOException, SolrServerException {
     MapSolrParams params =

solr/core/src/test/org/apache/solr/handler/admin/SystemInfoHandlerTest.java

@@ -21,11 +21,9 @@
 import java.util.Arrays;
 import org.apache.solr.SolrTestCase;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.util.stats.MetricUtils;
 
 public class SystemInfoHandlerTest extends SolrTestCase {
 
-  // NOCOMMIT: This is broken
   public void testMagickGetter() {
 
     OperatingSystemMXBean os = ManagementFactory.getOperatingSystemMXBean();
@@ -38,7 +36,7 @@ public void testMagickGetter() {
 
     // make another using MetricUtils.addMXBeanMetrics()
     SimpleOrderedMap<Object> info2 = new SimpleOrderedMap<>();
-    MetricUtils.addMXBeanMetrics(os, OperatingSystemMXBean.class, info2::add);
+    SystemInfoHandler.forEachGetterValue(os, OperatingSystemMXBean.class, info2::add);
 
     // make sure they got the same thing
     for (String p : Arrays.asList("name", "version", "arch")) {