You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/12/16 08:39:27 UTC

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5773: [WIP] Refactor Prometheus plugin

DaanHoogland commented on a change in pull request #5773:
URL: https://github.com/apache/cloudstack/pull/5773#discussion_r770322379



##########
File path: plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java
##########
@@ -131,6 +142,24 @@ private void addHostMetrics(final List<Item> metricsList, final long dcId, final
             int isDedicated = (dr != null) ? 1 : 0;
             metricsList.add(new ItemHostIsDedicated(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), isDedicated));
 
+            List<String> hostTags = _hostTagsDao.gethostTags(host.getId());
+            String hosttags = StringUtils.join(hostTags, ",");
+            for (String tag : hostTags) {
+                Integer current = totalHosts.get(tag) != null ? totalHosts.get(tag) : 0;
+                totalHosts.put(tag, current + 1);
+            }
+            if (host.getStatus() == Status.Up) {
+                for (String tag : hostTags) {
+                    Integer current = upHosts.get(tag) != null ? upHosts.get(tag) : 0;
+                    upHosts.put(tag, current + 1);
+                }
+            } else if (host.getStatus() == Status.Disconnected || host.getStatus() == Status.Down) {
+                for (String tag : hostTags) {
+                    Integer current = downHosts.get(tag) != null ? downHosts.get(tag) : 0;
+                    downHosts.put(tag, current + 1);
+                }
+            }
+

Review comment:
       addHostMetrics() is already way to long, and the new functionality shouldn't be added to it, so as an example
   this functionality seems like it could be extracted and there is some repetition in it that can be generalised as well.
   ```suggestion
               String hosttags = markTagMaps(totalHosts, upHosts,  downHosts);
   ```
   and then add
   ```
           private String markTagMaps(Map<String, Integer> totalHosts, Map<String, Integer> upHosts, Map<String, Integer> downHosts) {
               List<String> hostTags = _hostTagsDao.gethostTags(host.getId());
               markTags(hostTags,totalHosts);
               if (host.getStatus() == Status.Up) {
                   markTags(hostTags, upHosts);
               } else if (host.getStatus() == Status.Disconnected || host.getStatus() == Status.Down) {
                   markTags(hostTags, downHosts);
               }
               return StringUtils.join(hostTags, ",");
           }
   ```
   and
   ```
     private void markTags(List<String tags, Map<String, Integer> tagMap) {
               for (String tag : tags) {
                   Integer current = tagMap.get(tag) != null ? tagMap.get(tag) : 0;
                   tagMap.put(tag, current + 1);
               }
           }
   ```

##########
File path: plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java
##########
@@ -144,53 +173,75 @@ private void addHostMetrics(final List<Item> metricsList, final long dcId, final
             final String cpuFactor = String.valueOf(CapacityManager.CpuOverprovisioningFactor.valueIn(host.getClusterId()));
             final CapacityVO cpuCapacity = capacityDao.findByHostIdType(host.getId(), Capacity.CAPACITY_TYPE_CPU);
             if (cpuCapacity != null) {
-                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, USED, cpuCapacity.getUsedCapacity()));
-                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, TOTAL, cpuCapacity.getTotalCapacity()));
+                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, USED, cpuCapacity.getUsedCapacity(), isDedicated, hosttags));
+                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, TOTAL, cpuCapacity.getTotalCapacity(), isDedicated, hosttags));
             } else {
-                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, USED, 0L));
-                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, TOTAL, 0L));
+                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, USED, 0L, isDedicated, hosttags));
+                metricsList.add(new ItemHostCpu(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), cpuFactor, TOTAL, 0L, isDedicated, hosttags));
             }
 
             final String memoryFactor = String.valueOf(CapacityManager.MemOverprovisioningFactor.valueIn(host.getClusterId()));
             final CapacityVO memCapacity = capacityDao.findByHostIdType(host.getId(), Capacity.CAPACITY_TYPE_MEMORY);
             if (memCapacity != null) {
-                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, USED, memCapacity.getUsedCapacity(), isDedicated));
-                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, TOTAL, memCapacity.getTotalCapacity(), isDedicated));
+                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, USED, memCapacity.getUsedCapacity(), isDedicated, hosttags));
+                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, TOTAL, memCapacity.getTotalCapacity(), isDedicated, hosttags));
             } else {
-                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, USED, 0L, isDedicated));
-                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, TOTAL, 0L, isDedicated));
+                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, USED, 0L, isDedicated, hosttags));
+                metricsList.add(new ItemHostMemory(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), memoryFactor, TOTAL, 0L, isDedicated, hosttags));
             }
 
             metricsList.add(new ItemHostVM(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), vmDao.listByHostId(host.getId()).size()));
 
             final CapacityVO coreCapacity = capacityDao.findByHostIdType(host.getId(), Capacity.CAPACITY_TYPE_CPU_CORE);
             if (coreCapacity != null) {
-                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), USED, coreCapacity.getUsedCapacity(), isDedicated));
-                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), TOTAL, coreCapacity.getTotalCapacity(), isDedicated));
+                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), USED, coreCapacity.getUsedCapacity(), isDedicated, hosttags));
+                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), TOTAL, coreCapacity.getTotalCapacity(), isDedicated, hosttags));
             } else {
-                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), USED, 0L, isDedicated));
-                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), TOTAL, 0L, isDedicated));
+                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), USED, 0L, isDedicated, hosttags));
+                metricsList.add(new ItemVMCore(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), TOTAL, 0L, isDedicated, hosttags));
             }
         }
 
         final List<CapacityDaoImpl.SummedCapacity> cpuCapacity = capacityDao.findCapacityBy((int) Capacity.CAPACITY_TYPE_CPU, dcId, null, null);
         if (cpuCapacity != null && cpuCapacity.size() > 0) {
-            metricsList.add(new ItemHostCpu(zoneName, zoneUuid, null, null, null, null, ALLOCATED, cpuCapacity.get(0).getAllocatedCapacity() != null ? cpuCapacity.get(0).getAllocatedCapacity() : 0));
+            metricsList.add(new ItemHostCpu(zoneName, zoneUuid, null, null, null, null, ALLOCATED, cpuCapacity.get(0).getAllocatedCapacity() != null ? cpuCapacity.get(0).getAllocatedCapacity() : 0, 0, ""));
         }
 
         final List<CapacityDaoImpl.SummedCapacity> memCapacity = capacityDao.findCapacityBy((int) Capacity.CAPACITY_TYPE_MEMORY, dcId, null, null);
         if (memCapacity != null && memCapacity.size() > 0) {
-            metricsList.add(new ItemHostMemory(zoneName, zoneUuid, null, null, null, null, ALLOCATED, memCapacity.get(0).getAllocatedCapacity() != null ? memCapacity.get(0).getAllocatedCapacity() : 0, 0));
+            metricsList.add(new ItemHostMemory(zoneName, zoneUuid, null, null, null, null, ALLOCATED, memCapacity.get(0).getAllocatedCapacity() != null ? memCapacity.get(0).getAllocatedCapacity() : 0, 0, ""));
         }
 
         final List<CapacityDaoImpl.SummedCapacity> coreCapacity = capacityDao.findCapacityBy((int) Capacity.CAPACITY_TYPE_CPU_CORE, dcId, null, null);
         if (coreCapacity != null && coreCapacity.size() > 0) {
-            metricsList.add(new ItemVMCore(zoneName, zoneUuid, null, null, null, ALLOCATED, coreCapacity.get(0).getAllocatedCapacity() != null ? coreCapacity.get(0).getAllocatedCapacity() : 0, 0));
+            metricsList.add(new ItemVMCore(zoneName, zoneUuid, null, null, null, ALLOCATED, coreCapacity.get(0).getAllocatedCapacity() != null ? coreCapacity.get(0).getAllocatedCapacity() : 0, 0, ""));
+        }
+
+        metricsList.add(new ItemHost(zoneName, zoneUuid, ONLINE, up, null));
+        metricsList.add(new ItemHost(zoneName, zoneUuid, OFFLINE, down, null));
+        metricsList.add(new ItemHost(zoneName, zoneUuid, TOTAL, total, null));
+        for (Map.Entry<String, Integer> entry : totalHosts.entrySet()) {
+            String tag = entry.getKey();
+            Integer count = entry.getValue();
+            metricsList.add(new ItemHost(zoneName, zoneUuid, TOTAL, count, tag));
+            if (upHosts.get(tag) != null) {
+                metricsList.add(new ItemHost(zoneName, zoneUuid, ONLINE, upHosts.get(tag), tag));
+            } else {
+                metricsList.add(new ItemHost(zoneName, zoneUuid, ONLINE, 0, tag));
+            }
+            if (downHosts.get(tag) != null) {
+                metricsList.add(new ItemHost(zoneName, zoneUuid, OFFLINE, downHosts.get(tag), tag));
+            } else {
+                metricsList.add(new ItemHost(zoneName, zoneUuid, OFFLINE, 0, tag));
+            }
+        }
+        for (Map.Entry<String, Integer> entry : totalHosts.entrySet()) {
+            String tag = entry.getKey();
+            Ternary<Long, Long, Long> allocatedCapacityByTag = capacityDao.findCapacityByZoneAndHostTag(dcId, tag);
+            metricsList.add(new ItemVMCore(zoneName, zoneUuid, null, null, null, ALLOCATED, allocatedCapacityByTag.first(), 0, tag));
+            metricsList.add(new ItemHostCpu(zoneName, zoneUuid, null, null, null, null, ALLOCATED, allocatedCapacityByTag.second(),0, tag));
+            metricsList.add(new ItemHostMemory(zoneName, zoneUuid, null, null, null, null, ALLOCATED, allocatedCapacityByTag.third(), 0, tag));

Review comment:
       please extract this

##########
File path: plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java
##########
@@ -201,6 +252,18 @@ private void addVMMetrics(final List<Item> metricsList, final long dcId, final S
             }
             metricsList.add(new ItemVM(zoneName, zoneUuid, state.name().toLowerCase(), count));
         }
+
+        List<String> allHostTags = hostDao.listAll().stream()
+                .flatMap( h -> _hostTagsDao.gethostTags(h.getId()).stream())
+                .distinct()
+                .collect(Collectors.toList());
+
+        for (final State state : State.values()) {
+            for (final String hosttag : allHostTags) {
+                final Long count = vmDao.countByZoneAndStateAndHostTag(dcId, state, hosttag);
+                metricsList.add(new ItemVMByTag(zoneName, zoneUuid, state.name().toLowerCase(), count, hosttag));
+            }
+        }

Review comment:
       please extract this




-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org