dsmiley commented on code in PR #3402:
URL: https://github.com/apache/solr/pull/3402#discussion_r2167752560
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -40,12 +48,107 @@ public class PrometheusResponseWriter implements
QueryResponseWriter {
public void write(
OutputStream out, SolrQueryRequest request, SolrQueryResponse response,
String contentType)
throws IOException {
- var prometheusTextFormatWriter = new PrometheusTextFormatWriter(false);
- prometheusTextFormatWriter.write(out, (MetricSnapshots)
response.getValues().get("metrics"));
+
+ Map<String, PrometheusMetricReader> readers =
+ (Map<String, PrometheusMetricReader>)
response.getValues().get("metrics");
+
+ List<MetricSnapshot> snapshots =
+ readers.values().stream().flatMap(r -> r.collect().stream()).toList();
+
+ new PrometheusTextFormatWriter(false).write(out,
mergeSnapshots(snapshots));
}
@Override
public String getContentType(SolrQueryRequest request, SolrQueryResponse
response) {
return CONTENT_TYPE_PROMETHEUS;
}
+
+ /**
+ * Merge a collection of individual {@link MetricSnapshot} instances into
one {@link
+ * MetricSnapshots}. This is necessary because we create a {@link
SdkMeterProvider} per Solr core
+ * resulting in duplicate metric names across cores which is an illegal
format if not under the
+ * same prometheus grouping.
+ */
+ private MetricSnapshots mergeSnapshots(List<MetricSnapshot> snapshots) {
+ Map<String, CounterSnapshot.Builder> counterSnapshotMap = new HashMap<>();
+ Map<String, GaugeSnapshot.Builder> gaugeSnapshotMap = new HashMap<>();
+ Map<String, HistogramSnapshot.Builder> histogramSnapshotMap = new
HashMap<>();
+ InfoSnapshot otelInfoSnapshots = null;
+
+ for (MetricSnapshot snapshot : snapshots) {
+ String metricName = snapshot.getMetadata().getPrometheusName();
+
+ switch (snapshot) {
+ case CounterSnapshot counterSnapshot -> {
+ counterSnapshotMap.computeIfAbsent(
+ metricName,
+ k -> {
+ var base =
+ CounterSnapshot.builder()
+ .name(counterSnapshot.getMetadata().getName())
+ .help(counterSnapshot.getMetadata().getHelp());
+
+ return counterSnapshot.getMetadata().hasUnit()
+ ? base.unit(counterSnapshot.getMetadata().getUnit())
+ : base;
+ });
+
counterSnapshot.getDataPoints().forEach(counterSnapshotMap.get(metricName)::dataPoint);
Review Comment:
Thanks! What I specifically was confused about was the line of code that
had an appearance of being side-effect free, thus pointless. But the method
reference `dataPoint` _adds_ a data point. It looks ambiguous to my fresh eyes
on this API.
Having to write all this here is a bit of a PITA
--
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
