Re: [pbs-devel] [PATCH proxmox/proxmox-backup v4] add metrics server capability

[Matthias Heiserer <m.heiserer@proxmox.com>]

The patch generally works (connecting with influxdb server, sending 
data, using api to create config), but some issues/suggestions:

Passing a value to the --enable in api create parameter always results 
in an error:
"Error: parameter verification errors
parameter 'enable': Expected boolean value."

Some personal preferences: I'd prefer if "api get" showed whether a 
token was set in the config, instead of leaving the field blank.

When adding a metricserver via "api create" using an already existing 
id, the old server gets overwritten. This might be 
undesired/problematic. Potential fix would be to forbid creating metric 
servers with an existing id.

On 17.01.2022 11:48, Dominik Csapak wrote:
> this series adds support for exporting metrics data to external
> metric servers.
> 
> for now this includes only data we gather for RRD, though it should
> not be hard to extend that functionality
> 
> also only influxdb (udp/http(s)) is currently supported, but it should
> also not be too hard to include more options here
> 
> i did not include gui/cli patches yet, as i find the
> proxmox-backup-manager options are already too much and i waited for
> the gui for some feedback.
> 
> for testing, the metric servers can be added either by
> calling 'proxmox-backup debug api ...' or by manually editing the
> file
> 
> ofc, proxmox-backup depends on bumped versions of the proxmox-* crates
> 
> changes from v3:
> * rebase on master
> * introduced helper functions instead of InfluxDBHttp::new
> * start tokio task directly in the helper
> * combine channel close + join
> * fix api description
> * combine host/port/protocol in the api types
> * introduce a connect_to_udp helper
> * use NixPath in the fs_info helper
> 
> changes from v2:
> * rebase on master
> * rustfmt
> * clippy (fixed not everything)
> * renamed DiskUsage in proxmox-sys and added some more fields
> * added 'enable' property for the config (like we have in pve)
> * subtracted 50bytes from mtu in the udp variant (for ip header)
> 
> changes from v1:
> * fixed ipv6 support for udp (tested it this time ;) )
> * dropped the 'flush' functionality of the MetricsChannel, but kept the
>    wrapper struct: it did not do what i intended, and after rethinking it,
>    turns out it's not necessary (as we autoflush when the data gets to large,
>    or when we close the channel). kept the struct so that the interface
>    can stay the same even if we want to implement a manual flush in the future
> * improved the influxdb line formatter
> * removed variables like 'names2' by reorganizing the code
> * used Arc::clone(&foo) instead of foo.clone() (better visibilty)
> * used CamelCase for the DeletableProperties
> 
> proxmox:
> 
> Dominik Csapak (4):
>    proxmox-sys: make some structs serializable
>    proxmox-sys: add FileSystemInformation struct and helper
>    proxmox-async: add connect_to_udp helper
>    proxmox-metrics: implement metrics server client code
> 
>   Cargo.toml                            |   1 +
>   proxmox-async/Cargo.toml              |   2 +-
>   proxmox-async/src/io/mod.rs           |   3 +
>   proxmox-async/src/io/udp_connect.rs   |  18 ++++
>   proxmox-metrics/Cargo.toml            |  21 ++++
>   proxmox-metrics/debian/changelog      |   5 +
>   proxmox-metrics/debian/copyright      |  16 ++++
>   proxmox-metrics/debian/debcargo.toml  |   7 ++
>   proxmox-metrics/src/influxdb/http.rs  | 132 ++++++++++++++++++++++++++
>   proxmox-metrics/src/influxdb/mod.rs   |   7 ++
>   proxmox-metrics/src/influxdb/udp.rs   |  80 ++++++++++++++++
>   proxmox-metrics/src/influxdb/utils.rs |  50 ++++++++++
>   proxmox-metrics/src/lib.rs            | 117 +++++++++++++++++++++++
>   proxmox-sys/Cargo.toml                |   1 +
>   proxmox-sys/src/fs/mod.rs             |  37 ++++++++
>   proxmox-sys/src/linux/procfs/mod.rs   |   7 +-
>   16 files changed, 500 insertions(+), 4 deletions(-)
>   create mode 100644 proxmox-async/src/io/udp_connect.rs
>   create mode 100644 proxmox-metrics/Cargo.toml
>   create mode 100644 proxmox-metrics/debian/changelog
>   create mode 100644 proxmox-metrics/debian/copyright
>   create mode 100644 proxmox-metrics/debian/debcargo.toml
>   create mode 100644 proxmox-metrics/src/influxdb/http.rs
>   create mode 100644 proxmox-metrics/src/influxdb/mod.rs
>   create mode 100644 proxmox-metrics/src/influxdb/udp.rs
>   create mode 100644 proxmox-metrics/src/influxdb/utils.rs
>   create mode 100644 proxmox-metrics/src/lib.rs
> 
> proxmox-backup:
> 
> Dominik Csapak (6):
>    use 'fs_info' from proxmox-sys
>    pbs-api-types: add metrics api types
>    pbs-config: add metrics config class
>    backup-proxy: decouple stats gathering from rrd update
>    proxmox-backup-proxy: send metrics to configured metrics server
>    api: add metricserver endpoints
> 
>   Cargo.toml                                   |   1 +
>   pbs-api-types/src/lib.rs                     |  15 +
>   pbs-api-types/src/metrics.rs                 | 138 ++++++++
>   pbs-config/Cargo.toml                        |   1 +
>   pbs-config/src/lib.rs                        |   1 +
>   pbs-config/src/metrics.rs                    | 115 +++++++
>   src/api2/admin/datastore.rs                  |   4 +-
>   src/api2/config/metricserver/influxdbhttp.rs | 266 +++++++++++++++
>   src/api2/config/metricserver/influxdbudp.rs  | 243 +++++++++++++
>   src/api2/config/metricserver/mod.rs          |  16 +
>   src/api2/config/mod.rs                       |   2 +
>   src/api2/node/status.rs                      |  11 +-
>   src/api2/status.rs                           |   4 +-
>   src/bin/proxmox-backup-proxy.rs              | 342 +++++++++++++++----
>   src/tools/disks/mod.rs                       |  21 +-
>   15 files changed, 1081 insertions(+), 99 deletions(-)
>   create mode 100644 pbs-api-types/src/metrics.rs
>   create mode 100644 pbs-config/src/metrics.rs
>   create mode 100644 src/api2/config/metricserver/influxdbhttp.rs
>   create mode 100644 src/api2/config/metricserver/influxdbudp.rs
>   create mode 100644 src/api2/config/metricserver/mod.rs
>