From 170a358d79a2f032af4d899dce75452359f010c2 Mon Sep 17 00:00:00 2001 From: brinkcoder Date: Wed, 5 Mar 2025 01:30:41 +0100 Subject: [PATCH] add only_metrics, diff_values and derived_values. docs: clarify filter logic, consistency for list --- collectors/nfs3Metric.md | 69 ++++++++++++++---------- collectors/nfs4Metric.md | 114 ++++++++++++++++++++++----------------- collectors/nfsMetric.go | 94 +++++++++++++++++++++----------- 3 files changed, 168 insertions(+), 109 deletions(-) diff --git a/collectors/nfs3Metric.md b/collectors/nfs3Metric.md index 63937ea..96c484f 100644 --- a/collectors/nfs3Metric.md +++ b/collectors/nfs3Metric.md @@ -1,39 +1,52 @@ - ## `nfs3stat` collector ```json "nfs3stat": { - "nfsstat" : "/path/to/nfsstat", + "nfsstat": "/path/to/nfsstat", "exclude_metrics": [ - "nfs3_total" - ] + "total" + ], + "only_metrics": [ + "read_diff" + ], + "send_abs_values": true, + "send_diff_values": true, + "send_derived_values": true } ``` -The `nfs3stat` collector reads data from `nfsstat` command and outputs a handful **node** metrics. If a metric is not required, it can be excluded from forwarding it to the sink. There is currently no possibility to get the metrics per mount point. +The `nfs3stat` collector reads data from `nfsstat` command and outputs a handful **node** metrics. +Both filtering mechanisms are supported: +- `exclude_metrics`: Excludes the specified metrics. Uses base metric names without the nfs3_ prefix. +- `only_metrics`: If provided, only the listed metrics are collected. This takes precedence over `exclude_metrics`. -Metrics: -* `nfs3_total` -* `nfs3_null` -* `nfs3_getattr` -* `nfs3_setattr` -* `nfs3_lookup` -* `nfs3_access` -* `nfs3_readlink` -* `nfs3_read` -* `nfs3_write` -* `nfs3_create` -* `nfs3_mkdir` -* `nfs3_symlink` -* `nfs3_remove` -* `nfs3_rmdir` -* `nfs3_rename` -* `nfs3_link` -* `nfs3_readdir` -* `nfs3_readdirplus` -* `nfs3_fsstat` -* `nfs3_fsinfo` -* `nfs3_pathconf` -* `nfs3_commit` +**Absolute Metrics:** +- `nfs3_total` +- `nfs3_null` +- `nfs3_getattr` +- `nfs3_setattr` +- `nfs3_lookup` +- `nfs3_access` +- `nfs3_readlink` +- `nfs3_read` +- `nfs3_write` +- `nfs3_create` +- `nfs3_mkdir` +- `nfs3_symlink` +- `nfs3_remove` +- `nfs3_rmdir` +- `nfs3_rename` +- `nfs3_link` +- `nfs3_readdir` +- `nfs3_readdirplus` +- `nfs3_fsstat` +- `nfs3_fsinfo` +- `nfs3_pathconf` +- `nfs3_commit` +**Diff Metrics:** +For each metric, if `send_diff_values` is enabled, the collector computes the difference (current value minus previous value) and sends it with the suffix `_diff`. + +**Derived Metrics:** +For each metric, if `send_derived_values` is enabled, the collector computes the rate (difference divided by the time interval) and sends it with the suffix `_rate`. diff --git a/collectors/nfs4Metric.md b/collectors/nfs4Metric.md index 71d9613..5f368dc 100644 --- a/collectors/nfs4Metric.md +++ b/collectors/nfs4Metric.md @@ -1,62 +1,76 @@ - ## `nfs4stat` collector ```json "nfs4stat": { - "nfsstat" : "/path/to/nfsstat", + "nfsstat": "/path/to/nfsstat", "exclude_metrics": [ - "nfs4_total" - ] + "total" + ], + "only_metrics": [ + "read" + ], + "send_abs_values": true, + "send_diff_values": true, + "send_derived_values": true } ``` -The `nfs4stat` collector reads data from `nfsstat` command and outputs a handful **node** metrics. If a metric is not required, it can be excluded from forwarding it to the sink. There is currently no possibility to get the metrics per mount point. +The `nfs4stat` collector reads data from `nfsstat` command and outputs a handful **node** metrics. + +Both filtering mechanisms are supported: +- `exclude_metrics`: Excludes the specified metrics. Uses base metric names without the nfs4_ prefix. +- `only_metrics`: If provided, only the listed metrics are collected. This takes precedence over `exclude_metrics`. -Metrics: -* `nfs4_total` -* `nfs4_null` -* `nfs4_read` -* `nfs4_write` -* `nfs4_commit` -* `nfs4_open` -* `nfs4_open_conf` -* `nfs4_open_noat` -* `nfs4_open_dgrd` -* `nfs4_close` -* `nfs4_setattr` -* `nfs4_fsinfo` -* `nfs4_renew` -* `nfs4_setclntid` -* `nfs4_confirm` -* `nfs4_lock` -* `nfs4_lockt` -* `nfs4_locku` -* `nfs4_access` -* `nfs4_getattr` -* `nfs4_lookup` -* `nfs4_lookup_root` -* `nfs4_remove` -* `nfs4_rename` -* `nfs4_link` -* `nfs4_symlink` -* `nfs4_create` -* `nfs4_pathconf` -* `nfs4_statfs` -* `nfs4_readlink` -* `nfs4_readdir` -* `nfs4_server_caps` -* `nfs4_delegreturn` -* `nfs4_getacl` -* `nfs4_setacl` -* `nfs4_rel_lkowner` -* `nfs4_exchange_id` -* `nfs4_create_session` -* `nfs4_destroy_session` -* `nfs4_sequence` -* `nfs4_get_lease_time` -* `nfs4_reclaim_comp` -* `nfs4_secinfo_no` -* `nfs4_bind_conn_to_ses` +**Absolute Metrics:** +- `nfs4_total` +- `nfs4_null` +- `nfs4_read` +- `nfs4_write` +- `nfs4_commit` +- `nfs4_open` +- `nfs4_open_conf` +- `nfs4_open_noat` +- `nfs4_open_dgrd` +- `nfs4_close` +- `nfs4_setattr` +- `nfs4_fsinfo` +- `nfs4_renew` +- `nfs4_setclntid` +- `nfs4_confirm` +- `nfs4_lock` +- `nfs4_lockt` +- `nfs4_locku` +- `nfs4_access` +- `nfs4_getattr` +- `nfs4_lookup` +- `nfs4_lookup_root` +- `nfs4_remove` +- `nfs4_rename` +- `nfs4_link` +- `nfs4_symlink` +- `nfs4_create` +- `nfs4_pathconf` +- `nfs4_statfs` +- `nfs4_readlink` +- `nfs4_readdir` +- `nfs4_server_caps` +- `nfs4_delegreturn` +- `nfs4_getacl` +- `nfs4_setacl` +- `nfs4_rel_lkowner` +- `nfs4_exchange_id` +- `nfs4_create_session` +- `nfs4_destroy_session` +- `nfs4_sequence` +- `nfs4_get_lease_time` +- `nfs4_reclaim_comp` +- `nfs4_secinfo_no` +- `nfs4_bind_conn_to_ses` +- `nfs4_seek` +**Diff Metrics:** +For each metric, if `send_diff_values` is enabled, the collector computes the difference and sends it with the suffix `_diff`. +**Derived Metrics:** +For each metric, if `send_derived_values` is enabled, the collector computes the rate (difference divided by the time interval) and sends it with the suffix `_rate`. diff --git a/collectors/nfsMetric.go b/collectors/nfsMetric.go index 019e25c..06776b8 100644 --- a/collectors/nfsMetric.go +++ b/collectors/nfsMetric.go @@ -3,15 +3,12 @@ package collectors import ( "encoding/json" "fmt" - "log" - - // "os" "os/exec" "strconv" "strings" "time" - lp "github.com/ClusterCockpit/cc-energy-manager/pkg/cc-message" + lp "github.com/ClusterCockpit/cc-lib/ccMessage" ) // First part contains the code for the general NfsCollector. @@ -29,8 +26,12 @@ type nfsCollector struct { tags map[string]string version string config struct { - Nfsstats string `json:"nfsstat"` - ExcludeMetrics []string `json:"exclude_metrics,omitempty"` + Nfsstats string `json:"nfsstat"` + ExcludeMetrics []string `json:"exclude_metrics,omitempty"` + OnlyMetrics []string `json:"only_metrics,omitempty"` + SendAbsoluteValues bool `json:"send_abs_values,omitempty"` + SendDiffValues bool `json:"send_diff_values,omitempty"` + SendDerivedValues bool `json:"send_derived_values,omitempty"` } data map[string]NfsCollectorData } @@ -46,14 +47,12 @@ func (m *nfsCollector) initStats() error { continue } if lf[1] == m.version { + // Use the base metric name (without prefix) for filtering. name := strings.Trim(lf[3], ":") if _, exist := m.data[name]; !exist { value, err := strconv.ParseInt(lf[4], 0, 64) if err == nil { - x := m.data[name] - x.current = value - x.last = value - m.data[name] = x + m.data[name] = NfsCollectorData{current: value, last: value} } } } @@ -89,24 +88,42 @@ func (m *nfsCollector) updateStats() error { return err } +// shouldOutput returns true if a metric (base name + variant) should be forwarded. +// The variant is "" for absolute, "_diff" for diff, and "_rate" for derived. +// If only_metrics is set, only the metric names that exactly match (base+variant) are forwarded. +func (m *nfsCollector) shouldOutput(baseName, variant string) bool { + finalName := baseName + variant + if len(m.config.OnlyMetrics) > 0 { + for _, n := range m.config.OnlyMetrics { + if n == finalName { + return true + } + } + return false + } + for _, n := range m.config.ExcludeMetrics { + if n == finalName { + return false + } + } + return true +} + func (m *nfsCollector) MainInit(config json.RawMessage) error { - m.config.Nfsstats = string(NFSSTAT_EXEC) - // Read JSON configuration + m.config.Nfsstats = NFSSTAT_EXEC if len(config) > 0 { err := json.Unmarshal(config, &m.config) if err != nil { - log.Print(err.Error()) return err } } - m.meta = map[string]string{ - "source": m.name, - "group": "NFS", + // For backwards compatibility, send_abs_values defaults to true. + if !m.config.SendAbsoluteValues { + m.config.SendAbsoluteValues = true } - m.tags = map[string]string{ - "type": "node", - } - // Check if nfsstat is in executable search path + // Diff and derived default to false if not specified. + m.meta = map[string]string{"source": m.name, "group": "NFS"} + m.tags = map[string]string{"type": "node"} _, err := exec.LookPath(m.config.Nfsstats) if err != nil { return fmt.Errorf("NfsCollector.Init(): Failed to find nfsstat binary '%s': %v", m.config.Nfsstats, err) @@ -123,7 +140,6 @@ func (m *nfsCollector) Read(interval time.Duration, output chan lp.CCMessage) { return } timestamp := time.Now() - m.updateStats() prefix := "" switch m.version { @@ -134,16 +150,32 @@ func (m *nfsCollector) Read(interval time.Duration, output chan lp.CCMessage) { default: prefix = "nfs" } - for name, data := range m.data { - if _, skip := stringArrayContains(m.config.ExcludeMetrics, name); skip { - continue + // For each metric, apply filtering based on the final output name. + // Absolute metric: final name = prefix + "_" + name + if m.config.SendAbsoluteValues && m.shouldOutput(name, "") { + absMsg, err := lp.NewMessage(fmt.Sprintf("%s_%s", prefix, name), m.tags, m.meta, map[string]interface{}{"value": data.current}, timestamp) + if err == nil { + absMsg.AddMeta("version", m.version) + output <- absMsg + } } - value := data.current - data.last - y, err := lp.NewMessage(fmt.Sprintf("%s_%s", prefix, name), m.tags, m.meta, map[string]interface{}{"value": value}, timestamp) - if err == nil { - y.AddMeta("version", m.version) - output <- y + if m.config.SendDiffValues && m.shouldOutput(name, "_diff") { + diff := data.current - data.last + diffMsg, err := lp.NewMessage(fmt.Sprintf("%s_%s_diff", prefix, name), m.tags, m.meta, map[string]interface{}{"value": diff}, timestamp) + if err == nil { + diffMsg.AddMeta("version", m.version) + output <- diffMsg + } + } + if m.config.SendDerivedValues && m.shouldOutput(name, "_rate") { + diff := data.current - data.last + rate := float64(diff) / interval.Seconds() + derivedMsg, err := lp.NewMessage(fmt.Sprintf("%s_%s_rate", prefix, name), m.tags, m.meta, map[string]interface{}{"value": rate}, timestamp) + if err == nil { + derivedMsg.AddMeta("version", m.version) + output <- derivedMsg + } } } } @@ -162,14 +194,14 @@ type Nfs4Collector struct { func (m *Nfs3Collector) Init(config json.RawMessage) error { m.name = "Nfs3Collector" - m.version = `v3` + m.version = "v3" m.setup() return m.MainInit(config) } func (m *Nfs4Collector) Init(config json.RawMessage) error { m.name = "Nfs4Collector" - m.version = `v4` + m.version = "v4" m.setup() return m.MainInit(config) }