mlbiscoc commented on code in PR #3499:
URL: https://github.com/apache/solr/pull/3499#discussion_r2304574241
##########
solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java:
##########
@@ -17,724 +17,242 @@
package org.apache.solr.handler.admin;
-import com.codahale.metrics.Counter;
import io.opentelemetry.api.common.Attributes;
-import io.prometheus.metrics.model.snapshots.CounterSnapshot;
-import io.prometheus.metrics.model.snapshots.GaugeSnapshot;
-import io.prometheus.metrics.model.snapshots.Labels;
-import io.prometheus.metrics.model.snapshots.MetricSnapshot;
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
-import java.util.Arrays;
-import java.util.HashMap;
import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.common.MapWriter;
import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.core.PluginBag;
-import org.apache.solr.core.PluginInfo;
import org.apache.solr.handler.RequestHandlerBase;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.security.AuthorizationContext;
-import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
/** Test for {@link MetricsHandler} */
public class MetricsHandlerTest extends SolrTestCaseJ4 {
@BeforeClass
public static void beforeClass() throws Exception {
-
initCore("solrconfig-minimal.xml", "schema.xml");
h.getCoreContainer().waitForLoadingCoresToFinish(30000);
-
- // manually register & seed some metrics in solr.jvm and solr.jetty for
testing via handler
- // (use "solrtest_" prefix just in case the jvm or jetty adds a "foo"
metric at some point)
- Counter c = h.getCoreContainer().getMetricManager().counter(null,
"solr.jvm", "solrtest_foo");
- c.inc();
- c = h.getCoreContainer().getMetricManager().counter(null, "solr.jetty",
"solrtest_foo");
- c.inc(2);
- // test escapes
- c = h.getCoreContainer().getMetricManager().counter(null, "solr.jetty",
"solrtest_foo:bar");
- c.inc(3);
-
- h.getCoreContainer()
- .getMetricManager()
- .meter(null, "solr.jetty",
"org.eclipse.jetty.server.handler.DefaultHandler.2xx-responses");
- h.getCoreContainer()
- .getMetricManager()
- .counter(
- null, "solr.jetty",
"org.eclipse.jetty.server.handler.DefaultHandler.active-requests");
- h.getCoreContainer()
- .getMetricManager()
- .timer(null, "solr.jetty",
"org.eclipse.jetty.server.handler.DefaultHandler.dispatches");
- }
-
- @AfterClass
- public static void cleanupMetrics() {
- if (null != h) {
-
h.getCoreContainer().getMetricManager().registry("solr.jvm").remove("solrtest_foo");
-
h.getCoreContainer().getMetricManager().registry("solr.jetty").remove("solrtest_foo");
-
h.getCoreContainer().getMetricManager().registry("solr.jetty").remove("solrtest_foo:bar");
- }
}
- // NOCOMMIT: This test does a bunch of /admin/metrics calls with various
params, with various
- // filters and parameters. We have not migrated all the metrics to otel yet
or even created any
- // filters. Once that is done, we should revisit this test and assert the
prometheus response.
@Test
- @BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";)
- public void test() throws Exception {
+ public void testMetricNamesFiltering() throws Exception {
+ String expectedRequestsMetricName = "solr_core_requests";
MetricsHandler handler = new MetricsHandler(h.getCoreContainer());
+ assertQ(req("*:*"), "//result[@numFound='0']");
+
SolrQueryResponse resp = new SolrQueryResponse();
handler.handleRequestBody(
req(
CommonParams.QT,
"/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json"),
- resp);
- NamedList<?> values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- NamedList<?> nl = (NamedList<?>) values.get("solr.core.collection1");
- assertNotNull(nl);
- Object o = nl.get("SEARCHER.new.errors");
- assertNotNull(o); // counter type
- assertTrue(o instanceof MapWriter);
- // response wasn't serialized, so we get here whatever MetricUtils
produced instead of NamedList
- assertNotNull(((MapWriter) o)._get("count", null));
- assertEquals(0L, ((MapWriter) nl.get("SEARCHER.new.errors"))._get("count",
null));
- assertNotNull(nl.get("INDEX.segments")); // int gauge
- assertTrue((int) ((MapWriter) nl.get("INDEX.segments"))._get("value",
null) >= 0);
- assertNotNull(nl.get("INDEX.sizeInBytes")); // long gauge
- assertTrue((long) ((MapWriter) nl.get("INDEX.sizeInBytes"))._get("value",
null) >= 0);
- nl = (NamedList<?>) values.get("solr.node");
- assertNotNull(nl.get("CONTAINER.cores.loaded")); // int gauge
- assertEquals(1, ((MapWriter)
nl.get("CONTAINER.cores.loaded"))._get("value", null));
- assertNotNull(nl.get("ADMIN./admin/authorization.clientErrors")); // timer
type
- Map<String, Object> map = new HashMap<>();
- ((MapWriter) nl.get("ADMIN./admin/authorization.clientErrors")).toMap(map);
- assertEquals(5, map.size());
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "group",
- "jvm,jetty"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(2, values.size());
- assertNotNull(values.get("solr.jetty"));
- assertNotNull(values.get("solr.jvm"));
-
- resp = new SolrQueryResponse();
- // "collection" works too, because it's a prefix for "collection1"
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "registry",
- "solr.core.collection,solr.jvm"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(2, values.size());
- assertNotNull(values.get("solr.core.collection1"));
- assertNotNull(values.get("solr.jvm"));
-
- resp = new SolrQueryResponse();
- // "collection" works too, because it's a prefix for "collection1"
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "registry",
- "solr.core.collection",
- "registry",
- "solr.jvm"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(2, values.size());
- assertNotNull(values.get("solr.core.collection1"));
- assertNotNull(values.get("solr.jvm"));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "group",
- "jvm,jetty"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(2, values.size());
- assertNotNull(values.get("solr.jetty"));
- assertNotNull(values.get("solr.jvm"));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
CommonParams.WT,
- "json",
- "group",
- "jvm",
- "group",
- "jetty"),
+ MetricsHandler.PROMETHEUS_METRICS_WT,
+ MetricsHandler.METRIC_NAME_PARAM,
+ expectedRequestsMetricName),
resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(2, values.size());
- assertNotNull(values.get("solr.jetty"));
- assertNotNull(values.get("solr.jvm"));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "group",
- "node",
- "type",
- "counter"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(1, values.size());
- values = (NamedList<?>) values.get("solr.node");
- assertNotNull(values);
- assertNull(values.get("ADMIN./admin/authorization.errors")); // this is a
timer node
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "prefix",
- "CONTAINER.cores,CONTAINER.threadPool"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(1, values.size());
- assertNotNull(values.get("solr.node"));
- values = (NamedList<?>) values.get("solr.node");
- assertEquals(27, values.size());
- assertNotNull(values.get("CONTAINER.cores.lazy")); // this is a gauge node
-
assertNotNull(values.get("CONTAINER.threadPool.coreContainerWorkExecutor.completed"));
-
assertNotNull(values.get("CONTAINER.threadPool.coreLoadExecutor.completed"));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "prefix",
- "CONTAINER.cores",
- "regex",
- "C.*thread.*completed"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertNotNull(values.get("solr.node"));
- values = (NamedList<?>) values.get("solr.node");
- assertEquals(7, values.size());
-
assertNotNull(values.get("CONTAINER.threadPool.coreContainerWorkExecutor.completed"));
-
assertNotNull(values.get("CONTAINER.threadPool.coreLoadExecutor.completed"));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- CommonParams.WT,
- "json",
- "prefix",
- "CACHE.core.fieldCache",
- "property",
- "entries_count",
- MetricsHandler.COMPACT_PARAM,
- "true"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertNotNull(values.get("solr.core.collection1"));
- values = (NamedList<?>) values.get("solr.core.collection1");
- assertEquals(1, values.size());
- MapWriter writer = (MapWriter) values.get("CACHE.core.fieldCache");
- assertNotNull(writer);
- assertNotNull(writer._get("entries_count", null));
-
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "group",
- "jvm",
- "prefix",
- "CONTAINER.cores"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- assertEquals(0, values.size());
+ var metrics = resp.getValues().get("metrics");
+ MetricSnapshots snapshots = (MetricSnapshots) metrics;
+ assertEquals(1, snapshots.size());
+ assertEquals(expectedRequestsMetricName,
snapshots.get(0).getMetadata().getPrometheusName());
- resp = new SolrQueryResponse();
- handler.handleRequestBody(
- req(
- CommonParams.QT,
- "/admin/metrics",
- MetricsHandler.COMPACT_PARAM,
- "false",
- CommonParams.WT,
- "json",
- "group",
- "node",
- "type",
- "timer",
- "prefix",
- "CONTAINER.cores"),
- resp);
- values = resp.getValues();
- assertNotNull(values.get("metrics"));
- SimpleOrderedMap<?> map1 = (SimpleOrderedMap<?>) values.get("metrics");
- assertEquals(0, map1.size());
handler.close();
}
@Test
- public void testPropertyFilter() throws Exception {
- assertQ(req("*:*"), "//result[@numFound='0']");
-
+ public void testMultipleMetricNamesFiltering() throws Exception {
+ String expectedRequestsMetricName = "solr_core_requests";
+ String expectedSearcherMetricName = "solr_core_searcher_new";
MetricsHandler handler = new MetricsHandler(h.getCoreContainer());
+ assertQ(req("*:*"), "//result[@numFound='0']");
+
SolrQueryResponse resp = new SolrQueryResponse();
handler.handleRequestBody(
req(
CommonParams.QT,
"/admin/metrics",
CommonParams.WT,
- "json",
- MetricsHandler.COMPACT_PARAM,
- "true",
- "group",
- "core",
- "prefix",
- "CACHE.searcher"),
+ MetricsHandler.PROMETHEUS_METRICS_WT,
+ MetricsHandler.METRIC_NAME_PARAM,
+ expectedRequestsMetricName + "," + expectedSearcherMetricName),
resp);
- NamedList<?> values = resp.getValues();
- assertNotNull(values.get("metrics"));
- values = (NamedList<?>) values.get("metrics");
- NamedList<?> nl = (NamedList<?>) values.get("solr.core.collection1");
- assertNotNull(nl);
- assertTrue(nl.size() > 0);
- nl.forEach(
- (k, v) -> {
- assertTrue(v instanceof MapWriter);
- Map<String, Object> map = new HashMap<>();
- ((MapWriter) v).toMap(map);
- assertTrue(map.size() > 2);
- });
+ var metrics = (MetricSnapshots) resp.getValues().get("metrics");
+ assertEquals(2, metrics.size());
+ assertEquals(expectedRequestsMetricName,
metrics.get(0).getMetadata().getPrometheusName());
+ assertEquals(expectedSearcherMetricName,
metrics.get(1).getMetadata().getPrometheusName());
Review Comment:
I don't think this is actually true and I'm not going to bet on it. Changed
up the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
