dsmiley commented on code in PR #3444:
URL: https://github.com/apache/solr/pull/3444#discussion_r2238139799
##########
solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java:
##########
@@ -276,10 +272,21 @@ private void testQueryAgainstDownReplica() throws
Exception {
client.query(new SolrQuery("*:*"));
count++;
- long c = cnt.getCount();
+ double c = getSelectRequestCount(leaderCore);
- assertEquals("Query wasn't served by leader", count, c);
+ assertEquals("Query wasn't served by leader", count, c, 0.0);
}
}
}
+
+ private Double getSelectRequestCount(SolrCore core) {
Review Comment:
why a `Double` instead of `long` ?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.client.solrj;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.Http2SolrClient;
+import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+
+public final class SolrJMetricTestUtils {
+
+ public static double getPrometheusMetricValue(CloudSolrClient solrClient,
String metricName)
+ throws SolrServerException, IOException {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ NamedList<Object> resp = solrClient.request(req);
+ try (InputStream in = (InputStream) resp.get("stream")) {
+ String output = new String(in.readAllBytes(), StandardCharsets.UTF_8);
+ var line =
+ output
+ .lines()
+ .filter(l -> l.startsWith(metricName))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Metric not found: " +
metricName));
+
+ return Double.parseDouble(line.substring(line.lastIndexOf(" ") +
1).trim());
+ }
+ }
+
+ public static Double getNumCoreRequests(
+ String baseUrl, String collectionName, String category, String handler)
+ throws SolrServerException, IOException {
+
+ try (Http2SolrClient client = new
Http2SolrClient.Builder(baseUrl).build()) {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
Review Comment:
FYI try `SolrParams.of("wt", "prometheus")`
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.client.solrj;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.Http2SolrClient;
+import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+
+public final class SolrJMetricTestUtils {
+
+ public static double getPrometheusMetricValue(CloudSolrClient solrClient,
String metricName)
+ throws SolrServerException, IOException {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ NamedList<Object> resp = solrClient.request(req);
+ try (InputStream in = (InputStream) resp.get("stream")) {
+ String output = new String(in.readAllBytes(), StandardCharsets.UTF_8);
+ var line =
+ output
+ .lines()
+ .filter(l -> l.startsWith(metricName))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Metric not found: " +
metricName));
+
+ return Double.parseDouble(line.substring(line.lastIndexOf(" ") +
1).trim());
+ }
+ }
+
+ public static Double getNumCoreRequests(
+ String baseUrl, String collectionName, String category, String handler)
+ throws SolrServerException, IOException {
+
+ try (Http2SolrClient client = new
Http2SolrClient.Builder(baseUrl).build()) {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ NamedList<Object> resp = client.request(req);
+ try (InputStream in = (InputStream) resp.get("stream")) {
+ String output = new String(in.readAllBytes(), StandardCharsets.UTF_8);
+ String metricName;
+
+ metricName = "solr_core_requests_total";
+
+ var line =
+ output
+ .lines()
+ .filter(
+ l ->
+ l.contains(metricName)
+ && l.contains(String.format("category=\"%s\"",
category))
+ && l.contains(String.format("collection=\"%s\"",
collectionName))
+ && l.contains(String.format("handler=\"%s\"",
handler)))
+ .findFirst();
+
+ return line.map(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")
+ 1).trim()))
+ .orElse(0.0);
+ }
+ }
+ }
+
+ public static Double getNumNodeRequestErrors(String baseUrl, String
category, String handler)
+ throws SolrServerException, IOException {
+
+ try (Http2SolrClient client = new
Http2SolrClient.Builder(baseUrl).build()) {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ NamedList<Object> resp = client.request(req);
+ try (InputStream in = (InputStream) resp.get("stream")) {
+ String output = new String(in.readAllBytes(), StandardCharsets.UTF_8);
+ String metricName;
+ metricName = "solr_node_requests_errors_total";
+ var line =
+ output
+ .lines()
+ .filter(
+ l ->
+ l.contains(metricName)
+ && l.contains(String.format("category=\"%s\"",
category))
+ && l.contains(String.format("handler=\"%s\"",
handler)))
+ .toList();
+
+ // Sum both client and server errors
+ return line.stream()
+ .map(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ") +
1).trim()))
Review Comment:
see `Stream.mapToDouble` and `Stream.mapToLong`
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.client.solrj;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.Http2SolrClient;
+import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+
+public final class SolrJMetricTestUtils {
+
+ public static double getPrometheusMetricValue(CloudSolrClient solrClient,
String metricName)
+ throws SolrServerException, IOException {
+ var req =
+ new GenericSolrRequest(
+ SolrRequest.METHOD.GET,
+ "/admin/metrics",
+ SolrRequest.SolrRequestType.ADMIN,
+ new ModifiableSolrParams().set("wt", "prometheus"));
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ NamedList<Object> resp = solrClient.request(req);
+ try (InputStream in = (InputStream) resp.get("stream")) {
+ String output = new String(in.readAllBytes(), StandardCharsets.UTF_8);
+ var line =
+ output
+ .lines()
+ .filter(l -> l.startsWith(metricName))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Metric not found: " +
metricName));
+
+ return Double.parseDouble(line.substring(line.lastIndexOf(" ") +
1).trim());
+ }
+ }
+
+ public static Double getNumCoreRequests(
Review Comment:
why not `long`?
##########
solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java:
##########
Review Comment:
nice to finally see just otel and not also dropwizard simultaneously
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java:
##########
@@ -60,21 +58,7 @@ public void testRetry() throws Exception {
solrClient.add(collectionName, new SolrInputDocument("id", "1"));
- ModifiableSolrParams params = new ModifiableSolrParams();
- String updateRequestCountKey =
-
"solr.core.testRetry.shard1.replica_n1:UPDATE./update.requestTimes:count";
- params.set("key", updateRequestCountKey);
- params.set("indent", "true");
- params.set(CommonParams.WT, "xml");
-
- var metricsRequest =
- new GenericSolrRequest(METHOD.GET, "/admin/metrics",
SolrRequestType.ADMIN, params);
- metricsRequest.setRequiresCollection(false);
-
- NamedList<Object> namedList = solrClient.request(metricsRequest);
- System.out.println(namedList);
- NamedList<?> metrics = (NamedList<?>) namedList.get("metrics");
- assertEquals(1L, metrics.get(updateRequestCountKey));
+ assertEquals(1.0, getPrometheusMetricValue(solrClient,
prometheusMetric), 0.0);
Review Comment:
nice utility method
--
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]