dsmiley commented on code in PR #2405:
URL: https://github.com/apache/solr/pull/2405#discussion_r1610641774
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java:
##########
@@ -41,14 +46,30 @@ public SolrCoreMetric(
this.metricName = metricName;
labels.put("core", coreName);
if (cloudMode) {
- String[] coreNameParsed = coreName.split("_");
- labels.put("collection", coreNameParsed[1]);
- labels.put("shard", coreNameParsed[2]);
- labels.put("replica", coreNameParsed[3] + "_" + coreNameParsed[4]);
+ appendCloudModeLabels();
}
}
+ public Labels getLabels() {
+ return Labels.of(new ArrayList<>(labels.keySet()), new
ArrayList<>(labels.values()));
+ }
+
public abstract SolrCoreMetric parseLabels();
- public abstract void toPrometheus(SolrPrometheusCoreExporter
solrPrometheusCoreRegistry);
+ public abstract void toPrometheus(SolrPrometheusCoreExporter
solrPrometheusCoreExporter);
+
+ private void appendCloudModeLabels() {
+ Pattern p = Pattern.compile("^core_(.*)_(shard[0-9]+)_(replica_n[0-9]+)");
Review Comment:
Should it end in a dollar-sign thus it clearly matches the entire input?
why compile each time this is called?
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/SolrPrometheusCoreExporter.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.metrics.prometheus;
+
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreCacheMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreHandlerMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreHighlighterMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreIndexMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreNoOpMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreSearcherMetric;
+import org.apache.solr.metrics.prometheus.core.SolrCoreTlogMetric;
+
+/**
+ * This class maintains a {@link
io.prometheus.metrics.model.snapshots.MetricSnapshot}s exported
+ * from solr.core {@link com.codahale.metrics.MetricRegistry}
+ */
+public class SolrPrometheusCoreExporter extends SolrPrometheusExporter {
+ public final String coreName;
+ public final boolean cloudMode;
+
+ public SolrPrometheusCoreExporter(String coreName, boolean cloudMode) {
+ super();
+ this.coreName = coreName;
+ this.cloudMode = cloudMode;
+ }
+
+ /**
+ * Export {@link Metric} to {@link
io.prometheus.metrics.model.snapshots.MetricSnapshot} and
+ * registers the Snapshot
+ *
+ * @param dropwizardMetric the {@link Meter} to be exported
+ * @param metricName Dropwizard metric name
+ */
+ @Override
+ public void exportDropwizardMetric(Metric dropwizardMetric, String
metricName) {
+ SolrCoreMetric solrCoreMetric = categorizeCoreMetric(dropwizardMetric,
metricName);
+ solrCoreMetric.parseLabels().toPrometheus(this);
+ }
+
+ private SolrCoreMetric categorizeCoreMetric(Metric dropwizardMetric, String
metricName) {
+ String metricCategory = metricName.split("\\.")[0];
+ switch (CoreCategory.valueOf(metricCategory)) {
+ case ADMIN:
+ case QUERY:
+ case UPDATE:
+ case REPLICATION:
+ {
Review Comment:
can these redundant brackets in these cases be removed?
##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -3032,6 +3034,7 @@ public PluginBag<QueryResponseWriter>
getResponseWriters() {
m.put("csv", new CSVResponseWriter());
m.put("schema.xml", new SchemaXmlResponseWriter());
m.put("smile", new SmileResponseWriter());
+ m.put(PROMETHEUS_METRICS_WT, new PrometheusResponseWriter());
Review Comment:
Nevermind. We even have schema xml format, another special purpose one.
Maybe some day this could be improved.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/SolrPrometheusExporter.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.metrics.prometheus;
+
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.Timer;
+import io.prometheus.metrics.model.snapshots.CounterSnapshot;
+import io.prometheus.metrics.model.snapshots.GaugeSnapshot;
+import io.prometheus.metrics.model.snapshots.Labels;
+import io.prometheus.metrics.model.snapshots.MetricMetadata;
+import io.prometheus.metrics.model.snapshots.MetricSnapshot;
+import io.prometheus.metrics.model.snapshots.MetricSnapshots;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Base class for all {@link SolrPrometheusExporter} holding {@link
MetricSnapshot}s. Can export
+ * {@link com.codahale.metrics.Metric} to {@link MetricSnapshot} to be
outputted for {@link
+ * org.apache.solr.response.PrometheusResponseWriter}
+ */
+public abstract class SolrPrometheusExporter implements PrometheusExporterInfo
{
+ protected final Map<String, List<CounterSnapshot.CounterDataPointSnapshot>>
metricCounters;
+ protected final Map<String, List<GaugeSnapshot.GaugeDataPointSnapshot>>
metricGauges;
+
+ public SolrPrometheusExporter() {
+ this.metricCounters = new HashMap<>();
+ this.metricGauges = new HashMap<>();
+ }
+
+ /**
+ * Export {@link Metric} to {@link
io.prometheus.metrics.model.snapshots.MetricSnapshot} and
+ * registers the Snapshot
+ *
+ * @param dropwizardMetric the {@link Metric} to be exported
+ * @param metricName Dropwizard metric name
+ */
+ public abstract void exportDropwizardMetric(Metric dropwizardMetric, String
metricName);
+
+ /**
+ * Export {@link Meter} to {@link
+ *
io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot}
and collect
+ * datapoint
+ *
+ * @param metricName name of metric after export
+ * @param dropwizardMetric the {@link Meter} to be exported
+ * @param labels label names and values to record
+ */
+ public void exportMeter(String metricName, Meter dropwizardMetric, Labels
labels) {
+ CounterSnapshot.CounterDataPointSnapshot dataPoint =
+ createCounterDatapoint((double) dropwizardMetric.getCount(), labels);
+ collectCounterDatapoint(metricName, dataPoint);
+ }
+
+ /**
+ * Export {@link com.codahale.metrics.Counter} to {@link
+ *
io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot}
and collect
+ * datapoint
+ *
+ * @param metricName name of prometheus metric
+ * @param dropwizardMetric the {@link com.codahale.metrics.Counter} to be
exported
+ * @param labels label names and values to record
+ */
+ public void exportCounter(
+ String metricName, com.codahale.metrics.Counter dropwizardMetric, Labels
labels) {
+ CounterSnapshot.CounterDataPointSnapshot dataPoint =
+ createCounterDatapoint((double) dropwizardMetric.getCount(), labels);
+ collectCounterDatapoint(metricName, dataPoint);
+ }
+
+ /**
+ * Export {@link Timer} ands its mean rate to {@link
+ *
io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot} and
collect
+ * datapoint
+ *
+ * @param metricName name of prometheus metric
+ * @param dropwizardMetric the {@link Timer} to be exported
+ * @param labels label names and values to record
+ */
+ public void exportTimer(String metricName, Timer dropwizardMetric, Labels
labels) {
+ GaugeSnapshot.GaugeDataPointSnapshot dataPoint =
+ createGaugeDatapoint(dropwizardMetric.getSnapshot().getMean(), labels);
+ collectGaugeDatapoint(metricName, dataPoint);
+ }
+
+ /**
+ * Export {@link com.codahale.metrics.Gauge} to {@link
+ *
io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot} and
collect to
+ * datapoint. Unlike other Dropwizard metric types, Gauges can have more
complex types. In the
+ * case of a hashmap, collect each as an individual metric and have its key
appended as a label to
+ * the metric called "item"
+ *
+ * @param metricName name of prometheus metric
+ * @param dropwizardMetricRaw the {@link com.codahale.metrics.Gauge} to be
exported
+ * @param labels label names and values to record
+ */
+ public void exportGauge(
+ String metricName, com.codahale.metrics.Gauge<?> dropwizardMetricRaw,
Labels labels) {
+ Object dropwizardMetric = (dropwizardMetricRaw).getValue();
+ if (dropwizardMetric instanceof Number) {
+ GaugeSnapshot.GaugeDataPointSnapshot dataPoint =
+ createGaugeDatapoint(((Number) dropwizardMetric).doubleValue(),
labels);
+ collectGaugeDatapoint(metricName, dataPoint);
+ } else if (dropwizardMetric instanceof HashMap) {
+ HashMap<?, ?> itemsMap = (HashMap<?, ?>) dropwizardMetric;
+ for (Object item : itemsMap.keySet()) {
+ if (itemsMap.get(item) instanceof Number) {
+ GaugeSnapshot.GaugeDataPointSnapshot dataPoint =
+ createGaugeDatapoint(
+ ((Number) itemsMap.get(item)).doubleValue(),
+ labels.merge(Labels.of("item", (String) item)));
+ collectGaugeDatapoint(metricName, dataPoint);
+ }
+ }
+ }
+ }
+
+ /**
+ * Create a {@link
io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot}
+ * with labels
+ *
+ * @param value metric value
+ * @param labels set of name/values labels
+ */
+ public CounterSnapshot.CounterDataPointSnapshot createCounterDatapoint(
+ double value, Labels labels) {
+ return
CounterSnapshot.CounterDataPointSnapshot.builder().value(value).labels(labels).build();
+ }
+
+ /**
+ * Create a {@link
io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot}
+ * with labels
+ *
+ * @param value metric value
+ * @param labels set of name/values labels
+ */
+ public GaugeSnapshot.GaugeDataPointSnapshot createGaugeDatapoint(double
value, Labels labels) {
+ return
GaugeSnapshot.GaugeDataPointSnapshot.builder().value(value).labels(labels).build();
+ }
+
+ /**
+ * Collects {@link
io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot}
+ * and appends to existing metric or create new metric if name does not exist
+ *
+ * @param metricName Name of metric
+ * @param dataPoint Counter datapoint to be collected
+ */
+ public void collectCounterDatapoint(
+ String metricName, CounterSnapshot.CounterDataPointSnapshot dataPoint) {
+ if (!metricCounters.containsKey(metricName)) {
+ metricCounters.put(metricName, new ArrayList<>());
+ }
+ metricCounters.get(metricName).add(dataPoint);
+ }
+
+ /**
+ * Collects {@link
io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot} and
+ * appends to existing metric or create new metric if name does not exist
+ *
+ * @param metricName Name of metric
+ * @param dataPoint Gauge datapoint to be collected
+ */
+ public void collectGaugeDatapoint(
+ String metricName, GaugeSnapshot.GaugeDataPointSnapshot dataPoint) {
+ if (!metricGauges.containsKey(metricName)) {
+ metricGauges.put(metricName, new ArrayList<>());
+ }
+ metricGauges.get(metricName).add(dataPoint);
+ }
+
+ /**
+ * Returns an immutable {@link MetricSnapshots} from the {@link
+ * io.prometheus.metrics.model.snapshots.DataPointSnapshot}s collected from
the registry
+ */
+ public MetricSnapshots collect() {
Review Comment:
Does the order matter, like will we return all counters then all gauges in
the result? If so, I could see that being kind of annoying but not a deal
breaker of course.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreSearcherMetric.java:
##########
@@ -47,24 +45,18 @@ public SolrCoreMetric parseLabels() {
}
@Override
- public void toPrometheus(SolrPrometheusCoreExporter
solrPrometheusCoreRegistry) {
+ public void toPrometheus(SolrPrometheusCoreExporter
solrPrometheusCoreExporter) {
Review Comment:
nice. You also could have simply called it "exporter" or "prometheus". It
might then be so short enough that the usages become one-liners. Any way it's
fine.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/PrometheusExporterInfo.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.metrics.prometheus;
+
+public interface PrometheusExporterInfo {
+ /** Category of prefix Solr Core dropwizard handler metric names */
+ enum CoreCategory {
Review Comment:
Should this go into SolrCoreMetric as it's CORE specific?
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -16,31 +16,115 @@
*/
package org.apache.solr.response;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer;
import io.prometheus.metrics.expositionformats.PrometheusTextFormatWriter;
import java.io.IOException;
import java.io.OutputStream;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+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.util.NamedList;
+import org.apache.solr.metrics.AggregateMetric;
import org.apache.solr.metrics.prometheus.SolrPrometheusCoreExporter;
import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.util.stats.MetricUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+/**
+ * Response writer for Prometheus metrics. This is used only by the {@link
+ * org.apache.solr.handler.admin.MetricsHandler}
+ */
@SuppressWarnings(value = "unchecked")
public class PrometheusResponseWriter extends RawResponseWriter {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
@Override
public void write(OutputStream out, SolrQueryRequest request,
SolrQueryResponse response)
throws IOException {
NamedList<Object> prometheusRegistries =
(NamedList<Object>) response.getValues().get("metrics");
- Map<String, Object> registryMap = prometheusRegistries.asShallowMap();
- PrometheusTextFormatWriter prometheusTextFormatWriter = new
PrometheusTextFormatWriter(false);
- registryMap.forEach(
- (name, registry) -> {
+ Iterator<Map.Entry<String, Object>> it = prometheusRegistries.iterator();
Review Comment:
Using an Iterator manually reminds me of the Java 1.1 days.... in a bad way.
It's needless here, can use Java 7 for loop syntax on any Iterable., saving
you 2 lines.
##########
solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java:
##########
@@ -43,47 +42,35 @@ public SolrCoreMetric parseLabels() {
String category = parsedMetric[0];
String handler = parsedMetric[1];
String type = parsedMetric[2];
- labels.put("category", category);
- labels.put("type", type);
- labels.put("handler", handler);
+ labels.putAll(Map.of("category", category, "type", type, "handler",
handler));
Review Comment:
3 puts was fine! I meant, basically, that you could inline those 3 vars
above such that you only have the 3 put lines, not 2x.
putAll with a one-off Map seems... not great.
--
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
