public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability
@ 2022-06-02 12:18 Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys Dominik Csapak
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

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

proxmox-backup depends on bumped versions of the proxmox-* crates

changes from v5/6:
* rebased on master

changes from v4:
* rebase on master
* move connect_to_udp to udp::connect(), and let it really try every address
* adds 'test_influxdb_http/udp' functions that try to connect for some
  sanity checks. (needed a little refactor in the http/udp parts but
  mostly code move)
* checks the server connection on create/edit via api when the enable
  flag is set
* adds a generic 'list' api call in /admin/metricserver since we need
  a place where we return *all* metrics servers regardless of type
  (and this is the way we do it e.g. for realms)
* adds the gui to view/add/edit/delete the metric servers
  (i put it under configuration, but that gets crowded... maybe there
  is a better place?)

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

Dominik Csapak (8):
  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
  ui: add window/InfluxDbEdit
  ui: add MetricServerView and use it

 Cargo.toml                                   |   1 +
 pbs-api-types/src/lib.rs                     |  17 +
 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/admin/metricserver.rs               |  91 +++++
 src/api2/admin/mod.rs                        |   2 +
 src/api2/config/metricserver/influxdbhttp.rs | 314 +++++++++++++++++
 src/api2/config/metricserver/influxdbudp.rs  | 269 +++++++++++++++
 src/api2/config/metricserver/mod.rs          |  16 +
 src/api2/config/mod.rs                       |   2 +
 src/api2/node/status.rs                      |  13 +-
 src/api2/status.rs                           |   4 +-
 src/bin/proxmox-backup-proxy.rs              | 336 +++++++++++++++----
 src/tools/disks/mod.rs                       |  20 +-
 www/Makefile                                 |   2 +
 www/NavigationTree.js                        |   6 +
 www/config/MetricServerView.js               | 145 ++++++++
 www/window/InfluxDbEdit.js                   | 218 ++++++++++++
 21 files changed, 1617 insertions(+), 98 deletions(-)
 create mode 100644 pbs-api-types/src/metrics.rs
 create mode 100644 pbs-config/src/metrics.rs
 create mode 100644 src/api2/admin/metricserver.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
 create mode 100644 www/config/MetricServerView.js
 create mode 100644 www/window/InfluxDbEdit.js

-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-07  6:51   ` [pbs-devel] applied: " Thomas Lamprecht
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 2/8] pbs-api-types: add metrics api types Dominik Csapak
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

as replacement for 'disk_usage' in tools. also remove that there since
we do not need it anymore

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/admin/datastore.rs     |  4 ++--
 src/api2/node/status.rs         | 13 ++++++++++---
 src/api2/status.rs              |  4 ++--
 src/bin/proxmox-backup-proxy.rs |  4 ++--
 src/tools/disks/mod.rs          | 20 +-------------------
 5 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 952fe2e0..84219189 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -675,11 +675,11 @@ pub fn status(
     };
 
     Ok(if store_stats {
-        let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
+        let storage = proxmox_sys::fs::fs_info(&datastore.base_path())?;
         DataStoreStatus {
             total: storage.total,
             used: storage.used,
-            avail: storage.avail,
+            avail: storage.available,
             gc_status,
             counts,
         }
diff --git a/src/api2/node/status.rs b/src/api2/node/status.rs
index da394625..3554980f 100644
--- a/src/api2/node/status.rs
+++ b/src/api2/node/status.rs
@@ -1,4 +1,3 @@
-use std::path::Path;
 use std::process::Command;
 
 use anyhow::{bail, format_err, Error};
@@ -9,7 +8,9 @@ use proxmox_sys::linux::procfs;
 use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
 use proxmox_schema::api;
 
-use pbs_api_types::{NodePowerCommand, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT};
+use pbs_api_types::{
+    NodePowerCommand, StorageStatus, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_POWER_MANAGEMENT,
+};
 
 use crate::api2::types::{
     NodeCpuInformation, NodeInformation, NodeMemoryCounters, NodeStatus, NodeSwapCounters,
@@ -77,10 +78,16 @@ fn get_status(
         uname.version()
     );
 
+    let disk = proxmox_sys::fs::fs_info(proxmox_lang::c_str!("/"))?;
+
     Ok(NodeStatus {
         memory,
         swap,
-        root: crate::tools::disks::disk_usage(Path::new("/"))?,
+        root: StorageStatus {
+            total: disk.total,
+            used: disk.used,
+            avail: disk.available,
+        },
         uptime: procfs::read_proc_uptime()?.0 as u64,
         loadavg,
         kversion,
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 55c811a5..8b97bef9 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -64,13 +64,13 @@ pub fn datastore_status(
                 continue;
             }
         };
-        let status = crate::tools::disks::disk_usage(&datastore.base_path())?;
+        let status = proxmox_sys::fs::fs_info(&datastore.base_path())?;
 
         let mut entry = DataStoreStatusListItem {
             store: store.clone(),
             total: status.total as i64,
             used: status.used as i64,
-            avail: status.avail as i64,
+            avail: status.available as i64,
             history: None,
             history_start: None,
             history_delta: None,
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index b45c3456..bf4b62f6 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -1126,7 +1126,7 @@ fn check_schedule(worker_type: &str, event_str: &str, id: &str) -> bool {
 }
 
 fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, rrd_prefix: &str) {
-    match proxmox_backup::tools::disks::disk_usage(path) {
+    match proxmox_sys::fs::fs_info(path) {
         Ok(status) => {
             let rrd_key = format!("{}/total", rrd_prefix);
             rrd_update_gauge(&rrd_key, status.total as f64);
@@ -1134,7 +1134,7 @@ fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, rrd_prefix: &st
             rrd_update_gauge(&rrd_key, status.used as f64);
         }
         Err(err) => {
-            eprintln!("read disk_usage on {:?} failed - {}", path, err);
+            eprintln!("read fs info on {:?} failed - {}", path, err);
         }
     }
 
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 568dccbf..77635fc4 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -19,7 +19,7 @@ use proxmox_lang::{io_bail, io_format_err};
 use proxmox_schema::api;
 use proxmox_sys::linux::procfs::{mountinfo::Device, MountInfo};
 
-use pbs_api_types::{StorageStatus, BLOCKDEVICE_NAME_REGEX};
+use pbs_api_types::BLOCKDEVICE_NAME_REGEX;
 
 mod zfs;
 pub use zfs::*;
@@ -528,24 +528,6 @@ impl Disk {
     }
 }
 
-/// Returns disk usage information (total, used, avail)
-pub fn disk_usage(path: &std::path::Path) -> Result<StorageStatus, Error> {
-    let mut stat: libc::statfs64 = unsafe { std::mem::zeroed() };
-
-    use nix::NixPath;
-
-    let res = path.with_nix_path(|cstr| unsafe { libc::statfs64(cstr.as_ptr(), &mut stat) })?;
-    nix::errno::Errno::result(res)?;
-
-    let bsize = stat.f_bsize as u64;
-
-    Ok(StorageStatus {
-        total: stat.f_blocks * bsize,
-        used: (stat.f_blocks - stat.f_bfree) * bsize,
-        avail: stat.f_bavail * bsize,
-    })
-}
-
 #[api()]
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "lowercase")]
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 2/8] pbs-api-types: add metrics api types
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class Dominik Csapak
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

InfluxDbUdp and InfluxDbHttp for now

introduces schemas for host:port and https urls

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/lib.rs     |  17 +++++
 pbs-api-types/src/metrics.rs | 138 +++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+)
 create mode 100644 pbs-api-types/src/metrics.rs

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index d9c8cee1..70c9ec45 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -120,6 +120,9 @@ pub use traffic_control::*;
 mod zfs;
 pub use zfs::*;
 
+mod metrics;
+pub use metrics::*;
+
 #[rustfmt::skip]
 #[macro_use]
 mod local_macros {
@@ -131,6 +134,7 @@ mod local_macros {
     macro_rules! DNS_ALIAS_NAME {
         () => (concat!(r"(?:(?:", DNS_ALIAS_LABEL!() , r"\.)*", DNS_ALIAS_LABEL!(), ")"))
     }
+    macro_rules! PORT_REGEX_STR { () => (r"(?:[0-9]{1,4}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])") }
 }
 
 const_regex! {
@@ -144,6 +148,8 @@ const_regex! {
     pub DNS_NAME_REGEX =  concat!(r"^", DNS_NAME!(), r"$");
     pub DNS_ALIAS_REGEX =  concat!(r"^", DNS_ALIAS_NAME!(), r"$");
     pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|",  IPRE!(), r")$");
+    pub HOST_PORT_REGEX = concat!(r"^(?:", DNS_NAME!(), "|", IPRE_BRACKET!(), "):", PORT_REGEX_STR!() ,"$");
+    pub HTTP_URL_REGEX = concat!(r"^https?://(?:(?:(?:", DNS_NAME!(), "|", IPRE_BRACKET!(), ")(?::", PORT_REGEX_STR!() ,")?)|", IPV6RE!(),")(?:/[^\x00-\x1F\x7F]*)?$");
 
     pub SHA256_HEX_REGEX = r"^[a-f0-9]{64}$"; // fixme: define in common_regex ?
 
@@ -201,6 +207,8 @@ pub const SYSTEMD_DATETIME_FORMAT: ApiStringFormat =
 pub const HOSTNAME_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&HOSTNAME_REGEX);
 pub const OPENSSL_CIPHERS_TLS_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&OPENSSL_CIPHERS_REGEX);
+pub const HOST_PORT_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&HOST_PORT_REGEX);
+pub const HTTP_URL_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&HTTP_URL_REGEX);
 
 pub const DNS_ALIAS_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&DNS_ALIAS_REGEX);
 
@@ -244,6 +252,15 @@ pub const DNS_NAME_OR_IP_SCHEMA: Schema = StringSchema::new("DNS name or IP addr
     .format(&DNS_NAME_OR_IP_FORMAT)
     .schema();
 
+pub const HOST_PORT_SCHEMA: Schema =
+    StringSchema::new("host:port combination (Host can be DNS name or IP address).")
+        .format(&HOST_PORT_FORMAT)
+        .schema();
+
+pub const HTTP_URL_SCHEMA: Schema = StringSchema::new("HTTP(s) url with optional port.")
+    .format(&HTTP_URL_FORMAT)
+    .schema();
+
 pub const NODE_SCHEMA: Schema = StringSchema::new("Node name (or 'localhost')")
     .format(&HOSTNAME_FORMAT)
     .schema();
diff --git a/pbs-api-types/src/metrics.rs b/pbs-api-types/src/metrics.rs
new file mode 100644
index 00000000..239d6c80
--- /dev/null
+++ b/pbs-api-types/src/metrics.rs
@@ -0,0 +1,138 @@
+use serde::{Deserialize, Serialize};
+
+use crate::{
+    HOST_PORT_SCHEMA, HTTP_URL_SCHEMA, PROXMOX_SAFE_ID_FORMAT, SINGLE_LINE_COMMENT_SCHEMA,
+};
+use proxmox_schema::{api, Schema, StringSchema, Updater};
+
+pub const METRIC_SERVER_ID_SCHEMA: Schema = StringSchema::new("Metrics Server ID.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(3)
+    .max_length(32)
+    .schema();
+
+pub const INFLUXDB_BUCKET_SCHEMA: Schema = StringSchema::new("InfluxDB Bucket.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(3)
+    .max_length(32)
+    .default("proxmox")
+    .schema();
+
+pub const INFLUXDB_ORGANIZATION_SCHEMA: Schema = StringSchema::new("InfluxDB Organization.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(3)
+    .max_length(32)
+    .default("proxmox")
+    .schema();
+
+#[api(
+    properties: {
+        name: {
+            schema: METRIC_SERVER_ID_SCHEMA,
+        },
+        enable: {
+            type: bool,
+            optional: true,
+            default: true,
+        },
+        host: {
+            schema: HOST_PORT_SCHEMA,
+        },
+        mtu: {
+            type: u16,
+            optional: true,
+            default: 1500,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// InfluxDB Server (UDP)
+pub struct InfluxDbUdp {
+    #[updater(skip)]
+    pub name: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// Enables or disables the metrics server
+    pub enable: Option<bool>,
+    /// the host + port
+    pub host: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// The MTU
+    pub mtu: Option<u16>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+}
+
+#[api(
+    properties: {
+        name: {
+            schema: METRIC_SERVER_ID_SCHEMA,
+        },
+        enable: {
+            type: bool,
+            optional: true,
+            default: true,
+        },
+        url: {
+            schema: HTTP_URL_SCHEMA,
+        },
+        token: {
+            type: String,
+            optional: true,
+        },
+        bucket: {
+            schema: INFLUXDB_BUCKET_SCHEMA,
+            optional: true,
+        },
+        organization: {
+            schema: INFLUXDB_ORGANIZATION_SCHEMA,
+            optional: true,
+        },
+        "max-body-size": {
+            type: usize,
+            optional: true,
+            default: 25_000_000,
+        },
+        "verify-tls": {
+            type: bool,
+            optional: true,
+            default: true,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// InfluxDB Server (HTTP(s))
+pub struct InfluxDbHttp {
+    #[updater(skip)]
+    pub name: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// Enables or disables the metrics server
+    pub enable: Option<bool>,
+    /// The base url of the influxdb server
+    pub url: String,
+    /// The Optional Token
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// The (optional) API token
+    pub token: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub bucket: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub organization: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// The (optional) maximum body size
+    pub max_body_size: Option<usize>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// If true, the certificate will be validated.
+    pub verify_tls: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 2/8] pbs-api-types: add metrics api types Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-03 10:16   ` Wolfgang Bumiller
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update Dominik Csapak
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

a section config like in pve

also adds a helper to get Metrics structs for all configured servers

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 Cargo.toml                |   1 +
 pbs-config/Cargo.toml     |   1 +
 pbs-config/src/lib.rs     |   1 +
 pbs-config/src/metrics.rs | 115 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+)
 create mode 100644 pbs-config/src/metrics.rs

diff --git a/Cargo.toml b/Cargo.toml
index 9776a5ee..4a77f10e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -96,6 +96,7 @@ pxar = { version = "0.10.1", features = [ "tokio-io" ] }
 proxmox-http = { version = "0.6.1", features = [ "client", "http-helpers", "websocket" ] }
 proxmox-io = "1"
 proxmox-lang = "1.1"
+proxmox-metrics = "0.1"
 proxmox-router = { version = "1.2.2", features = [ "cli" ] }
 proxmox-schema = { version = "1.3.1", features = [ "api-macro" ] }
 proxmox-section-config = "1"
diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml
index 1e43bd9f..d367321b 100644
--- a/pbs-config/Cargo.toml
+++ b/pbs-config/Cargo.toml
@@ -25,6 +25,7 @@ proxmox-time = "1"
 proxmox-serde = "0.1"
 proxmox-shared-memory = "0.2"
 proxmox-sys = "0.2"
+proxmox-metrics = "0.1"
 
 pbs-api-types = { path = "../pbs-api-types" }
 pbs-buildcfg = { path = "../pbs-buildcfg" }
diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
index a6627caa..a83db4e1 100644
--- a/pbs-config/src/lib.rs
+++ b/pbs-config/src/lib.rs
@@ -6,6 +6,7 @@ pub mod domains;
 pub mod drive;
 pub mod key_config;
 pub mod media_pool;
+pub mod metrics;
 pub mod network;
 pub mod prune;
 pub mod remote;
diff --git a/pbs-config/src/metrics.rs b/pbs-config/src/metrics.rs
new file mode 100644
index 00000000..3d4428f8
--- /dev/null
+++ b/pbs-config/src/metrics.rs
@@ -0,0 +1,115 @@
+use std::collections::HashMap;
+
+use anyhow::Error;
+use lazy_static::lazy_static;
+
+use proxmox_metrics::{influxdb_http, influxdb_udp, Metrics};
+use proxmox_schema::*;
+use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
+
+use pbs_api_types::{InfluxDbHttp, InfluxDbUdp, METRIC_SERVER_ID_SCHEMA};
+
+use crate::{open_backup_lockfile, BackupLockGuard};
+
+lazy_static! {
+    pub static ref CONFIG: SectionConfig = init();
+}
+
+fn init() -> SectionConfig {
+    let mut config = SectionConfig::new(&METRIC_SERVER_ID_SCHEMA);
+
+    let udp_schema = match InfluxDbUdp::API_SCHEMA {
+        Schema::Object(ref object_schema) => object_schema,
+        _ => unreachable!(),
+    };
+
+    let udp_plugin = SectionConfigPlugin::new(
+        "influxdb-udp".to_string(),
+        Some("name".to_string()),
+        udp_schema,
+    );
+    config.register_plugin(udp_plugin);
+
+    let http_schema = match InfluxDbHttp::API_SCHEMA {
+        Schema::Object(ref object_schema) => object_schema,
+        _ => unreachable!(),
+    };
+
+    let http_plugin = SectionConfigPlugin::new(
+        "influxdb-http".to_string(),
+        Some("name".to_string()),
+        http_schema,
+    );
+
+    config.register_plugin(http_plugin);
+
+    config
+}
+
+pub const METRIC_SERVER_CFG_FILENAME: &str = "/etc/proxmox-backup/metricserver.cfg";
+pub const METRIC_SERVER_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.metricserver.lck";
+
+/// Get exclusive lock
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+    open_backup_lockfile(METRIC_SERVER_CFG_LOCKFILE, None, true)
+}
+
+pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
+    let content = proxmox_sys::fs::file_read_optional_string(METRIC_SERVER_CFG_FILENAME)?
+        .unwrap_or_else(|| "".to_string());
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = CONFIG.parse(METRIC_SERVER_CFG_FILENAME, &content)?;
+    Ok((data, digest))
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let raw = CONFIG.write(METRIC_SERVER_CFG_FILENAME, config)?;
+    crate::replace_backup_config(METRIC_SERVER_CFG_FILENAME, raw.as_bytes())
+}
+
+// shell completion helper
+pub fn complete_remote_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(),
+        Err(_) => return vec![],
+    }
+}
+
+/// Get the metric server connections from a config
+pub fn get_metric_server_connections(
+    metric_config: SectionConfigData,
+) -> Result<(Vec<Metrics>, Vec<String>), Error> {
+    let mut futures = Vec::new();
+    let mut names = Vec::new();
+
+    for config in
+        metric_config.convert_to_typed_array::<pbs_api_types::InfluxDbUdp>("influxdb-udp")?
+    {
+        if !config.enable.unwrap_or(true) {
+            continue;
+        }
+        let future = influxdb_udp(&config.host, config.mtu);
+        names.push(config.name);
+        futures.push(future);
+    }
+
+    for config in
+        metric_config.convert_to_typed_array::<pbs_api_types::InfluxDbHttp>("influxdb-http")?
+    {
+        if !config.enable.unwrap_or(true) {
+            continue;
+        }
+        let future = influxdb_http(
+            &config.url,
+            config.organization.as_deref().unwrap_or("proxmox"),
+            config.bucket.as_deref().unwrap_or("proxmox"),
+            config.token.as_deref(),
+            config.verify_tls.unwrap_or(true),
+            config.max_body_size.unwrap_or(25_000_000),
+        )?;
+        names.push(config.name);
+        futures.push(future);
+    }
+    Ok((futures, names))
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
                   ` (2 preceding siblings ...)
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-02 14:09   ` Matthias Heiserer
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 5/8] proxmox-backup-proxy: send metrics to configured metrics server Dominik Csapak
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

that way we can reuse the stats gathered

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 216 +++++++++++++++++++++-----------
 1 file changed, 144 insertions(+), 72 deletions(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index bf4b62f6..5699f6cc 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -20,8 +20,11 @@ use tokio_stream::wrappers::ReceiverStream;
 use proxmox_http::client::{RateLimitedStream, ShareableRateLimit};
 use proxmox_lang::try_block;
 use proxmox_router::{RpcEnvironment, RpcEnvironmentType, UserInformation};
-use proxmox_sys::fs::CreateOptions;
-use proxmox_sys::linux::socket::set_tcp_keepalive;
+use proxmox_sys::fs::{CreateOptions, FileSystemInformation};
+use proxmox_sys::linux::{
+    procfs::{Loadavg, ProcFsMemInfo, ProcFsNetDev, ProcFsStat},
+    socket::set_tcp_keepalive,
+};
 use proxmox_sys::logrotate::LogRotate;
 use proxmox_sys::{task_log, task_warn};
 
@@ -40,6 +43,7 @@ use proxmox_backup::{
         auth::check_pbs_auth,
         jobstate::{self, Job},
     },
+    tools::disks::BlockDevStat,
     traffic_control_cache::TRAFFIC_CONTROL_CACHE,
 };
 
@@ -993,81 +997,94 @@ async fn run_stat_generator() {
     loop {
         let delay_target = Instant::now() + Duration::from_secs(10);
 
-        generate_host_stats().await;
+        let stats = match tokio::task::spawn_blocking(|| {
+            let hoststats = collect_host_stats_sync();
+            let (hostdisk, datastores) = collect_disk_stats_sync();
+            Arc::new((hoststats, hostdisk, datastores))
+        })
+        .await
+        {
+            Ok(res) => res,
+            Err(err) => {
+                log::error!("collecting host stats paniced: {}", err);
+                tokio::time::sleep_until(tokio::time::Instant::from_std(delay_target)).await;
+                continue;
+            }
+        };
 
-        rrd_sync_journal();
+        let rrd_future = tokio::task::spawn_blocking(move || {
+            rrd_update_host_stats_sync(&stats.0, &stats.1, &stats.2);
+            rrd_sync_journal();
+        });
 
         tokio::time::sleep_until(tokio::time::Instant::from_std(delay_target)).await;
     }
 }
 
-async fn generate_host_stats() {
-    match tokio::task::spawn_blocking(generate_host_stats_sync).await {
-        Ok(()) => (),
-        Err(err) => log::error!("generate_host_stats paniced: {}", err),
-    }
+struct HostStats {
+    proc: Option<ProcFsStat>,
+    meminfo: Option<ProcFsMemInfo>,
+    net: Option<Vec<ProcFsNetDev>>,
+    load: Option<Loadavg>,
+}
+
+struct DiskStat {
+    name: String,
+    usage: Option<FileSystemInformation>,
+    dev: Option<BlockDevStat>,
 }
 
-fn generate_host_stats_sync() {
+fn collect_host_stats_sync() -> HostStats {
     use proxmox_sys::linux::procfs::{
         read_loadavg, read_meminfo, read_proc_net_dev, read_proc_stat,
     };
 
-    match read_proc_stat() {
-        Ok(stat) => {
-            rrd_update_gauge("host/cpu", stat.cpu);
-            rrd_update_gauge("host/iowait", stat.iowait_percent);
-        }
+    let proc = match read_proc_stat() {
+        Ok(stat) => Some(stat),
         Err(err) => {
             eprintln!("read_proc_stat failed - {}", err);
+            None
         }
-    }
+    };
 
-    match read_meminfo() {
-        Ok(meminfo) => {
-            rrd_update_gauge("host/memtotal", meminfo.memtotal as f64);
-            rrd_update_gauge("host/memused", meminfo.memused as f64);
-            rrd_update_gauge("host/swaptotal", meminfo.swaptotal as f64);
-            rrd_update_gauge("host/swapused", meminfo.swapused as f64);
-        }
+    let meminfo = match read_meminfo() {
+        Ok(stat) => Some(stat),
         Err(err) => {
             eprintln!("read_meminfo failed - {}", err);
+            None
         }
-    }
+    };
 
-    match read_proc_net_dev() {
-        Ok(netdev) => {
-            use pbs_config::network::is_physical_nic;
-            let mut netin = 0;
-            let mut netout = 0;
-            for item in netdev {
-                if !is_physical_nic(&item.device) {
-                    continue;
-                }
-                netin += item.receive;
-                netout += item.send;
-            }
-            rrd_update_derive("host/netin", netin as f64);
-            rrd_update_derive("host/netout", netout as f64);
-        }
+    let net = match read_proc_net_dev() {
+        Ok(netdev) => Some(netdev),
         Err(err) => {
             eprintln!("read_prox_net_dev failed - {}", err);
+            None
         }
-    }
+    };
 
-    match read_loadavg() {
-        Ok(loadavg) => {
-            rrd_update_gauge("host/loadavg", loadavg.0 as f64);
-        }
+    let load = match read_loadavg() {
+        Ok(loadavg) => Some(loadavg),
         Err(err) => {
             eprintln!("read_loadavg failed - {}", err);
+            None
         }
+    };
+
+    HostStats {
+        proc,
+        meminfo,
+        net,
+        load,
     }
+}
 
+fn collect_disk_stats_sync() -> (DiskStat, Vec<DiskStat>) {
     let disk_manager = DiskManage::new();
 
-    gather_disk_stats(disk_manager.clone(), Path::new("/"), "host");
+    let root = gather_disk_stats(disk_manager.clone(), Path::new("/"), "host");
 
+    let mut datastores = Vec::new();
     match pbs_config::datastore::config() {
         Ok((config, _)) => {
             let datastore_list: Vec<DataStoreConfig> = config
@@ -1081,15 +1098,80 @@ fn generate_host_stats_sync() {
                 {
                     continue;
                 }
-                let rrd_prefix = format!("datastore/{}", config.name);
                 let path = std::path::Path::new(&config.path);
-                gather_disk_stats(disk_manager.clone(), path, &rrd_prefix);
+                datastores.push(gather_disk_stats(disk_manager.clone(), path, &config.name));
             }
         }
         Err(err) => {
             eprintln!("read datastore config failed - {}", err);
         }
     }
+
+    (root, datastores)
+}
+
+fn rrd_update_host_stats_sync(host: &HostStats, hostdisk: &DiskStat, datastores: &[DiskStat]) {
+    if let Some(stat) = &host.proc {
+        rrd_update_gauge("host/cpu", stat.cpu);
+        rrd_update_gauge("host/iowait", stat.iowait_percent);
+    }
+
+    if let Some(meminfo) = &host.meminfo {
+        rrd_update_gauge("host/memtotal", meminfo.memtotal as f64);
+        rrd_update_gauge("host/memused", meminfo.memused as f64);
+        rrd_update_gauge("host/swaptotal", meminfo.swaptotal as f64);
+        rrd_update_gauge("host/swapused", meminfo.swapused as f64);
+    }
+
+    if let Some(netdev) = &host.net {
+        use pbs_config::network::is_physical_nic;
+        let mut netin = 0;
+        let mut netout = 0;
+        for item in netdev {
+            if !is_physical_nic(&item.device) {
+                continue;
+            }
+            netin += item.receive;
+            netout += item.send;
+        }
+        rrd_update_derive("host/netin", netin as f64);
+        rrd_update_derive("host/netout", netout as f64);
+    }
+
+    if let Some(loadavg) = &host.load {
+        rrd_update_gauge("host/loadavg", loadavg.0 as f64);
+    }
+
+    rrd_update_disk_stat(&hostdisk, "host");
+
+    for stat in datastores {
+        let rrd_prefix = format!("datastore/{}", stat.name);
+        rrd_update_disk_stat(&stat, &rrd_prefix);
+    }
+}
+
+fn rrd_update_disk_stat(disk: &DiskStat, rrd_prefix: &str) {
+    if let Some(status) = &disk.usage {
+        let rrd_key = format!("{}/total", rrd_prefix);
+        rrd_update_gauge(&rrd_key, status.total as f64);
+        let rrd_key = format!("{}/used", rrd_prefix);
+        rrd_update_gauge(&rrd_key, status.used as f64);
+    }
+
+    if let Some(stat) = &disk.dev {
+        let rrd_key = format!("{}/read_ios", rrd_prefix);
+        rrd_update_derive(&rrd_key, stat.read_ios as f64);
+        let rrd_key = format!("{}/read_bytes", rrd_prefix);
+        rrd_update_derive(&rrd_key, (stat.read_sectors * 512) as f64);
+
+        let rrd_key = format!("{}/write_ios", rrd_prefix);
+        rrd_update_derive(&rrd_key, stat.write_ios as f64);
+        let rrd_key = format!("{}/write_bytes", rrd_prefix);
+        rrd_update_derive(&rrd_key, (stat.write_sectors * 512) as f64);
+
+        let rrd_key = format!("{}/io_ticks", rrd_prefix);
+        rrd_update_derive(&rrd_key, (stat.io_ticks as f64) / 1000.0);
+    }
 }
 
 fn check_schedule(worker_type: &str, event_str: &str, id: &str) -> bool {
@@ -1125,21 +1207,17 @@ fn check_schedule(worker_type: &str, event_str: &str, id: &str) -> bool {
     next <= now
 }
 
-fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, rrd_prefix: &str) {
-    match proxmox_sys::fs::fs_info(path) {
-        Ok(status) => {
-            let rrd_key = format!("{}/total", rrd_prefix);
-            rrd_update_gauge(&rrd_key, status.total as f64);
-            let rrd_key = format!("{}/used", rrd_prefix);
-            rrd_update_gauge(&rrd_key, status.used as f64);
-        }
+fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, name: &str) -> DiskStat {
+    let usage = match proxmox_sys::fs::fs_info(path) {
+        Ok(status) => Some(status),
         Err(err) => {
             eprintln!("read fs info on {:?} failed - {}", path, err);
+            None
         }
-    }
+    };
 
-    match disk_manager.find_mounted_device(path) {
-        Ok(None) => {}
+    let dev = match disk_manager.find_mounted_device(path) {
+        Ok(None) => None,
         Ok(Some((fs_type, device, source))) => {
             let mut device_stat = None;
             match (fs_type.as_str(), source) {
@@ -1161,24 +1239,18 @@ fn gather_disk_stats(disk_manager: Arc<DiskManage>, path: &Path, rrd_prefix: &st
                     }
                 }
             }
-            if let Some(stat) = device_stat {
-                let rrd_key = format!("{}/read_ios", rrd_prefix);
-                rrd_update_derive(&rrd_key, stat.read_ios as f64);
-                let rrd_key = format!("{}/read_bytes", rrd_prefix);
-                rrd_update_derive(&rrd_key, (stat.read_sectors * 512) as f64);
-
-                let rrd_key = format!("{}/write_ios", rrd_prefix);
-                rrd_update_derive(&rrd_key, stat.write_ios as f64);
-                let rrd_key = format!("{}/write_bytes", rrd_prefix);
-                rrd_update_derive(&rrd_key, (stat.write_sectors * 512) as f64);
-
-                let rrd_key = format!("{}/io_ticks", rrd_prefix);
-                rrd_update_derive(&rrd_key, (stat.io_ticks as f64) / 1000.0);
-            }
+            device_stat
         }
         Err(err) => {
             eprintln!("find_mounted_device failed - {}", err);
+            None
         }
+    };
+
+    DiskStat {
+        name: name.to_string(),
+        usage,
+        dev,
     }
 }
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 5/8] proxmox-backup-proxy: send metrics to configured metrics server
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
                   ` (3 preceding siblings ...)
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints Dominik Csapak
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

and keep the data as similar as possible to pve (tags/fields)

datastores get their own 'object' type and reside in the "blockstat"
measurement

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 126 +++++++++++++++++++++++++++++++-
 1 file changed, 123 insertions(+), 3 deletions(-)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 5699f6cc..88dc2fd5 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -19,6 +19,7 @@ use tokio_stream::wrappers::ReceiverStream;
 
 use proxmox_http::client::{RateLimitedStream, ShareableRateLimit};
 use proxmox_lang::try_block;
+use proxmox_metrics::MetricsData;
 use proxmox_router::{RpcEnvironment, RpcEnvironmentType, UserInformation};
 use proxmox_sys::fs::{CreateOptions, FileSystemInformation};
 use proxmox_sys::linux::{
@@ -28,6 +29,7 @@ use proxmox_sys::linux::{
 use proxmox_sys::logrotate::LogRotate;
 use proxmox_sys::{task_log, task_warn};
 
+use pbs_config::metrics::get_metric_server_connections;
 use pbs_datastore::DataStore;
 
 use proxmox_rest_server::{
@@ -1012,15 +1014,113 @@ async fn run_stat_generator() {
             }
         };
 
-        let rrd_future = tokio::task::spawn_blocking(move || {
-            rrd_update_host_stats_sync(&stats.0, &stats.1, &stats.2);
-            rrd_sync_journal();
+        let rrd_future = tokio::task::spawn_blocking({
+            let stats = Arc::clone(&stats);
+            move || {
+                rrd_update_host_stats_sync(&stats.0, &stats.1, &stats.2);
+                rrd_sync_journal();
+            }
         });
 
+        let metrics_future = send_data_to_metric_servers(stats);
+
+        let (rrd_res, metrics_res) = join!(rrd_future, metrics_future);
+        if let Err(err) = rrd_res {
+            log::error!("rrd update panicked: {}", err);
+        }
+        if let Err(err) = metrics_res {
+            log::error!("error during metrics sending: {}", err);
+        }
+
         tokio::time::sleep_until(tokio::time::Instant::from_std(delay_target)).await;
     }
 }
 
+async fn send_data_to_metric_servers(
+    stats: Arc<(HostStats, DiskStat, Vec<DiskStat>)>,
+) -> Result<(), Error> {
+    let (config, _digest) = pbs_config::metrics::config()?;
+    let (channels, names) = get_metric_server_connections(config)?;
+
+    if channels.is_empty() {
+        return Ok(());
+    }
+
+    let ctime = proxmox_time::epoch_i64();
+    let nodename = proxmox_sys::nodename();
+
+    let mut values = Vec::new();
+
+    let mut cpuvalue = match &stats.0.proc {
+        Some(stat) => serde_json::to_value(stat)?,
+        None => json!({}),
+    };
+
+    if let Some(loadavg) = &stats.0.load {
+        cpuvalue["avg1"] = Value::from(loadavg.0);
+        cpuvalue["avg5"] = Value::from(loadavg.1);
+        cpuvalue["avg15"] = Value::from(loadavg.2);
+    }
+
+    values.push(Arc::new(
+        MetricsData::new("cpustat", ctime, cpuvalue)?
+            .tag("object", "host")
+            .tag("host", nodename),
+    ));
+
+    if let Some(stat) = &stats.0.meminfo {
+        values.push(Arc::new(
+            MetricsData::new("memory", ctime, stat)?
+                .tag("object", "host")
+                .tag("host", nodename),
+        ));
+    }
+
+    if let Some(netdev) = &stats.0.net {
+        for item in netdev {
+            values.push(Arc::new(
+                MetricsData::new("nics", ctime, item)?
+                    .tag("object", "host")
+                    .tag("host", nodename)
+                    .tag("instance", item.device.clone()),
+            ));
+        }
+    }
+
+    values.push(Arc::new(
+        MetricsData::new("blockstat", ctime, stats.1.to_value())?
+            .tag("object", "host")
+            .tag("host", nodename),
+    ));
+
+    for datastore in stats.2.iter() {
+        values.push(Arc::new(
+            MetricsData::new("blockstat", ctime, datastore.to_value())?
+                .tag("object", "host")
+                .tag("host", nodename)
+                .tag("datastore", datastore.name.clone()),
+        ));
+    }
+
+    let results = proxmox_metrics::send_data_to_channels(&values, &channels).await;
+    for (res, name) in results.into_iter().zip(names.iter()) {
+        if let Err(err) = res {
+            log::error!("error sending into channel of {}: {}", name, err);
+        }
+    }
+
+    futures::future::join_all(channels.into_iter().zip(names.into_iter()).map(
+        |(channel, name)| async move {
+            if let Err(err) = channel.join().await {
+                log::error!("error sending to metric server {}: {}", name, err);
+            }
+        },
+    ))
+    .await;
+
+    Ok(())
+}
+
 struct HostStats {
     proc: Option<ProcFsStat>,
     meminfo: Option<ProcFsMemInfo>,
@@ -1034,6 +1134,26 @@ struct DiskStat {
     dev: Option<BlockDevStat>,
 }
 
+impl DiskStat {
+    fn to_value(&self) -> Value {
+        let mut value = json!({});
+        if let Some(usage) = &self.usage {
+            value["total"] = Value::from(usage.total);
+            value["used"] = Value::from(usage.used);
+            value["avail"] = Value::from(usage.available);
+        }
+
+        if let Some(dev) = &self.dev {
+            value["read_ios"] = Value::from(dev.read_ios);
+            value["read_bytes"] = Value::from(dev.read_sectors * 512);
+            value["write_ios"] = Value::from(dev.write_ios);
+            value["write_bytes"] = Value::from(dev.write_sectors * 512);
+            value["io_ticks"] = Value::from(dev.io_ticks / 1000);
+        }
+        value
+    }
+}
+
 fn collect_host_stats_sync() -> HostStats {
     use proxmox_sys::linux::procfs::{
         read_loadavg, read_meminfo, read_proc_net_dev, read_proc_stat,
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
                   ` (4 preceding siblings ...)
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 5/8] proxmox-backup-proxy: send metrics to configured metrics server Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-07 12:44   ` Thomas Lamprecht
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 7/8] ui: add window/InfluxDbEdit Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it Dominik Csapak
  7 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

but in contrast to pve, we split the api by type of the section config,
since we cannot handle multiple types in the updater

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/admin/metricserver.rs               |  91 ++++++
 src/api2/admin/mod.rs                        |   2 +
 src/api2/config/metricserver/influxdbhttp.rs | 314 +++++++++++++++++++
 src/api2/config/metricserver/influxdbudp.rs  | 269 ++++++++++++++++
 src/api2/config/metricserver/mod.rs          |  16 +
 src/api2/config/mod.rs                       |   2 +
 6 files changed, 694 insertions(+)
 create mode 100644 src/api2/admin/metricserver.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

diff --git a/src/api2/admin/metricserver.rs b/src/api2/admin/metricserver.rs
new file mode 100644
index 00000000..728d1599
--- /dev/null
+++ b/src/api2/admin/metricserver.rs
@@ -0,0 +1,91 @@
+use anyhow::Error;
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{METRIC_SERVER_ID_SCHEMA, PRIV_SYS_AUDIT, SINGLE_LINE_COMMENT_SCHEMA};
+use pbs_config::metrics;
+
+#[api]
+#[derive(Deserialize, Serialize, PartialEq, Eq)]
+/// Type of the metric server
+pub enum MetricServerType {
+    /// InfluxDB HTTP
+    #[serde(rename = "influxdb-http")]
+    InfluxDbHttp,
+    /// InfluxDB UDP
+    #[serde(rename = "influxdb-udp")]
+    InfluxDbUdp,
+}
+
+#[api(
+    properties: {
+        name: {
+            schema: METRIC_SERVER_ID_SCHEMA,
+        },
+        "type": {
+            type: MetricServerType,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Basic information about a metric server thats available for all types
+pub struct MetricServerInfo {
+    pub name: String,
+    #[serde(rename = "type")]
+    pub ty: MetricServerType,
+    /// Enables or disables the metrics server
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enable: Option<bool>,
+    /// The target server
+    pub server: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+}
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List of configured metric servers.",
+        type: Array,
+        items: { type: MetricServerInfo },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// List configured metric servers.
+pub fn list_metric_servers(
+    _param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<MetricServerInfo>, Error> {
+    let (config, digest) = metrics::config()?;
+    let mut list = Vec::new();
+
+    for (_, (section_type, v)) in config.sections.iter() {
+        let mut entry = v.clone();
+        entry["type"] = Value::from(section_type.clone());
+        if entry.get("url").is_some() {
+            entry["server"] = entry["url"].clone();
+        }
+        if entry.get("host").is_some() {
+            entry["server"] = entry["host"].clone();
+        }
+        list.push(serde_json::from_value(entry)?);
+    }
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(list)
+}
+
+pub const ROUTER: Router = Router::new().get(&API_METHOD_LIST_METRIC_SERVERS);
diff --git a/src/api2/admin/mod.rs b/src/api2/admin/mod.rs
index d5a2c527..ae3c1875 100644
--- a/src/api2/admin/mod.rs
+++ b/src/api2/admin/mod.rs
@@ -5,6 +5,7 @@ use proxmox_router::{Router, SubdirMap};
 use proxmox_sys::sortable;
 
 pub mod datastore;
+pub mod metricserver;
 pub mod namespace;
 pub mod prune;
 pub mod sync;
@@ -14,6 +15,7 @@ pub mod verify;
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     ("datastore", &datastore::ROUTER),
+    ("metricserver", &metricserver::ROUTER),
     ("prune", &prune::ROUTER),
     ("sync", &sync::ROUTER),
     ("traffic-control", &traffic_control::ROUTER),
diff --git a/src/api2/config/metricserver/influxdbhttp.rs b/src/api2/config/metricserver/influxdbhttp.rs
new file mode 100644
index 00000000..0c292332
--- /dev/null
+++ b/src/api2/config/metricserver/influxdbhttp.rs
@@ -0,0 +1,314 @@
+use anyhow::{bail, format_err, Error};
+use hex::FromHex;
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+use proxmox_metrics::test_influxdb_http;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{
+    InfluxDbHttp, InfluxDbHttpUpdater, METRIC_SERVER_ID_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
+    PROXMOX_CONFIG_DIGEST_SCHEMA,
+};
+
+use pbs_config::metrics;
+
+async fn test_server(config: &InfluxDbHttp) -> Result<(), Error> {
+    if config.enable.unwrap_or(true) {
+        test_influxdb_http(
+            &config.url,
+            config.organization.as_deref().unwrap_or("proxmox"),
+            config.bucket.as_deref().unwrap_or("proxmox"),
+            config.token.as_deref(),
+            config.verify_tls.unwrap_or(true),
+        )
+        .await
+        .map_err(|err| format_err!("could not connect to {}: {}", config.url, err))
+    } else {
+        Ok(())
+    }
+}
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List of configured InfluxDB http metric servers.",
+        type: Array,
+        items: { type: InfluxDbHttp },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// List configured InfluxDB http metric servers.
+pub fn list_influxdb_http_servers(
+    _param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<InfluxDbHttp>, Error> {
+    let (config, digest) = metrics::config()?;
+
+    let mut list: Vec<InfluxDbHttp> = config.convert_to_typed_array("influxdb-http")?;
+
+    // don't return token via api
+    for item in list.iter_mut() {
+        item.token = None;
+    }
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            config: {
+                type: InfluxDbHttp,
+                flatten: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Create a new InfluxDB http server configuration
+pub async fn create_influxdb_http_server(config: InfluxDbHttp) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, _digest) = metrics::config()?;
+
+    if metrics.sections.get(&config.name).is_some() {
+        bail!("metric server '{}' already exists.", config.name);
+    }
+
+    test_server(&config).await?;
+
+    metrics.set_data(&config.name, "influxdb-http", &config)?;
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Remove a InfluxDB http server configuration
+pub fn delete_influxdb_http_server(
+    name: String,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, expected_digest) = metrics::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    if metrics.sections.remove(&name).is_none() {
+        bail!("name '{}' does not exist.", name);
+    }
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+        },
+    },
+    returns:  { type: InfluxDbHttp },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Read the InfluxDB http server configuration
+pub fn read_influxdb_http_server(
+    name: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<InfluxDbHttp, Error> {
+    let (metrics, digest) = metrics::config()?;
+
+    let mut config: InfluxDbHttp = metrics.lookup("influxdb-http", &name)?;
+
+    config.token = None;
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(config)
+}
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the enable property.
+    Enable,
+    /// Delete the token property.
+    Token,
+    /// Delete the bucket property.
+    Bucket,
+    /// Delete the organization property.
+    Organization,
+    /// Delete the max_body_size property.
+    MaxBodySize,
+    /// Delete the verify_tls property.
+    VerifyTls,
+    /// Delete the comment property.
+    Comment,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+            update: {
+                type: InfluxDbHttpUpdater,
+                flatten: true,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Update an InfluxDB http server configuration
+pub async fn update_influxdb_http_server(
+    name: String,
+    update: InfluxDbHttpUpdater,
+    delete: Option<Vec<DeletableProperty>>,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, expected_digest) = metrics::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut config: InfluxDbHttp = metrics.lookup("influxdb-http", &name)?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::Enable => {
+                    config.enable = None;
+                }
+                DeletableProperty::Token => {
+                    config.token = None;
+                }
+                DeletableProperty::Bucket => {
+                    config.bucket = None;
+                }
+                DeletableProperty::Organization => {
+                    config.organization = None;
+                }
+                DeletableProperty::MaxBodySize => {
+                    config.max_body_size = None;
+                }
+                DeletableProperty::VerifyTls => {
+                    config.verify_tls = None;
+                }
+                DeletableProperty::Comment => {
+                    config.comment = None;
+                }
+            }
+        }
+    }
+
+    if let Some(comment) = update.comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            config.comment = None;
+        } else {
+            config.comment = Some(comment);
+        }
+    }
+
+    if let Some(url) = update.url {
+        config.url = url;
+    }
+
+    if update.enable.is_some() {
+        config.enable = update.enable;
+    }
+    if update.token.is_some() {
+        config.token = update.token;
+    }
+    if update.bucket.is_some() {
+        config.bucket = update.bucket;
+    }
+    if update.organization.is_some() {
+        config.organization = update.organization;
+    }
+    if update.max_body_size.is_some() {
+        config.max_body_size = update.max_body_size;
+    }
+    if update.verify_tls.is_some() {
+        config.verify_tls = update.verify_tls;
+    }
+
+    test_server(&config).await?;
+
+    metrics.set_data(&name, "influxdb-http", &config)?;
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_INFLUXDB_HTTP_SERVER)
+    .put(&API_METHOD_UPDATE_INFLUXDB_HTTP_SERVER)
+    .delete(&API_METHOD_DELETE_INFLUXDB_HTTP_SERVER);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_INFLUXDB_HTTP_SERVERS)
+    .post(&API_METHOD_CREATE_INFLUXDB_HTTP_SERVER)
+    .match_all("name", &ITEM_ROUTER);
diff --git a/src/api2/config/metricserver/influxdbudp.rs b/src/api2/config/metricserver/influxdbudp.rs
new file mode 100644
index 00000000..388863d5
--- /dev/null
+++ b/src/api2/config/metricserver/influxdbudp.rs
@@ -0,0 +1,269 @@
+use anyhow::{bail, format_err, Error};
+use hex::FromHex;
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+use proxmox_metrics::test_influxdb_udp;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{
+    InfluxDbUdp, InfluxDbUdpUpdater, METRIC_SERVER_ID_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY,
+    PROXMOX_CONFIG_DIGEST_SCHEMA,
+};
+
+use pbs_config::metrics;
+
+async fn test_server(address: &str) -> Result<(), Error> {
+    test_influxdb_udp(address)
+        .await
+        .map_err(|err| format_err!("cannot conect to {}: {}", address, err))
+}
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List of configured InfluxDB udp metric servers.",
+        type: Array,
+        items: { type: InfluxDbUdp },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// List configured InfluxDB udp metric servers.
+pub fn list_influxdb_udp_servers(
+    _param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<InfluxDbUdp>, Error> {
+    let (config, digest) = metrics::config()?;
+
+    let list = config.convert_to_typed_array("influxdb-udp")?;
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            config: {
+                type: InfluxDbUdp,
+                flatten: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Create a new InfluxDB udp server configuration
+pub async fn create_influxdb_udp_server(config: InfluxDbUdp) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, _digest) = metrics::config()?;
+
+    if metrics.sections.get(&config.name).is_some() {
+        bail!("metric server '{}' already exists.", config.name);
+    }
+
+    if config.enable.unwrap_or(true) {
+        test_server(&config.host).await?;
+    }
+
+    metrics.set_data(&config.name, "influxdb-udp", &config)?;
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Remove a InfluxDB udp server configuration
+pub fn delete_influxdb_udp_server(
+    name: String,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, expected_digest) = metrics::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    if metrics.sections.remove(&name).is_none() {
+        bail!("name '{}' does not exist.", name);
+    }
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+        },
+    },
+    returns:  { type: InfluxDbUdp },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Read the InfluxDB udp server configuration
+pub fn read_influxdb_udp_server(
+    name: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<InfluxDbUdp, Error> {
+    let (metrics, digest) = metrics::config()?;
+
+    let config = metrics.lookup("influxdb-udp", &name)?;
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(config)
+}
+
+#[api()]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {
+    /// Delete the enable property.
+    Enable,
+    /// Delete the mtu property.
+    Mtu,
+    /// Delete the comment property.
+    Comment,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: METRIC_SERVER_ID_SCHEMA,
+            },
+            update: {
+                type: InfluxDbUdpUpdater,
+                flatten: true,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Update an InfluxDB udp server configuration
+pub async fn update_influxdb_udp_server(
+    name: String,
+    update: InfluxDbUdpUpdater,
+    delete: Option<Vec<DeletableProperty>>,
+    digest: Option<String>,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let _lock = metrics::lock_config()?;
+
+    let (mut metrics, expected_digest) = metrics::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut config: InfluxDbUdp = metrics.lookup("influxdb-udp", &name)?;
+
+    if let Some(delete) = delete {
+        for delete_prop in delete {
+            match delete_prop {
+                DeletableProperty::Enable => {
+                    config.enable = None;
+                }
+                DeletableProperty::Mtu => {
+                    config.mtu = None;
+                }
+                DeletableProperty::Comment => {
+                    config.comment = None;
+                }
+            }
+        }
+    }
+
+    if let Some(comment) = update.comment {
+        let comment = comment.trim().to_string();
+        if comment.is_empty() {
+            config.comment = None;
+        } else {
+            config.comment = Some(comment);
+        }
+    }
+
+    if let Some(host) = update.host {
+        config.host = host;
+    }
+
+    if update.enable.is_some() {
+        config.enable = update.enable;
+    }
+    if update.mtu.is_some() {
+        config.mtu = update.mtu;
+    }
+
+    metrics.set_data(&name, "influxdb-udp", &config)?;
+
+    if config.enable.unwrap_or(true) {
+        test_server(&config.host).await?;
+    }
+
+    metrics::save_config(&metrics)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_INFLUXDB_UDP_SERVER)
+    .put(&API_METHOD_UPDATE_INFLUXDB_UDP_SERVER)
+    .delete(&API_METHOD_DELETE_INFLUXDB_UDP_SERVER);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_INFLUXDB_UDP_SERVERS)
+    .post(&API_METHOD_CREATE_INFLUXDB_UDP_SERVER)
+    .match_all("name", &ITEM_ROUTER);
diff --git a/src/api2/config/metricserver/mod.rs b/src/api2/config/metricserver/mod.rs
new file mode 100644
index 00000000..cbce34f7
--- /dev/null
+++ b/src/api2/config/metricserver/mod.rs
@@ -0,0 +1,16 @@
+use proxmox_router::{Router, SubdirMap};
+use proxmox_router::list_subdirs_api_method;
+use proxmox_sys::sortable;
+
+pub mod influxdbudp;
+pub mod influxdbhttp;
+
+#[sortable]
+const SUBDIRS: SubdirMap = &sorted!([
+    ("influxdb-http", &influxdbhttp::ROUTER),
+    ("influxdb-udp", &influxdbudp::ROUTER),
+]);
+
+pub const ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(SUBDIRS))
+    .subdirs(SUBDIRS);
diff --git a/src/api2/config/mod.rs b/src/api2/config/mod.rs
index ffba94ba..17f1d1fe 100644
--- a/src/api2/config/mod.rs
+++ b/src/api2/config/mod.rs
@@ -10,6 +10,7 @@ pub mod changer;
 pub mod datastore;
 pub mod drive;
 pub mod media_pool;
+pub mod metricserver;
 pub mod prune;
 pub mod remote;
 pub mod sync;
@@ -26,6 +27,7 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("datastore", &datastore::ROUTER),
     ("drive", &drive::ROUTER),
     ("media-pool", &media_pool::ROUTER),
+    ("metricserver", &metricserver::ROUTER),
     ("prune", &prune::ROUTER),
     ("remote", &remote::ROUTER),
     ("sync", &sync::ROUTER),
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 7/8] ui: add window/InfluxDbEdit
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
                   ` (5 preceding siblings ...)
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it Dominik Csapak
  7 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

contains both windows for HTTP and UDP

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/Makefile               |   1 +
 www/window/InfluxDbEdit.js | 218 +++++++++++++++++++++++++++++++++++++
 2 files changed, 219 insertions(+)
 create mode 100644 www/window/InfluxDbEdit.js

diff --git a/www/Makefile b/www/Makefile
index 311f4753..3a36daba 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -82,6 +82,7 @@ JSSRC=							\
 	window/VerifyJobEdit.js				\
 	window/VerifyAll.js				\
 	window/ZFSCreate.js				\
+	window/InfluxDbEdit.js				\
 	dashboard/DataStoreStatistics.js		\
 	dashboard/LongestTasks.js			\
 	dashboard/RunningTasks.js			\
diff --git a/www/window/InfluxDbEdit.js b/www/window/InfluxDbEdit.js
new file mode 100644
index 00000000..4ac9fff3
--- /dev/null
+++ b/www/window/InfluxDbEdit.js
@@ -0,0 +1,218 @@
+Ext.define('PBS.window.InfluxDbHttpEdit', {
+    extend: 'Proxmox.window.Edit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    subject: 'InfluxDB (HTTP)',
+
+    cbindData: function() {
+	let me = this;
+	me.isCreate = !me.serverid;
+	me.serverid = me.serverid || "";
+	me.url = `/api2/extjs/config/metricserver/influxdb-http/${me.serverid}`;
+	me.tokenEmptyText = me.isCreate ? '' : gettext('unchanged');
+	me.method = me.isCreate ? 'POST' : 'PUT';
+	if (!me.isCreate) {
+	    me.subject = `${me.subject}: ${me.serverid}`;
+	}
+	return {};
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+
+	    column1: [
+		{
+		    xtype: 'pmxDisplayEditField',
+		    name: 'name',
+		    fieldLabel: gettext('Name'),
+		    allowBlank: false,
+		    cbind: {
+			editable: '{isCreate}',
+			value: '{serverid}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'url',
+		    fieldLabel: gettext('URL'),
+		    allowBlank: false,
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'checkbox',
+		    name: 'enable',
+		    fieldLabel: gettext('Enabled'),
+		    inputValue: 1,
+		    uncheckedValue: 0,
+		    checked: true,
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'organization',
+		    fieldLabel: gettext('Organization'),
+		    emptyText: 'proxmox',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'bucket',
+		    fieldLabel: gettext('Bucket'),
+		    emptyText: 'proxmox',
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'token',
+		    fieldLabel: gettext('Token'),
+		    allowBlank: true,
+		    deleteEmpty: false,
+		    submitEmpty: false,
+		    cbind: {
+			emptyText: '{tokenEmptyText}',
+		    },
+		},
+	    ],
+
+	    columnB: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'comment',
+		    fieldLabel: gettext('Comment'),
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+
+	    advancedColumn1: [
+		{
+		    xtype: 'proxmoxintegerfield',
+		    name: 'max-body-size',
+		    fieldLabel: gettext('Batch Size (b)'),
+		    minValue: 1,
+		    emptyText: '25000000',
+		    submitEmpty: false,
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+	},
+    ],
+});
+
+Ext.define('PBS.window.InfluxDbUdpEdit', {
+    extend: 'Proxmox.window.Edit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    subject: 'InfluxDB (UDP)',
+
+    cbindData: function() {
+	let me = this;
+	me.isCreate = !me.serverid;
+	me.serverid = me.serverid || "";
+	me.url = `/api2/extjs/config/metricserver/influxdb-udp/${me.serverid}`;
+	me.method = me.isCreate ? 'POST' : 'PUT';
+	if (!me.isCreate) {
+	    me.subject = `${me.subject}: ${me.serverid}`;
+	}
+	return {};
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+
+	    onGetValues: function(values) {
+		values.host += `:${values.port}`;
+		delete values.port;
+		return values;
+	    },
+
+	    column1: [
+		{
+		    xtype: 'pmxDisplayEditField',
+		    name: 'name',
+		    fieldLabel: gettext('Name'),
+		    allowBlank: false,
+		    cbind: {
+			editable: '{isCreate}',
+			value: '{serverid}',
+		    },
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'host',
+		    fieldLabel: gettext('Host'),
+		    allowBlank: false,
+		},
+	    ],
+
+	    column2: [
+		{
+		    xtype: 'checkbox',
+		    name: 'enable',
+		    fieldLabel: gettext('Enabled'),
+		    inputValue: 1,
+		    uncheckedValue: 0,
+		    checked: true,
+		},
+		{
+		    xtype: 'proxmoxintegerfield',
+		    name: 'port',
+		    minValue: 1,
+		    maxValue: 65535,
+		    fieldLabel: gettext('Port'),
+		    allowBlank: false,
+		},
+	    ],
+
+	    columnB: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'comment',
+		    fieldLabel: gettext('Comment'),
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+
+	    advancedColumn1: [
+		{
+		    xtype: 'proxmoxintegerfield',
+		    name: 'mtu',
+		    fieldLabel: 'MTU',
+		    minValue: 1,
+		    emptyText: '1500',
+		    submitEmpty: false,
+		    cbind: {
+			deleteEmpty: '{!isCreate}',
+		    },
+		},
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+	me.callParent();
+
+	me.load({
+	    success: function(response, options) {
+		let values = response.result.data;
+		let [_match, host, port] = /^(.*):(\d+)$/.exec(values.host) || [];
+		values.host = host;
+		values.port = port;
+		me.setValues(values);
+	    },
+	});
+    },
+});
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it
  2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
                   ` (6 preceding siblings ...)
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 7/8] ui: add window/InfluxDbEdit Dominik Csapak
@ 2022-06-02 12:18 ` Dominik Csapak
  2022-06-07 12:55   ` Thomas Lamprecht
  7 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2022-06-02 12:18 UTC (permalink / raw)
  To: pbs-devel

simple CRUD interface to show/add/edit/delete metric servers

it's a bit different from PVE's so that it's harder to reuse that to
copy it. If we need it again, we can still refactor and combine them.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/Makefile                   |   1 +
 www/NavigationTree.js          |   6 ++
 www/config/MetricServerView.js | 145 +++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+)
 create mode 100644 www/config/MetricServerView.js

diff --git a/www/Makefile b/www/Makefile
index 3a36daba..757c1fd5 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -62,6 +62,7 @@ JSSRC=							\
 	config/WebauthnView.js				\
 	config/CertificateView.js			\
 	config/NodeOptionView.js			\
+	config/MetricServerView.js			\
 	window/ACLEdit.js				\
 	window/BackupFileDownloader.js			\
 	window/BackupGroupChangeOwner.js		\
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 5f44aace..dc69f312 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -68,6 +68,12 @@ Ext.define('PBS.store.NavigationStore', {
 			path: 'pbsCertificateConfiguration',
 			leaf: true,
 		    },
+		    {
+			text: gettext('Metric Server'),
+			iconCls: 'fa fa-bar-chart',
+			path: 'pbsMetricServerView',
+			leaf: true,
+		    },
 		    {
 			text: gettext('Subscription'),
 			iconCls: 'fa fa-support',
diff --git a/www/config/MetricServerView.js b/www/config/MetricServerView.js
new file mode 100644
index 00000000..b904a427
--- /dev/null
+++ b/www/config/MetricServerView.js
@@ -0,0 +1,145 @@
+Ext.define('PBS.config.MetricServerView', {
+    extend: 'Ext.grid.Panel',
+    alias: ['widget.pbsMetricServerView'],
+
+    stateful: true,
+    stateId: 'grid-metricserver',
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	render_type: function(value) {
+	    switch (value) {
+		case 'influxdb-http': return "InfluxDB (HTTP)";
+		case 'influxdb-udp': return "InfluxDB (UDP)";
+		default: return Proxmox.Utils.unknownText;
+	    }
+	},
+
+	get_xtype: function(value) {
+	    switch (value) {
+		case 'influxdb-http': return "InfluxDbHttp";
+		case 'influxdb-udp': return "InfluxDbUdp";
+		default: throw "invalid type";
+	    }
+	},
+
+	editWindow: function(xtype, id) {
+	    let me = this;
+	    Ext.create(`PBS.window.${xtype}Edit`, {
+		serverid: id,
+		autoShow: true,
+		autoLoad: true,
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	},
+
+	addServer: function(button) {
+	    this.editWindow(this.get_xtype(button.type));
+	},
+
+	editServer: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || selection.length < 1) {
+		return;
+	    }
+
+	    let cfg = selection[0].data;
+
+	    let xtype = me.get_xtype(cfg.type);
+	    me.editWindow(xtype, cfg.name);
+	},
+
+	reload: function() {
+	    this.getView().getStore().load();
+	},
+    },
+
+    store: {
+	autoLoad: true,
+	id: 'metricservers',
+	proxy: {
+	    type: 'proxmox',
+	    url: '/api2/json/admin/metricserver',
+	},
+    },
+
+    columns: [
+	{
+	    text: gettext('Name'),
+	    flex: 2,
+	    dataIndex: 'name',
+	},
+	{
+	    text: gettext('Type'),
+	    width: 150,
+	    dataIndex: 'type',
+	    renderer: 'render_type',
+	},
+	{
+	    text: gettext('Enabled'),
+	    dataIndex: 'disable',
+	    width: 100,
+	    renderer: Proxmox.Utils.format_neg_boolean,
+	},
+	{
+	    text: gettext('Target Server'),
+	    width: 200,
+	    dataIndex: 'server',
+	},
+	{
+	    text: gettext('Comment'),
+	    flex: 3,
+	    dataIndex: 'comment',
+	    renderer: Ext.htmlEncode,
+	},
+    ],
+
+    tbar: [
+	{
+	    text: gettext('Add'),
+	    menu: [
+		{
+		    text: 'InfluxDB (HTTP)',
+		    type: 'influxdb-http',
+		    iconCls: 'fa fa-fw fa-bar-chart',
+		    handler: 'addServer',
+		},
+		{
+		    text: 'InfluxDB (UDP)',
+		    type: 'influxdb-udp',
+		    iconCls: 'fa fa-fw fa-bar-chart',
+		    handler: 'addServer',
+		},
+	    ],
+	},
+	{
+	    text: gettext('Edit'),
+	    xtype: 'proxmoxButton',
+	    handler: 'editServer',
+	    disabled: true,
+	},
+	{
+	    xtype: 'proxmoxStdRemoveButton',
+	    getUrl: (rec) => `/api2/extjs/config/metricserver/${rec.data.type}/${rec.data.name}`,
+	    getRecordName: (rec) => rec.data.name,
+	    callback: 'reload',
+	},
+    ],
+
+    listeners: {
+	itemdblclick: 'editServer',
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	me.callParent();
+
+	Proxmox.Utils.monStoreErrors(me, me.getStore());
+    },
+});
-- 
2.30.2





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update Dominik Csapak
@ 2022-06-02 14:09   ` Matthias Heiserer
  0 siblings, 0 replies; 15+ messages in thread
From: Matthias Heiserer @ 2022-06-02 14:09 UTC (permalink / raw)
  To: pbs-devel

8<---snip snap
> +    rrd_update_disk_stat(&hostdisk, "host");
                             ^^^^^^
> +
> +    for stat in datastores {
> +        let rrd_prefix = format!("datastore/{}", stat.name);
> +        rrd_update_disk_stat(&stat, &rrd_prefix);
                                 ^^^^^
> +    }
> +}
Clippy says that the two marked parameters are a needless borrows, as 
they are already references.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class Dominik Csapak
@ 2022-06-03 10:16   ` Wolfgang Bumiller
  0 siblings, 0 replies; 15+ messages in thread
From: Wolfgang Bumiller @ 2022-06-03 10:16 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: pbs-devel

On Thu, Jun 02, 2022 at 02:18:06PM +0200, Dominik Csapak wrote:
(...)
> @@ -0,0 +1,115 @@
> +use std::collections::HashMap;
> +
> +use anyhow::Error;
> +use lazy_static::lazy_static;
> +
> +use proxmox_metrics::{influxdb_http, influxdb_udp, Metrics};
> +use proxmox_schema::*;
> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
> +
> +use pbs_api_types::{InfluxDbHttp, InfluxDbUdp, METRIC_SERVER_ID_SCHEMA};
> +
> +use crate::{open_backup_lockfile, BackupLockGuard};
> +
> +lazy_static! {
> +    pub static ref CONFIG: SectionConfig = init();
> +}
> +
> +fn init() -> SectionConfig {
> +    let mut config = SectionConfig::new(&METRIC_SERVER_ID_SCHEMA);
> +
> +    let udp_schema = match InfluxDbUdp::API_SCHEMA {
> +        Schema::Object(ref object_schema) => object_schema,
> +        _ => unreachable!(),
> +    };

New code should prefer using the `unwrap_<type>_schema()` const fns,
defining a const for the schema, in order to get some compile time
checks there.

    const UDP_SCHEMA: &ObjectSchema = InfluxDbUdp::API_SCHEMA.unwrap_object_schema();

Eg. if you ever `#[serde(flatten)]` some struct into the type the schema
type will change to `AllOfSchema`. See prune.rs as an example for this.
(We should also switch the remaining files probably.)

> +
> +    let udp_plugin = SectionConfigPlugin::new(
> +        "influxdb-udp".to_string(),
> +        Some("name".to_string()),
> +        udp_schema,
> +    );
> +    config.register_plugin(udp_plugin);
> +
> +    let http_schema = match InfluxDbHttp::API_SCHEMA {
> +        Schema::Object(ref object_schema) => object_schema,
> +        _ => unreachable!(),
> +    };
> +
> +    let http_plugin = SectionConfigPlugin::new(
> +        "influxdb-http".to_string(),
> +        Some("name".to_string()),
> +        http_schema,
> +    );
> +
> +    config.register_plugin(http_plugin);
> +
> +    config
> +}
> +
> +pub const METRIC_SERVER_CFG_FILENAME: &str = "/etc/proxmox-backup/metricserver.cfg";
> +pub const METRIC_SERVER_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.metricserver.lck";
> +
> +/// Get exclusive lock
> +pub fn lock_config() -> Result<BackupLockGuard, Error> {
> +    open_backup_lockfile(METRIC_SERVER_CFG_LOCKFILE, None, true)
> +}
> +
> +pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
> +    let content = proxmox_sys::fs::file_read_optional_string(METRIC_SERVER_CFG_FILENAME)?
> +        .unwrap_or_else(|| "".to_string());

Easier on the compiler (and a `const fn`):

    .unwrap_or_else(String::new)

or even

    .unwrap_or_default()

> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let data = CONFIG.parse(METRIC_SERVER_CFG_FILENAME, &content)?;
> +    Ok((data, digest))
> +}
> +
> +pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
> +    let raw = CONFIG.write(METRIC_SERVER_CFG_FILENAME, config)?;
> +    crate::replace_backup_config(METRIC_SERVER_CFG_FILENAME, raw.as_bytes())
> +}
> +
> +// shell completion helper
> +pub fn complete_remote_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> +    match config() {
> +        Ok((data, _digest)) => data.sections.iter().map(|(id, _)| id.to_string()).collect(),

I think you could simplify this to `.keys().cloned().collect()`




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys Dominik Csapak
@ 2022-06-07  6:51   ` Thomas Lamprecht
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2022-06-07  6:51 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

Am 02/06/2022 um 14:18 schrieb Dominik Csapak:
> as replacement for 'disk_usage' in tools. also remove that there since
> we do not need it anymore
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/api2/admin/datastore.rs     |  4 ++--
>  src/api2/node/status.rs         | 13 ++++++++++---
>  src/api2/status.rs              |  4 ++--
>  src/bin/proxmox-backup-proxy.rs |  4 ++--
>  src/tools/disks/mod.rs          | 20 +-------------------
>  5 files changed, 17 insertions(+), 28 deletions(-)
> 
>

applied, thanks! Had to remove a path::Path import in api2/node/status.rs which got
unused by this patch (maybe there was still some other unrelated usage from when you
created this, that got removed in the meantime - didn't checked as it's not a biggie
anyway). Anyhow, I squashed that in your path and added the info about when the
proxmox-sys helper got made available (in the recent 0.3 bump).




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints Dominik Csapak
@ 2022-06-07 12:44   ` Thomas Lamprecht
  2022-06-08  8:23     ` Dominik Csapak
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Lamprecht @ 2022-06-07 12:44 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

Am 02/06/2022 um 14:18 schrieb Dominik Csapak:
> but in contrast to pve, we split the api by type of the section config,
> since we cannot handle multiple types in the updater
> 

higher level comment:

I'd guess that it comes partially from how we named the perl module in PVE, but
I'd still drop the "server" part from file/directory names and thus module/api
path ones, IMO it doesn't really add much but extra length/noise (PVE's api also
uses only "metrics")





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it
  2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it Dominik Csapak
@ 2022-06-07 12:55   ` Thomas Lamprecht
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2022-06-07 12:55 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

Am 02/06/2022 um 14:18 schrieb Dominik Csapak:
> simple CRUD interface to show/add/edit/delete metric servers
> 
> it's a bit different from PVE's so that it's harder to reuse that to
> copy it. If we need it again, we can still refactor and combine them.
> 

looks OK, just some code style nits inline.

> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/Makefile                   |   1 +
>  www/NavigationTree.js          |   6 ++
>  www/config/MetricServerView.js | 145 +++++++++++++++++++++++++++++++++
>  3 files changed, 152 insertions(+)
>  create mode 100644 www/config/MetricServerView.js
> 
> diff --git a/www/Makefile b/www/Makefile
> index 3a36daba..757c1fd5 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -62,6 +62,7 @@ JSSRC=							\
>  	config/WebauthnView.js				\
>  	config/CertificateView.js			\
>  	config/NodeOptionView.js			\
> +	config/MetricServerView.js			\
>  	window/ACLEdit.js				\
>  	window/BackupFileDownloader.js			\
>  	window/BackupGroupChangeOwner.js		\
> diff --git a/www/NavigationTree.js b/www/NavigationTree.js
> index 5f44aace..dc69f312 100644
> --- a/www/NavigationTree.js
> +++ b/www/NavigationTree.js
> @@ -68,6 +68,12 @@ Ext.define('PBS.store.NavigationStore', {
>  			path: 'pbsCertificateConfiguration',
>  			leaf: true,
>  		    },
> +		    {
> +			text: gettext('Metric Server'),
> +			iconCls: 'fa fa-bar-chart',
> +			path: 'pbsMetricServerView',
> +			leaf: true,
> +		    },

I'd rather put this as tab in the general "Configuration" panel, this is slightly too
niche to take up space in the main navigation tree directly.

>  		    {
>  			text: gettext('Subscription'),
>  			iconCls: 'fa fa-support',
> diff --git a/www/config/MetricServerView.js b/www/config/MetricServerView.js
> new file mode 100644
> index 00000000..b904a427
> --- /dev/null
> +++ b/www/config/MetricServerView.js
> @@ -0,0 +1,145 @@
> +Ext.define('PBS.config.MetricServerView', {
> +    extend: 'Ext.grid.Panel',
> +    alias: ['widget.pbsMetricServerView'],
> +
> +    stateful: true,
> +    stateId: 'grid-metricserver',
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	render_type: function(value) {

I'd prefer camelCase for new functions where sensible, like ExtJS uses everywhere and part of
our code base also does.

> +	    switch (value) {
> +		case 'influxdb-http': return "InfluxDB (HTTP)";
> +		case 'influxdb-udp': return "InfluxDB (UDP)";
> +		default: return Proxmox.Utils.unknownText;
> +	    }

I'd like to avoid switch case in JS, at least most of the time, there seldom nice and either
use more space while still being more noisy/harder to read than objects or require having
multiple statements on a single line, both not to nice. If JS would have a match statement
allowing to return a expression like we got in rust I'd be sold though.

Rather just use a object. with multiple such similar metric type derived info one could also
use a nested, schema like one

{
    indlufxdb: {
        xtype: '...',
        name: '...',
    },
}

While this is also using quite some vertical space it's at least easy to read due to being
like a static schema definition.

The renderer could then be also just inline, e.g.:
v => PBS.Schema.metricServer[v]?.name ?? Proxmox.Utils.unknownText,

but not to hard feelings for that.

> +	},
> +
> +	get_xtype: function(value) {
> +	    switch (value) {

same here (and thus the schema proposal)

> +		case 'influxdb-http': return "InfluxDbHttp";
> +		case 'influxdb-udp': return "InfluxDbUdp";
> +		default: throw "invalid type";
> +	    }
> +	},
> +
> +	editWindow: function(xtype, id) {
> +	    let me = this;
> +	    Ext.create(`PBS.window.${xtype}Edit`, {
> +		serverid: id,
> +		autoShow: true,
> +		autoLoad: true,
> +		listeners: {
> +		    destroy: () => me.reload(),
> +		},
> +	    });
> +	},
> +
> +	addServer: function(button) {
> +	    this.editWindow(this.get_xtype(button.type));
> +	},
> +
> +	editServer: function() {
> +	    let me = this;
> +	    let view = me.getView();
> +	    let selection = view.getSelection();
> +	    if (!selection || selection.length < 1) {
> +		return;
> +	    }
> +
> +	    let cfg = selection[0].data;
> +
> +	    let xtype = me.get_xtype(cfg.type);
> +	    me.editWindow(xtype, cfg.name);
> +	},
> +
> +	reload: function() {
> +	    this.getView().getStore().load();
> +	},
> +    },
> +
> +    store: {
> +	autoLoad: true,
> +	id: 'metricservers',
> +	proxy: {
> +	    type: 'proxmox',
> +	    url: '/api2/json/admin/metricserver',
> +	},
> +    },
> +
> +    columns: [
> +	{
> +	    text: gettext('Name'),
> +	    flex: 2,
> +	    dataIndex: 'name',
> +	},
> +	{
> +	    text: gettext('Type'),
> +	    width: 150,
> +	    dataIndex: 'type',
> +	    renderer: 'render_type',
> +	},
> +	{
> +	    text: gettext('Enabled'),
> +	    dataIndex: 'disable',
> +	    width: 100,
> +	    renderer: Proxmox.Utils.format_neg_boolean,
> +	},
> +	{
> +	    text: gettext('Target Server'),
> +	    width: 200,
> +	    dataIndex: 'server',
> +	},
> +	{
> +	    text: gettext('Comment'),
> +	    flex: 3,
> +	    dataIndex: 'comment',
> +	    renderer: Ext.htmlEncode,
> +	},
> +    ],
> +
> +    tbar: [
> +	{
> +	    text: gettext('Add'),
> +	    menu: [
> +		{
> +		    text: 'InfluxDB (HTTP)',
> +		    type: 'influxdb-http',
> +		    iconCls: 'fa fa-fw fa-bar-chart',
> +		    handler: 'addServer',
> +		},
> +		{
> +		    text: 'InfluxDB (UDP)',
> +		    type: 'influxdb-udp',
> +		    iconCls: 'fa fa-fw fa-bar-chart',
> +		    handler: 'addServer',
> +		},
> +	    ],
> +	},
> +	{
> +	    text: gettext('Edit'),
> +	    xtype: 'proxmoxButton',
> +	    handler: 'editServer',
> +	    disabled: true,
> +	},
> +	{
> +	    xtype: 'proxmoxStdRemoveButton',
> +	    getUrl: (rec) => `/api2/extjs/config/metricserver/${rec.data.type}/${rec.data.name}`,
> +	    getRecordName: (rec) => rec.data.name,
> +	    callback: 'reload',
> +	},
> +    ],
> +
> +    listeners: {
> +	itemdblclick: 'editServer',
> +    },
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.callParent();
> +
> +	Proxmox.Utils.monStoreErrors(me, me.getStore());
> +    },
> +});





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints
  2022-06-07 12:44   ` Thomas Lamprecht
@ 2022-06-08  8:23     ` Dominik Csapak
  0 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2022-06-08  8:23 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox Backup Server development discussion

On 6/7/22 14:44, Thomas Lamprecht wrote:
> Am 02/06/2022 um 14:18 schrieb Dominik Csapak:
>> but in contrast to pve, we split the api by type of the section config,
>> since we cannot handle multiple types in the updater
>>
> 
> higher level comment:
> 
> I'd guess that it comes partially from how we named the perl module in PVE, but
> I'd still drop the "server" part from file/directory names and thus module/api
> path ones, IMO it doesn't really add much but extra length/noise (PVE's api also
> uses only "metrics")
> 

makes sense, in pve, we do '.../metrics/server' for the api though, should i do that?
or keep it just '.../metrics' ?






^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-06-08  8:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-02 12:18 [pbs-devel] [PATCH proxmox-backup v7 0/8] add metrics server capability Dominik Csapak
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 1/8] use 'fs_info' from proxmox-sys Dominik Csapak
2022-06-07  6:51   ` [pbs-devel] applied: " Thomas Lamprecht
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 2/8] pbs-api-types: add metrics api types Dominik Csapak
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 3/8] pbs-config: add metrics config class Dominik Csapak
2022-06-03 10:16   ` Wolfgang Bumiller
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 4/8] backup-proxy: decouple stats gathering from rrd update Dominik Csapak
2022-06-02 14:09   ` Matthias Heiserer
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 5/8] proxmox-backup-proxy: send metrics to configured metrics server Dominik Csapak
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 6/8] api: add metricserver endpoints Dominik Csapak
2022-06-07 12:44   ` Thomas Lamprecht
2022-06-08  8:23     ` Dominik Csapak
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 7/8] ui: add window/InfluxDbEdit Dominik Csapak
2022-06-02 12:18 ` [pbs-devel] [PATCH proxmox-backup v7 8/8] ui: add MetricServerView and use it Dominik Csapak
2022-06-07 12:55   ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal