dsmiley commented on code in PR #3713:
URL: https://github.com/apache/solr/pull/3713#discussion_r2392398757
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -17,157 +17,190 @@
package org.apache.solr.client.solrj.impl;
-import java.util.ArrayList;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import java.util.EnumSet;
-import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.solr.client.solrj.SolrRequest.METHOD;
+import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.client.solrj.response.SimpleSolrResponse;
import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.StrUtils;
-import org.apache.solr.common.util.Utils;
/**
* This class is responsible for fetching metrics and other attributes from a
given node in Solr
* cluster. This is a helper class that is used by {@link
SolrClientNodeStateProvider}
*/
-// NOCOMMIT: Need to removed hardcoded references to Dropwizard metrics for
OTEL conversion, and
-// probably change enum structure to be more compatible with OTEL naming
public class NodeValueFetcher {
// well known tags
public static final String NODE = "node";
public static final String PORT = "port";
public static final String HOST = "host";
public static final String CORES = "cores";
public static final String SYSPROP = "sysprop.";
- public static final Set<String> tags =
- Set.of(NODE, PORT, HOST, CORES, Tags.FREEDISK.tagName,
Tags.HEAPUSAGE.tagName);
+ public static final Set<String> tags = Set.of(NODE, PORT, HOST, CORES);
public static final Pattern hostAndPortPattern =
Pattern.compile("(?:https?://)?([^:]+):(\\d+)");
public static final String METRICS_PREFIX = "metrics:";
/** Various well known tags that can be fetched from a node */
- public enum Tags {
- FREEDISK(
- "freedisk", "solr.node", "CONTAINER.fs.usableSpace",
"solr.node/CONTAINER.fs.usableSpace"),
- TOTALDISK(
- "totaldisk", "solr.node", "CONTAINER.fs.totalSpace",
"solr.node/CONTAINER.fs.totalSpace"),
- CORES("cores", "solr.node", "CONTAINER.cores", null) {
+ public enum Metrics {
+ FREEDISK("freedisk", "solr_cores_filesystem_disk_space", "type",
"usable_space"),
+ TOTALDISK("totaldisk", "solr_cores_filesystem_disk_space", "type",
"total_space"),
+ CORES("cores", "solr_cores_loaded") {
@Override
- public Object extractResult(Object root) {
- NamedList<?> node = (NamedList<?>) Utils.getObjectByPath(root, false,
"solr.node");
- int count = 0;
- for (String leafCoreMetricName : new String[] {"lazy", "loaded",
"unloaded"}) {
- Number n = (Number) node.get("CONTAINER.cores." +
leafCoreMetricName);
- if (n != null) count += n.intValue();
+ public Object extractResult(NamedList<Object> root) {
+ Object metrics = root.get("stream");
+ if (metrics == null || metricName == null) return null;
+
+ try (InputStream in = (InputStream) metrics) {
+ String[] lines = parsePrometheusOutput(in);
+ int count = 0;
+
+ for (String line : lines) {
+ if (shouldSkipPrometheusLine(line) ||
!line.startsWith(metricName)) continue;
+ count += (int) extractPrometheusValue(line);
+ }
+ return count;
+ } catch (Exception e) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Unable to read prometheus
metrics output", e);
}
- return count;
}
},
- SYSLOADAVG("sysLoadAvg", "solr.jvm", "os.systemLoadAverage",
"solr.jvm/os.systemLoadAverage"),
- HEAPUSAGE("heapUsage", "solr.jvm", "memory.heap.usage",
"solr.jvm/memory.heap.usage");
- // the metrics group
- public final String group;
- // the metrics prefix
- public final String prefix;
+ SYSLOADAVG("sysLoadAvg", "jvm_system_cpu_utilization_ratio");
+
public final String tagName;
- // the json path in the response
- public final String path;
+ public final String metricName;
+ public final String labelKey;
+ public final String labelValue;
+
+ Metrics(String name, String metricName) {
+ this(name, metricName, null, null);
+ }
- Tags(String name, String group, String prefix, String path) {
- this.group = group;
- this.prefix = prefix;
+ Metrics(String name, String metricName, String labelKey, String
labelValue) {
this.tagName = name;
- this.path = path;
+ this.metricName = metricName;
+ this.labelKey = labelKey;
+ this.labelValue = labelValue;
}
- public Object extractResult(Object root) {
- Object v = Utils.getObjectByPath(root, true, path);
- return v == null ? null : convertVal(v);
+ public Object extractResult(NamedList<Object> root) {
+ return extractFromPrometheusResponse(root, metricName, labelKey,
labelValue);
}
- public Object convertVal(Object val) {
- if (val instanceof String) {
- return Double.valueOf((String) val);
- } else if (val instanceof Number num) {
- return num.doubleValue();
+ /**
+ * Extract metric value from Prometheus response, optionally filtering by
label. This
+ * consolidated method handles both labeled and unlabeled metrics.
+ */
+ private static Long extractFromPrometheusResponse(
+ NamedList<Object> root, String metricName, String labelKey, String
labelValue) {
+ Object metrics = root.get("stream");
- } else {
- throw new IllegalArgumentException("Unknown type : " + val);
+ if (metrics == null || metricName == null) {
+ return null;
}
- }
- }
- /** Retrieve values that match JVM system properties and metrics. */
- private void getRemotePropertiesAndMetrics(
- Set<String> requestedTagNames, SolrClientNodeStateProvider.RemoteCallCtx
ctx) {
+ try (InputStream in = (InputStream) metrics) {
+ String[] lines = parsePrometheusOutput(in);
- Map<String, Set<Object>> metricsKeyVsTag = new HashMap<>();
- for (String tag : requestedTagNames) {
- if (tag.startsWith(SYSPROP)) {
- metricsKeyVsTag
- .computeIfAbsent(
- "solr.jvm:system.properties:" +
tag.substring(SYSPROP.length()),
- k -> new HashSet<>())
- .add(tag);
- } else if (tag.startsWith(METRICS_PREFIX)) {
- metricsKeyVsTag
- .computeIfAbsent(tag.substring(METRICS_PREFIX.length()), k -> new
HashSet<>())
- .add(tag);
+ for (String line : lines) {
+ if (shouldSkipPrometheusLine(line) || !line.startsWith(metricName))
continue;
+
+ // If metric with specific labels were requested, then return the
metric with that label
+ // and skip others
+ if (labelKey != null && labelValue != null) {
+ String expectedLabel = labelKey + "=\"" + labelValue + "\"";
+ if (!line.contains(expectedLabel)) {
+ continue;
+ }
+ }
+
+ return extractPrometheusValue(line);
+ }
+ } catch (Exception e) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR, "Unable to read prometheus
metrics output", e);
+ }
+
+ return null;
+ }
+
+ public static long extractPrometheusValue(String line) {
+ line = line.trim();
+ String actualValue;
+ if (line.contains("}")) {
+ actualValue = line.substring(line.lastIndexOf("} ") + 1);
+ } else {
+ actualValue = line.split(" ")[1];
}
+ return (long) Double.parseDouble(actualValue);
}
- if (!metricsKeyVsTag.isEmpty()) {
- SolrClientNodeStateProvider.fetchReplicaMetrics(ctx.getNode(), ctx,
metricsKeyVsTag);
+
+ public static String[] parsePrometheusOutput(InputStream inputStream)
throws Exception {
+ String output = new String(inputStream.readAllBytes(),
StandardCharsets.UTF_8);
+ return output.split("\n");
Review Comment:
This is a bit sad as it puts the whole response into memory as a String.
Yuck. Consider a streaming approach instead. See this code snippet
alternative for inspiration:
```java
try (BufferedReader reader = new BufferedReader(
new InputStreamReader(inputStream,
StandardCharsets.UTF_8))) {
// Loop over the lines and process them
reader.lines()
.filter(line -> !line.isBlank()) // Example operation:
skip empty lines
.forEach(line -> System.out.println("Line: " + line));
}
```
--
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]
