dsmiley commented on code in PR #3499:
URL: https://github.com/apache/solr/pull/3499#discussion_r2302105538
##########
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:
is the order sorted, and thus we can know the order without it changing?
##########
solr/core/src/test/org/apache/solr/metrics/otel/FilterablePrometheusMetricReaderTest.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.metrics.otel;
+
+import io.prometheus.metrics.model.snapshots.Labels;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Test;
+
+/**
+ * Test class for FilterablePrometheusMetricReader which focuses on
requiredLabelsFilter method
+ * which filters metric data points based on label requirements.
+ */
+public class FilterablePrometheusMetricReaderTest extends SolrTestCaseJ4 {
+
+ Labels actualLabels = Labels.of("key1", "value1", "key2", "value2", "key3",
"value3");
+
+ @Test
+ public void testFilterMatchingLabel() {
+ SortedMap<String, Set<String>> requiredLabels =
+ new TreeMap<>() {
+ {
+ put("key1", Set.of("value1"));
+ }
Review Comment:
Please don't use this style (subclass with anonymous constructor). Just do
`new TreeMap<>(Map.of(...))`
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
