[pbs-devel] [PATCH proxmox-backup] docs: add external metrics server page

[Lukas Wagner]

Hello 🙂 - some suggestions inline.

On  2024-08-05 11:42, Gabriel Goller wrote:
> Add External Metrics page to PBS's documentation. Most of it is copied
> from the PVE documentation, minus the Graphite part.
> 
> Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
> ---
>  docs/external_metric_server.rst | 57 +++++++++++++++++++++++++++++++++
>  docs/sysadmin.rst               |  2 ++
>  2 files changed, 59 insertions(+)
>  create mode 100644 docs/external_metric_server.rst
> 
> diff --git a/docs/external_metric_server.rst b/docs/external_metric_server.rst
> new file mode 100644
> index 000000000000..5dd36ea5b7e5
> --- /dev/null
> +++ b/docs/external_metric_server.rst
> @@ -0,0 +1,57 @@
> +.. _external_metric_server:
> +
> +External Metric Server
> +----------------------
> +
> +In Proxmox Backup, you can define external metric servers, which will
            ^ should be Proxmox Backup Server
> +periodically receive various stats about your host's memory, network, and disk

'periodically receive' sounds a bit like PBS is broadcasting metric data and the metric
servers are 'actively subscribed' to these broadcasts. In other words, the
active party seems to be the metric server, not PBS.
Maybe we should phrase this a bit differently to emphasize that PBS sends
the metrics to specifically configured metric servers?

Maybe:
"Proxmox Backup Server periodically sends various metrics about your host's memory,
network and disk activity to configured external metric servers."

What do you think? 

> +activity.
> +
> +Currently supported are:
> +
> + * InfluxDB (HTTP) (see https://docs.influxdata.com/influxdb/v2/ )
> + * InfluxDB (UDP) (see https://docs.influxdata.com/influxdb/v1/ )
> +
> +The external metric server definitions are saved in
> +'/etc/proxmox-backup/metricserver.cfg', and can be edited through the web
> +interface.
> +
> +.. note::
> +
> +   UDP support has been dropped in InfluxDB v2, so HTTP use is recommended.

I'd start the sentence with the actual recommendation and give the reasoning later.

"Using HTTP is recommended as UDP support has been dropped in InfluxDB v2"

Or something alike... :)

> +
> +InfluxDB (HTTP) plugin configuration
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The header line should be the same length as the text (I think you'll get a warning otherwise)

> +
> +The plugin can be configured to use the HTTP(s) API of InfluxDB 2.x.
> +InfluxDB 1.8.x does contain a forwards compatible API endpoint for this v2 API.
> +
> +Since InfluxDB's v2 API is only available with authentication, you have
> +to generate a token that can write into the correct bucket and set it.
> +
> +In the v2 compatible API of 1.8.x, you can use 'user:password' as token
> +(if required), and can omit the 'organization' since that has no meaning in InfluxDB 1.x.
> +
> +You can also set the maximum batch size (default 25000000 bytes) with the
> +'max-body-size' setting (this corresponds to the InfluxDB setting with the
> +same name).
> +
> +InfluxDB (UDP) plugin configuration
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The header line should be the same length as the text (I think you'll get a warning otherwise)
> +
> +Proxmox Backup can also send the data over UDP, but the InfluxDB server has to
> +be configured for this. The MTU can also be configured here, if necessary.

I'd split the first sentence, maybe:

"Proxmox Backup Server can also send data via UDP. This requires the InfluxDB server to be
configured correctly"

> +
> +Here is an example configuration for InfluxDB (on your InfluxDB server):
> +
> +----
> +[[udp]]
> +   enabled = true
> +   bind-address = "0.0.0.0:8089"
> +   database = "proxmox"
> +   batch-size = 1000
> +   batch-timeout = "1s"
> +----
> +
> +With this configuration, your server listens on all IP addresses on port 8089,
> +and writes the data in the *proxmox* database.

Maybe instead of "your server listens" -> "the InfluxDB server" to avoid
any confusion with the Backup Server.

> diff --git a/docs/sysadmin.rst b/docs/sysadmin.rst
> index 114ad4cbbe0e..d42b6a9b2af1 100644
> --- a/docs/sysadmin.rst
> +++ b/docs/sysadmin.rst
> @@ -30,6 +30,8 @@ please refer to the standard Debian documentation.
>  
>  .. include:: certificate-management.rst
>  
> +.. include:: external_metric_server.rst
> +
>  .. include:: services.rst
>  
>  .. include:: command-line-tools.rst

-- 
- Lukas