mlbiscoc commented on code in PR #1627: URL: https://github.com/apache/solr/pull/1627#discussion_r1224493642
########## solr/prometheus-exporter/src/java/org/apache/solr/prometheus/collector/MetricSamples.java: ########## @@ -71,4 +83,8 @@ public List<Collector.MetricFamilySamples> asList() { .filter(value -> !value.samples.isEmpty()) .collect(Collectors.toList()); } + + private void addUnseenSamples(Collector.MetricFamilySamples metricFamilySamples) { + seenSamples.addAll(metricFamilySamples.samples); Review Comment: Took another look at it and I agree, this is confusing. Just removed that function and added it inline where it was being called. As for the duplication explanation, I apologize I was wrong. It is not caused by collections, but cores. For the default configuration, `solr_ping` seems to be the biggest culprit because the labels are only `base_url` and `cluster_id`. The PingCollection scrapes the metrics at all cores, but `solr_ping` have the same labels and values across cores with the same host. This issue could possibly arise as well with custom configurations on metrics finding duplicates if the labels aren't unique enough. so this `seenSamples` should catch that and not output duplicates. -- 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