Sinscerly commented on code in PR #12112:
URL: https://github.com/apache/cloudstack/pull/12112#discussion_r2618715937


##########
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java:
##########
@@ -512,20 +512,48 @@ public void updateMetrics() {
     public String getMetrics() {
         final StringBuilder stringBuilder = new StringBuilder();
         stringBuilder.append("# Cloudstack Prometheus Metrics\n");
-        for (final Item item : metricsItems) {
+
+        List<Item> sortedItems = metricsItems.stream()
+            .sorted((item1, item2) -> item1.name.compareTo(item2.name))
+            .collect(Collectors.toList());
+
+        String currentMetricName = null;
+
+        for (Item item : sortedItems) {
+            if (!item.name.equals(currentMetricName)) {
+                currentMetricName = item.name;
+                stringBuilder.append("# HELP 
").append(currentMetricName).append(" ")
+                            .append(item.getHelp()).append("\n");

Review Comment:
   Hi @nvazquez,
   
   Well the item `ItemHostCpu` returns two different metrics: 
`cloudstack_host_cpu_usage_mhz_total` and 
`cloudstack_host_cpu_usage_mhz_total_by_tag`. I just made sure the correct help 
line is outputted for that metric. 
   
   In the end all metrics are sorted and each unique help line is added. so 
this will give:
   # HELP cloudstack_host_cpu_usage_mhz_total <help description>
   ... metrics for above type
   ... ''
   # HELP cloudstack_host_cpu_usage_mhz_total_by_tag <help description>
   ... metrics for above type
   
   So it seemed logical for me to also change that. Although I'm doubting it 
will work, as the `toMetricsString` is called after adding in the HELP and TYPE 
I see. Although this would require some bigger rework of adding metrics I 
think, split it up in separate items.
   
   ---
   Removing the `if (!item.name.equals(currentMetricName))` will cause harm as 
you only want it once for every unique metric name. As you want:
   ```
   # HELP cloudstack_domain_resource_count Resource usage count per domain
   # TYPE cloudstack_domain_resource_count gauge
   cloudstack_domain_resource_count{domain="/", type="memory"} 0
   cloudstack_domain_resource_count{domain="/", type="cpu"} 0
   cloudstack_domain_resource_count{domain="/", type="gpu"} 0
   cloudstack_domain_resource_count{domain="/", type="primary_storage"} 0
   ```
   And not the HELP and TYPE for every single metric item:
   ```
   # HELP cloudstack_domain_resource_count Resource usage count per domain
   # TYPE cloudstack_domain_resource_count gauge
   cloudstack_domain_resource_count{domain="/", type="memory"} 0
   # HELP cloudstack_domain_resource_count Resource usage count per domain
   # TYPE cloudstack_domain_resource_count gauge
   cloudstack_domain_resource_count{domain="/", type="cpu"} 0
   # HELP cloudstack_domain_resource_count Resource usage count per domain
   # TYPE cloudstack_domain_resource_count gauge
   cloudstack_domain_resource_count{domain="/", type="gpu"} 0
   # HELP cloudstack_domain_resource_count Resource usage count per domain
   # TYPE cloudstack_domain_resource_count gauge
   cloudstack_domain_resource_count{domain="/", type="primary_storage"} 0
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to