[pve-devel] [manager] A minor enhancement for external metrics to Influxdb and Graphite. So far with the data sent was impossible to know the node that host each lxc/qemu. This small modification allow this. It's very helpful to be able to group lxc/qemu by their host nodes when visualizing data for example with Grafana.

[Thomas Lamprecht]

Hi,

First thanks for the contribution to Proxmox VE.

I've seen two problems with the patch:
1) You patch has a style problem, you put the whole commit message in 
the first line.
Using a single short line to summarize the patch then a newline then a 
more verbose description of the patch would be great.
This makes viewing or searching the commit history much easier.

In your case it could be something like:

  > Status plugins: send nodename when updating CT/VM status
  >
  > So far with the data sent was impossible to know the node that host
  > each lxc/qemu.
  > This small modification allow this by sending also the nodename.
  > It's very helpful to be able to group lxc/qemu by their host nodes
  > when visualizing data for example with Grafana.


2) For influxdb this should be OK.
But as I have almost no experience with graphite and did not find a 
documentation regarding their path hierarchy I cannot comment for it.
What happens when I migrate a VM? With your patch the metrics are of the 
same VM get collected in another entry?
Can I view the whole VM statistic, independent of the node? Or can I 
just view the VM statistic from the time it was on node A and must 
change the query when I migrated it to node B?

cheers,
Thomas


On 02/03/2017 12:44 PM, Daniel1108 wrote:
> Regards
>
> Signed-off-by: Daniel1108 <danielgallegosanchez at gmail.com>
> ---
>   PVE/Service/pvestatd.pm | 4 ++--
>   PVE/Status/Graphite.pm  | 6 +++---
>   PVE/Status/InfluxDB.pm  | 8 ++++----
>   PVE/Status/Plugin.pm    | 4 ++--
>   4 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
> index f6aaa30..28e497c 100755
> --- a/PVE/Service/pvestatd.pm
> +++ b/PVE/Service/pvestatd.pm
> @@ -203,7 +203,7 @@ sub update_qemu_status {
>   	    my $plugin_config = $status_cfg->{ids}->{$id};
>   	    next if $plugin_config->{disable};
>   	    my $plugin = PVE::Status::Plugin->lookup($plugin_config->{type});
> -	    $plugin->update_qemu_status($plugin_config, $vmid, $d, $ctime);
> +	    $plugin->update_qemu_status($plugin_config, $vmid, $d, $ctime, $nodename);
>   	}
>       }
>   }
> @@ -403,7 +403,7 @@ sub update_lxc_status {
>   	    my $plugin_config = $status_cfg->{ids}->{$id};
>   	    next if $plugin_config->{disable};
>   	    my $plugin = PVE::Status::Plugin->lookup($plugin_config->{type});
> -	    $plugin->update_lxc_status($plugin_config, $vmid, $d, $ctime);
> +	    $plugin->update_lxc_status($plugin_config, $vmid, $d, $ctime, $nodename);
>   	}
>       }
>   }
> diff --git a/PVE/Status/Graphite.pm b/PVE/Status/Graphite.pm
> index f8d95d1..f46cb2b 100644
> --- a/PVE/Status/Graphite.pm
> +++ b/PVE/Status/Graphite.pm
> @@ -46,13 +46,13 @@ sub update_node_status {
>   
>   sub update_qemu_status {
>       my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> -    write_graphite_hash($plugin_config, $data, $ctime, "qemu.$vmid");
> +    write_graphite_hash($plugin_config, $data, $ctime, "qemu.$vmid.$nodename");
>   }
>   
>   sub update_lxc_status {
> -    my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> +    my ($class, $plugin_config, $vmid, $data, $ctime, $nodename) = @_;
>   
> -    write_graphite_hash($plugin_config, $data, $ctime, "lxc.$vmid");
> +    write_graphite_hash($plugin_config, $data, $ctime, "lxc.$vmid.$nodename");
>   }
>   
>   sub update_storage_status {
> diff --git a/PVE/Status/InfluxDB.pm b/PVE/Status/InfluxDB.pm
> index 0cce42b..7364e57 100644
> --- a/PVE/Status/InfluxDB.pm
> +++ b/PVE/Status/InfluxDB.pm
> @@ -38,11 +38,11 @@ sub update_node_status {
>   }
>   
>   sub update_qemu_status {
> -    my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> +    my ($class, $plugin_config, $vmid, $data, $ctime, $nodename) = @_;
>   
>       $ctime *= 1000000000;
>   
> -    my $object = "object=qemu,vmid=$vmid";
> +    my $object = "object=qemu,vmid=$vmid,nodename=$nodename";
>       if($data->{name} && $data->{name} ne '') {
>   	$object .= ",host=$data->{name}";
>       }
> @@ -51,11 +51,11 @@ sub update_qemu_status {
>   }
>   
>   sub update_lxc_status {
> -    my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> +    my ($class, $plugin_config, $vmid, $data, $ctime, $nodename) = @_;
>   
>       $ctime *= 1000000000;
>   
> -    my $object = "object=lxc,vmid=$vmid";
> +    my $object = "object=lxc,vmid=$vmid,nodename=$nodename";
>       if($data->{name} && $data->{name} ne '') {
>   	$object .= ",host=$data->{name}";
>       }
> diff --git a/PVE/Status/Plugin.pm b/PVE/Status/Plugin.pm
> index 070467c..ff7af89 100644
> --- a/PVE/Status/Plugin.pm
> +++ b/PVE/Status/Plugin.pm
> @@ -62,13 +62,13 @@ sub update_node_status {
>   }
>   
>   sub update_qemu_status {
> -    my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> +    my ($class, $plugin_config, $vmid, $data, $ctime, $nodename) = @_;
>   
>       die "please implement inside plugin";
>   }
>   
>   sub update_lxc_status {
> -    my ($class, $plugin_config, $vmid, $data, $ctime) = @_;
> +    my ($class, $plugin_config, $vmid, $data, $ctime, $nodename) = @_;
>   
>       die "please implement inside plugin";
>   }