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 2019/01/08 14:19:18 UTC
[GitHub] GabrielBrascher commented on a change in pull request #3078: Add
influxdb to statscollector
GabrielBrascher commented on a change in pull request #3078: Add influxdb to statscollector
URL: https://github.com/apache/cloudstack/pull/3078#discussion_r246011524
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -497,86 +572,35 @@ protected void runInContext() {
}
try {
- HashMap<Long, VmStatsEntry> vmStatsById = _userVmMgr.getVirtualMachineStatistics(host.getId(), host.getName(), vmIds);
+ Map<Long, VmStatsEntry> vmStatsById = _userVmMgr.getVirtualMachineStatistics(host.getId(), host.getName(), vmIds);
if (vmStatsById != null) {
- VmStatsEntry statsInMemory = null;
-
Set<Long> vmIdSet = vmStatsById.keySet();
for (Long vmId : vmIdSet) {
VmStatsEntry statsForCurrentIteration = vmStatsById.get(vmId);
- statsInMemory = (VmStatsEntry)_VmStats.get(vmId);
+ statsForCurrentIteration.setVmId(vmId);
+ UserVmVO userVmVo = _userVmDao.findById(vmId);
+ statsForCurrentIteration.setUserVmVO(userVmVo);
- if (statsInMemory == null) {
- //no stats exist for this vm, directly persist
- _VmStats.put(vmId, statsForCurrentIteration);
- } else {
- //update each field
- statsInMemory.setCPUUtilization(statsForCurrentIteration.getCPUUtilization());
- statsInMemory.setNumCPUs(statsForCurrentIteration.getNumCPUs());
- statsInMemory.setNetworkReadKBs(statsInMemory.getNetworkReadKBs() + statsForCurrentIteration.getNetworkReadKBs());
- statsInMemory.setNetworkWriteKBs(statsInMemory.getNetworkWriteKBs() + statsForCurrentIteration.getNetworkWriteKBs());
- statsInMemory.setDiskWriteKBs(statsInMemory.getDiskWriteKBs() + statsForCurrentIteration.getDiskWriteKBs());
- statsInMemory.setDiskReadIOs(statsInMemory.getDiskReadIOs() + statsForCurrentIteration.getDiskReadIOs());
- statsInMemory.setDiskWriteIOs(statsInMemory.getDiskWriteIOs() + statsForCurrentIteration.getDiskWriteIOs());
- statsInMemory.setDiskReadKBs(statsInMemory.getDiskReadKBs() + statsForCurrentIteration.getDiskReadKBs());
- statsInMemory.setMemoryKBs(statsForCurrentIteration.getMemoryKBs());
- statsInMemory.setIntFreeMemoryKBs(statsForCurrentIteration.getIntFreeMemoryKBs());
- statsInMemory.setTargetMemoryKBs(statsForCurrentIteration.getTargetMemoryKBs());
-
- _VmStats.put(vmId, statsInMemory);
- }
-
- /**
- * Add statistics to HashMap only when they should be send to a external stats collector
- * Performance wise it seems best to only append to the HashMap when needed
- */
- if (externalStatsEnabled) {
- VMInstanceVO vmVO = _vmInstance.findById(vmId);
- String vmName = vmVO.getUuid();
-
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".cpu.num", statsForCurrentIteration.getNumCPUs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".cpu.utilization", statsForCurrentIteration.getCPUUtilization());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".network.read_kbs", statsForCurrentIteration.getNetworkReadKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".network.write_kbs", statsForCurrentIteration.getNetworkWriteKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".disk.write_kbs", statsForCurrentIteration.getDiskWriteKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".disk.read_kbs", statsForCurrentIteration.getDiskReadKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".disk.write_iops", statsForCurrentIteration.getDiskWriteIOs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".disk.read_iops", statsForCurrentIteration.getDiskReadIOs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".memory.total_kbs", statsForCurrentIteration.getMemoryKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".memory.internalfree_kbs", statsForCurrentIteration.getIntFreeMemoryKBs());
- metrics.put(externalStatsPrefix + "cloudstack.stats.instances." + vmName + ".memory.target_kbs", statsForCurrentIteration.getTargetMemoryKBs());
+ storeVirtualMachineStatsInMemory(statsForCurrentIteration);
+ if (externalStatsType == ExternalStatsProtocol.GRAPHITE) {
+ prepareVmMetricsForGraphite(metrics, statsForCurrentIteration);
+ } else {
+ metrics.put(statsForCurrentIteration.getVmId(), statsForCurrentIteration);
}
-
}
- /**
- * Send the metrics to a external stats collector
- * We send it on a per-host basis to prevent that we flood the host
- * Currently only Graphite is supported
- */
if (!metrics.isEmpty()) {
- if (externalStatsType != null && externalStatsType == ExternalStatsProtocol.GRAPHITE) {
-
- if (externalStatsPort == -1) {
- externalStatsPort = 2003;
- }
-
- s_logger.debug("Sending VmStats of host " + host.getId() + " to Graphite host " + externalStatsHost + ":" + externalStatsPort);
-
- try {
- GraphiteClient g = new GraphiteClient(externalStatsHost, externalStatsPort);
- g.sendMetrics(metrics);
- } catch (GraphiteException e) {
- s_logger.debug("Failed sending VmStats to Graphite host " + externalStatsHost + ":" + externalStatsPort + ": " + e.getMessage());
- }
-
- metrics.clear();
+ if (externalStatsType == ExternalStatsProtocol.GRAPHITE) {
+ sendVmMetricsToGraphiteHost(metrics, host);
+ } else if (externalStatsType == ExternalStatsProtocol.INFLUXDB) {
+ sendMetricsToInfluxdb(metrics);
Review comment:
@DaanHoogland sorry, the lines 600 - 603 have changed, now are 596 and 598.
I was referring to the following mthods `sendVmMetricsToGraphiteHost(metrics, host);` and `sendMetricsToInfluxdb(metrics);`
I had two options: (i) keep the Graphite metrics Map as it was, and create a new map for the influxdb (for instance metricsInfluxDB):
```
if (externalStatsType == ExternalStatsProtocol.GRAPHITE) {
sendVmMetricsToGraphiteHost(metrics, host);
} else if (externalStatsType == ExternalStatsProtocol.INFLUXDB) {
sendMetricsToInfluxdb(metricsInfluxDB);
```
Or (ii) change the metrics map from `Map<String, Integer>` to a generic `Map<Object, Object>`, which implicates on changing the GraphiteClient.java sendMetrics method, and reuse the `metrics` for InfluxDB and Graphite.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services