dsmiley commented on code in PR #2405:
URL: https://github.com/apache/solr/pull/2405#discussion_r1612249699
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/PrometheusCoreExporterInfo.java:
##########
@@ -30,4 +32,7 @@ enum CoreCategory {
INDEX,
CORE
}
+
+ String PROMETHEUS_CLOUD_CORE_REGEX =
"^core_(.*)_(shard[0-9]+)_(replica_n[0-9]+)$";
Review Comment:
Why separate this into a constant?
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/PrometheusCoreExporterInfo.java:
##########
@@ -30,4 +32,7 @@ enum CoreCategory {
INDEX,
CORE
}
+
+ String PROMETHEUS_CLOUD_CORE_REGEX =
"^core_(.*)_(shard[0-9]+)_(replica_n[0-9]+)$";
+ Pattern PROMETHEUS_CLOUD_CORE_PATTERN =
Pattern.compile(PROMETHEUS_CLOUD_CORE_REGEX);
Review Comment:
But isn't the word "PROMETHEUS" in here actually inaccurate? This is
parsing existing internal names, it's not an external prometheus exported
pattern. I think you could just drop this part.
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -54,11 +54,11 @@ public void write(OutputStream out, SolrQueryRequest
request, SolrQueryResponse
throws IOException {
NamedList<Object> prometheusRegistries =
(NamedList<Object>) response.getValues().get("metrics");
- Iterator<Map.Entry<String, Object>> it = prometheusRegistries.iterator();
var prometheusTextFormatWriter = new PrometheusTextFormatWriter(false);
- while (it.hasNext()) {
- Map.Entry<String, Object> entry = it.next();
- SolrPrometheusCoreExporter prometheusExporter =
(SolrPrometheusCoreExporter) entry.getValue();
+ for (Iterator<Map.Entry<String, Object>> iter =
prometheusRegistries.iterator();
Review Comment:
This isn't what I meant. If you use IntelliJ, it should give you a hint --
"'for' loop can be replaced with enhanced 'for' ". Do that and I think you'll
appreciate the results.
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -54,11 +54,11 @@ public void write(OutputStream out, SolrQueryRequest
request, SolrQueryResponse
throws IOException {
NamedList<Object> prometheusRegistries =
(NamedList<Object>) response.getValues().get("metrics");
- Iterator<Map.Entry<String, Object>> it = prometheusRegistries.iterator();
var prometheusTextFormatWriter = new PrometheusTextFormatWriter(false);
- while (it.hasNext()) {
- Map.Entry<String, Object> entry = it.next();
- SolrPrometheusCoreExporter prometheusExporter =
(SolrPrometheusCoreExporter) entry.getValue();
+ for (Iterator<Map.Entry<String, Object>> iter =
prometheusRegistries.iterator();
+ iter.hasNext(); ) {
+ SolrPrometheusCoreExporter prometheusExporter =
Review Comment:
Another use of "var" here is plenty clear enough and will make this a
one-liner
--
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]
