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 and updated CHANGES.txt
   
   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

Reply via email to