mlbiscoc commented on code in PR #2902:
URL: https://github.com/apache/solr/pull/2902#discussion_r1894710055
##########
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:
Done!
##########
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:
Gotcha. Moved them in loops so we can remove all that hard coded `.put()`
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -113,8 +114,10 @@ public static void toPrometheus(
Metric dropwizardMetric = dropwizardMetrics.get(metricName);
formatter.exportDropwizardMetric(dropwizardMetric, metricName);
} catch (Exception e) {
- // Do not fail entirely for metrics exporting. Log and try to
export next metric
- log.warn("Error occurred exporting Dropwizard Metric to
Prometheus", e);
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Error occurred exporting Dropwizard Metric to Prometheus",
+ e);
Review Comment:
Also wanted to mention this. I was thinking to just remove the `log.warn`
and actually throw a SolrException. I think at the time I thought that not
failing metrics entirely and even just partially getting metrics was ok. But
after some thought, adding this would actually fail any metrics from posting
but helps exporting tests actually get caught if there is something wrong or
even a user finding a bug. WDYT?
##########
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:
Yeah good point. I feel like we discussed this on the PR when I introduced
this feature... Or maybe not. Regardless, it should probably be in
SolrCoreMetric, so I moved it.
--
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
