dsmiley commented on code in PR #2902:
URL: https://github.com/apache/solr/pull/2902#discussion_r1894345634
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java:
##########
@@ -30,18 +30,18 @@ public abstract class SolrCoreMetric extends SolrMetric {
public String coreName;
public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
- super(dropwizardMetric, metricName);
+ super(dropwizardMetric, metricName.substring(metricName.indexOf(".") + 1));
Matcher cloudPattern = CLOUD_CORE_PATTERN.matcher(metricName);
Matcher standalonePattern = STANDALONE_CORE_PATTERN.matcher(metricName);
if (cloudPattern.find()) {
- coreName = cloudPattern.group(1);
- labels.put("core", cloudPattern.group(1));
- labels.put("collection", cloudPattern.group(2));
- labels.put("shard", cloudPattern.group(3));
- labels.put("replica", cloudPattern.group(4));
+ coreName = cloudPattern.group("core");
+ labels.put("core", cloudPattern.group("core"));
+ labels.put("collection", cloudPattern.group("collection"));
+ labels.put("shard", cloudPattern.group("shard"));
+ labels.put("replica", cloudPattern.group("replica"));
} else if (standalonePattern.find()) {
- coreName = standalonePattern.group(1);
- labels.put("core", standalonePattern.group(1));
+ coreName = standalonePattern.group("core");
+ labels.put("core", standalonePattern.group("core"));
Review Comment:
I meant for you to do this generically with a loop, thus without the code
here actually referring to any names. See `Matcher.namedGroups()`.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java:
##########
@@ -30,18 +30,18 @@ public abstract class SolrCoreMetric extends SolrMetric {
public String coreName;
public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
- super(dropwizardMetric, metricName);
+ super(dropwizardMetric, metricName.substring(metricName.indexOf(".") + 1));
Matcher cloudPattern = CLOUD_CORE_PATTERN.matcher(metricName);
Matcher standalonePattern = STANDALONE_CORE_PATTERN.matcher(metricName);
Review Comment:
Shouldn't those patterns be defined here on SolrCoreMetric where they are
used?
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -552,6 +549,11 @@ private List<MetricType> parseMetricTypes(SolrParams
params) {
return metricTypes;
}
+ private String getCoreNameFromRegistry(String registryName) {
+ String coreName = registryName.substring(registryName.indexOf('.') + 1);
+ return coreName.replaceAll("\\.", "_");
Review Comment:
for single char find & replace, just use `replace`. You'll see by code
inspection it's much faster.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java:
##########
@@ -30,18 +30,18 @@ public abstract class SolrCoreMetric extends SolrMetric {
public String coreName;
public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
- super(dropwizardMetric, metricName);
+ super(dropwizardMetric, metricName.substring(metricName.indexOf(".") + 1));
Review Comment:
when you do that, a little comment like `// chop off ___` would be really
helpful to the reader
--
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
