Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17587: (9x backport) wt=prometheus fix duplicate TYPE information #3006

Merged
merged 2 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ Optimizations

Bug Fixes
---------------------
* PR#2680: Improve reliablity of NpmTasks finding needed files/commands. (Tyler Bertrand via Eric Pugh)
* SOLR-17587: Metrics: wt=prometheus fix for non-compliant exposition format containing duplicate TYPE lines.
(Matthew Biscocho via David Smiley)

Dependency Upgrades
---------------------
Expand All @@ -41,6 +42,8 @@ Other Changes
* GITHUB#2869: SolrTestCase now supports @LogLevel annotations (as SolrTestCaseJ4 has). Added LogLevelTestRule
for encapsulation and reuse. (David Smiley)

* GITHUB#2680: Improve reliablity of NpmTasks finding needed files/commands. (Tyler Bertrand via Eric Pugh)

================== 9.8.0 ==================
New Features
---------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,20 @@ 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(getCoreNameFromRegistry(registryName), dropwizardRegistry);
continue;
}

PrometheusResponseWriter.toPrometheus(
dropwizardRegistry,
registryName,
Expand All @@ -203,6 +213,22 @@ private NamedList<Object> handlePrometheusExport(SolrParams params) {
response.add(registryName, registry);
});
}

if (!mergedCoreRegistries.getMetrics().isEmpty()) {
PrometheusResponseWriter.toPrometheus(
mergedCoreRegistries,
"solr.core",
metricFilters,
mustMatchFilter,
propertyFilter,
false,
false,
true,
(registry) -> {
response.add("solr.core", registry);
});
}

return response;
}

Expand Down Expand Up @@ -523,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.replace(".", "_");
}

@Override
public String getDescription() {
return "A handler to return all the metrics gathered by Solr";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*/
package org.apache.solr.metrics.prometheus.core;

import java.util.regex.Pattern;

public interface PrometheusCoreFormatterInfo {
/** Category of prefix Solr Core dropwizard handler metric names */
enum CoreCategory {
Expand All @@ -32,6 +30,4 @@ enum CoreCategory {
INDEX,
CORE
}

Pattern CLOUD_CORE_PATTERN = Pattern.compile("^core_(.*)_(shard[0-9]+)_(replica_.[0-9]+)$");
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
public class SolrCoreCacheMetric extends SolrCoreMetric {
public static final String CORE_CACHE_SEARCHER_METRICS = "solr_metrics_core_cache";

public SolrCoreCacheMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreCacheMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ public class SolrCoreHandlerMetric extends SolrCoreMetric {
public static final String CORE_REQUESTS_TOTAL_TIME = "solr_metrics_core_requests_time";
public static final String CORE_REQUEST_TIMES = "solr_metrics_core_average_request_time";

public SolrCoreHandlerMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreHandlerMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
public class SolrCoreHighlighterMetric extends SolrCoreMetric {
public static final String CORE_HIGHLIGHER_METRICS = "solr_metrics_core_highlighter_requests";

public SolrCoreHighlighterMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreHighlighterMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
public class SolrCoreIndexMetric extends SolrCoreMetric {
public static final String CORE_INDEX_METRICS = "solr_metrics_core_index_size_bytes";

public SolrCoreIndexMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreIndexMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,45 @@
*/
package org.apache.solr.metrics.prometheus.core;

import static org.apache.solr.metrics.prometheus.core.PrometheusCoreFormatterInfo.CLOUD_CORE_PATTERN;

import com.codahale.metrics.Metric;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.solr.common.SolrException;
import org.apache.solr.metrics.prometheus.SolrMetric;

/** 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) {
super(dropwizardMetric, metricName);
this.coreName = coreName;
labels.put("core", coreName);
if (cloudMode) {
appendCloudModeLabels();
}
}
public static Pattern CLOUD_CORE_PATTERN =
Pattern.compile(
"(?<core>^core_(?<collection>.*)_(?<shard>shard[0-9]+)_(?<replica>replica_.[0-9]+)).(.*)$");
public static Pattern STANDALONE_CORE_PATTERN = Pattern.compile("^core_(?<core>.*?)\\.(.*)$");
public static List<String> CLOUD_LABEL_KEYS = List.of("core", "collection", "shard", "replica");
public static List<String> STANDALONE_LABEL_KEYS = List.of("core");
Comment on lines +33 to +34
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a static List of the keys for each regex expression instead of the Map and looped through these to add the labels

public String coreName;

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));
public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
// Remove Core Name prefix from metric as it is no longer needed for tag parsing from this point
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("core");
CLOUD_LABEL_KEYS.forEach(
(key) -> {
labels.put(key, cloudPattern.group(key));
});
} else if (standalonePattern.find()) {
coreName = standalonePattern.group("core");
STANDALONE_LABEL_KEYS.forEach(
(key) -> {
labels.put(key, standalonePattern.group(key));
});
} else {
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
"Core name does not match pattern for parsing " + coreName);
"Core name does not match pattern for parsing in metric " + metricName);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ public class SolrCoreSearcherMetric extends SolrCoreMetric {
public static final String CORE_SEARCHER_METRICS = "solr_metrics_core_searcher_documents";
public static final String CORE_SEARCHER_TIMES = "solr_metrics_core_average_searcher_warmup_time";

public SolrCoreSearcherMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreSearcherMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
public class SolrCoreTlogMetric extends SolrCoreMetric {
public static final String CORE_TLOG_METRICS = "solr_metrics_core_tlog";

public SolrCoreTlogMetric(
Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
super(dropwizardMetric, coreName, metricName, cloudMode);
public SolrCoreTlogMetric(Metric dropwizardMetric, String metricName) {
super(dropwizardMetric, metricName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,9 @@
*/
public class SolrPrometheusCoreFormatter extends SolrPrometheusFormatter
implements PrometheusCoreFormatterInfo {
public final String coreName;
public final boolean cloudMode;

public SolrPrometheusCoreFormatter(String coreName, boolean cloudMode) {
public SolrPrometheusCoreFormatter() {
super();
this.coreName = coreName;
this.cloudMode = cloudMode;
}

@Override
Expand All @@ -45,7 +41,7 @@ public void exportDropwizardMetric(Metric dropwizardMetric, String metricName) {

@Override
public SolrMetric categorizeMetric(Metric dropwizardMetric, String metricName) {
String metricCategory = metricName.split("\\.", 2)[0];
String metricCategory = metricName.split("\\.", 3)[1];
if (!Enums.getIfPresent(PrometheusCoreFormatterInfo.CoreCategory.class, metricCategory)
.isPresent()) {
return new SolrNoOpMetric();
Expand All @@ -55,17 +51,17 @@ public SolrMetric categorizeMetric(Metric dropwizardMetric, String metricName) {
case QUERY:
case UPDATE:
case REPLICATION:
return new SolrCoreHandlerMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreHandlerMetric(dropwizardMetric, metricName);
case TLOG:
return new SolrCoreTlogMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreTlogMetric(dropwizardMetric, metricName);
case CACHE:
return new SolrCoreCacheMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreCacheMetric(dropwizardMetric, metricName);
case SEARCHER:
return new SolrCoreSearcherMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreSearcherMetric(dropwizardMetric, metricName);
case HIGHLIGHTER:
return new SolrCoreHighlighterMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreHighlighterMetric(dropwizardMetric, metricName);
case INDEX:
return new SolrCoreIndexMetric(dropwizardMetric, coreName, metricName, cloudMode);
return new SolrCoreIndexMetric(dropwizardMetric, metricName);
case CORE:
default:
return new SolrNoOpMetric();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@
import java.io.IOException;
import java.io.OutputStream;
import java.lang.invoke.MethodHandles;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.metrics.AggregateMetric;
import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
Expand Down Expand Up @@ -115,30 +114,22 @@ 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);
}
});

consumer.accept(formatter);
}

public static SolrPrometheusFormatter getFormatterType(String registryName) {
String coreName;
boolean cloudMode = false;
String[] parsedRegistry = registryName.split("\\.");

switch (parsedRegistry[1]) {
case "core":
if (parsedRegistry.length == 3) {
coreName = parsedRegistry[2];
} else if (parsedRegistry.length == 5) {
coreName = Arrays.stream(parsedRegistry).skip(1).collect(Collectors.joining("_"));
cloudMode = true;
} else {
coreName = registryName;
}
return new SolrPrometheusCoreFormatter(coreName, cloudMode);
return new SolrPrometheusCoreFormatter();
case "jvm":
return new SolrPrometheusJvmFormatter();
case "jetty":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -725,8 +725,7 @@ public void testPrometheusMetricsCore() throws Exception {
NamedList<?> values = resp.getValues();
assertNotNull(values.get("metrics"));
values = (NamedList<?>) values.get("metrics");
SolrPrometheusFormatter formatter =
(SolrPrometheusFormatter) values.get("solr.core.collection1");
SolrPrometheusFormatter formatter = (SolrPrometheusFormatter) values.get("solr.core");
assertNotNull(formatter);
MetricSnapshots actualSnapshots = formatter.collect();
assertNotNull(actualSnapshots);
Expand Down Expand Up @@ -1009,8 +1008,7 @@ public void testPrometheusMetricsFilter() throws Exception {
NamedList<?> values = resp.getValues();
assertNotNull(values.get("metrics"));
values = (NamedList<?>) values.get("metrics");
SolrPrometheusFormatter formatter =
(SolrPrometheusFormatter) values.get("solr.core.collection1");
SolrPrometheusFormatter formatter = (SolrPrometheusFormatter) values.get("solr.core");
assertNotNull(formatter);
MetricSnapshots actualSnapshots = formatter.collect();
assertNotNull(actualSnapshots);
Expand Down
Loading