dsmiley commented on code in PR #3713:
URL: https://github.com/apache/solr/pull/3713#discussion_r2404068227


##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -49,18 +50,27 @@ public class NodeValueFetcher {
   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);
+  public static final Set<String> tags =
+      Set.of(
+          NODE,
+          PORT,
+          HOST,
+          CORES,
+          Metrics.FREEDISK.tagName,
+          Metrics.TOTALDISK.tagName,
+          Metrics.SYSLOADAVG.tagName);
   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 Metrics {
-    FREEDISK("freedisk", "solr_cores_filesystem_disk_space", "type", 
"usable_space"),
-    TOTALDISK("totaldisk", "solr_cores_filesystem_disk_space", "type", 
"total_space"),
+    FREEDISK("freedisk", "solr_disk_space_bytes", "type", "usable_space"),
+    TOTALDISK("totaldisk", "solr_disk_space_bytes", "type", "total_space"),
     CORES("cores", "solr_cores_loaded") {
       @Override
-      public Object extractFromPrometheus(InputStream 
prometheusResponseStream) {
-        return prometheusMetricStream(prometheusResponseStream)
+      public Object extractFromPrometheus(List<String> prometheusLines) {
+        return prometheusLines.stream()
+            .filter(line -> !isPrometheusCommentLine(line))

Review Comment:
   I'd say the caller should do the generic filtering so that we only have 
actual metric lines



##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -49,18 +50,27 @@ public class NodeValueFetcher {
   public static final String HOST = "host";
   public static final String CORES = "cores";
   public static final String SYSPROP = "sysprop.";

Review Comment:
   this looks like a prefix; shouldn't it's name reflect that?



##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -173,9 +184,11 @@ private void getRemoteMetricsFromTags(
       SimpleSolrResponse rsp =
           ctx.cloudSolrClient.getHttpClient().requestWithBaseUrl(baseUrl, 
req::process);
 
+      // TODO come up with a better solution to stream this response instead 
of loading in memory
       try (InputStream prometheusStream = (InputStream) 
rsp.getResponse().get(STREAM_KEY)) {
+        List<String> prometheusLines = 
Metrics.prometheusMetricStream(prometheusStream).toList();

Review Comment:
   prometheusMetricStream should, IMO, filter out the comments and blank lines 
so that we're left with only actual metric lines.



-- 
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]

Reply via email to