[GitHub] rhtyd commented on a change in pull request #3078: Add influxdb to statscollector

2019-01-09 Thread GitBox
rhtyd commented on a change in pull request #3078: Add influxdb to 
statscollector
URL: https://github.com/apache/cloudstack/pull/3078#discussion_r246449685
 
 

 ##
 File path: server/src/main/java/com/cloud/server/StatsCollector.java
 ##
 @@ -1307,14 +1322,231 @@ public String getCounternamebyCondition(long 
conditionId) {
 }
 }
 
+/**
+ * This class allows to writing metrics in InfluxDB for the table that 
matches the Collector extending it.
+ * Thus, VmStatsCollector and HostCollector can use same method to write 
on different measures (host_stats or vm_stats table).
+ */
+abstract class AbstractStatsCollector extends ManagedContextRunnable {
+/**
+ * Sends metrics to influxdb host. This method supports both VM and 
Host metrics
+ */
+protected void sendMetricsToInfluxdb(Map metrics) {
 
 Review comment:
   @GabrielBrascher @wido while it may work, I overall dislike the 
implementation approach. Instead, a pluggable interface could be implemented 
and the feature could be implemented as a `plugin` that way bloating the core 
StatCollector class could be avoided. All the influxdb related stuff could be 
at least moved to a different class of its own.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rhtyd commented on a change in pull request #3078: Add influxdb to statscollector

2019-01-09 Thread GitBox
rhtyd commented on a change in pull request #3078: Add influxdb to 
statscollector
URL: https://github.com/apache/cloudstack/pull/3078#discussion_r246448942
 
 

 ##
 File path: server/pom.xml
 ##
 @@ -157,6 +157,11 @@
 org.opensaml
 opensaml
 
+
+org.influxdb
+influxdb-java
+2.8
 
 Review comment:
   @GabrielBrascher please add a version variable in root `pom.xml`, that way 
it will be more maintainable to upgrade/check like we do for rest of the 
dependencies.


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:
us...@infra.apache.org


With regards,
Apache Git Services