mlbiscoc commented on code in PR #2902:
URL: https://github.com/apache/solr/pull/2902#discussion_r1894204642
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -186,10 +187,22 @@ private NamedList<Object>
handlePrometheusExport(SolrParams params) {
List<MetricType> metricTypes = parseMetricTypes(params);
List<MetricFilter> metricFilters =
metricTypes.stream().map(MetricType::asMetricFilter).collect(Collectors.toList());
+
Set<String> requestedRegistries = parseRegistries(params);
+ MetricRegistry mergedCoreRegistries = new MetricRegistry();
for (String registryName : requestedRegistries) {
MetricRegistry dropwizardRegistry = metricManager.registry(registryName);
+
+ // Merge all core registries into a single registry and
+ // append the core name to the metric to avoid duplicate metrics name
+ if (registryName.startsWith("solr.core")) {
+ mergedCoreRegistries.registerAll(
+
Arrays.stream(registryName.split("\\.")).skip(1).collect(Collectors.joining("_")),
Review Comment:
Added a private utility method
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreCacheMetric.java:
##########
@@ -38,7 +37,7 @@ public SolrCoreCacheMetric(
public SolrCoreMetric parseLabels() {
String[] parsedMetric = metricName.split("\\.");
if (dropwizardMetric instanceof Gauge) {
- labels.put("cacheType", parsedMetric[2]);
+ labels.put("cacheType", parsedMetric[3]);
Review Comment:
You're right. Might be a bit more confusing especially since the coreName
appended is only for internally merging the registries so Prometheus can format
the metrics. It doesn't actually change Dropwizards normal `admin/metrics` api
call naming. So instead I changed the constructor of `SolrCoreMetric`, after
the registries are already merged and exporting is happening, we just remove
that coreName from the dropwizard metric name and exporting happens as normal
without having to slide the indices.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java:
##########
@@ -25,28 +26,26 @@
/** Base class is a wrapper to export a solr.core {@link
com.codahale.metrics.Metric} */
public abstract class SolrCoreMetric extends SolrMetric {
+
public String coreName;
- public SolrCoreMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean
cloudMode) {
+ public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
- this.coreName = coreName;
- labels.put("core", coreName);
- if (cloudMode) {
- appendCloudModeLabels();
- }
- }
-
- private void appendCloudModeLabels() {
- Matcher m = CLOUD_CORE_PATTERN.matcher(coreName);
- if (m.find()) {
- labels.put("collection", m.group(1));
- labels.put("shard", m.group(2));
- labels.put("replica", m.group(3));
+ 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));
Review Comment:
Cool, learned something new. Added.
--
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]
