feat(metrics): add configurable metrics level filtering for Prometheus exporter #2174
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## reboot #2174 +/- ##
========================================
Coverage 92.26% 92.26%
========================================
Files 36 37 +1
Lines 3606 3763 +157
========================================
+ Hits 3327 3472 +145
- Misses 217 227 +10
- Partials 62 64 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…s exporter - Add configurable metrics level filtering to control which metric types are exported by the Prometheus exporter - Implement metrics level configuration via command line flags and YAML config - Add support for granular control over node, process, container, VM, and pod metrics - Added tests Supported metric levels: node, process, container, vm, pod, all, none Signed-off-by: Vimal Kumar <vimal78@gmail.com>
| MetricsLevelPod // 16 | ||
|
|
||
| // MetricsLevelAll represents all metrics enabled | ||
| MetricsLevelAll Level = MetricsLevelNode | MetricsLevelProcess | MetricsLevelContainer | MetricsLevelVM | MetricsLevelPod |
There was a problem hiding this comment.
Lets avoid the none and the all until there is a specific usecase.
metricsLevels: [] | null
indicates all
Is there a specific usecase of none ?
| @@ -36,6 +36,11 @@ exporter: | |||
| debugCollectors: | |||
| - go | |||
| - process | |||
| # metricsLevel: all | |||
| metricsLevel: | |||
There was a problem hiding this comment.
| metricsLevel: | |
| metricsLevels: |
to indicate multiple
| @@ -91,6 +93,55 @@ type ( | |||
| } | |||
| ) | |||
|
|
|||
| // MetricsLevelValue is a custom kingpin.Value that parses metrics levels directly into metrics.Level | |||
| type MetricsLevelValue struct { | |||
| level *metrics.Level | |||
There was a problem hiding this comment.
should this be pointer? won't value suffice?
| // Handle special cases | ||
| if value == "all" { | ||
| *m.level = metrics.MetricsLevelAll | ||
| return nil | ||
| } | ||
| if value == "none" { | ||
| *m.level = metrics.MetricsLevelNone | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Lets avoid the explicit "none" and and "all" cases until there is a specific use case.
There was a problem hiding this comment.
i don't see the need to remove these, the meanings are quite clear.
There was a problem hiding this comment.
Its not about the meaning but about the purpose ... when would user want to set kepler to not export any metric for instance?
There was a problem hiding this comment.
the argument against all is similar. By default all metrics are exported, so
metricLevels: [] | null
exports all metrics as well as ...
metricLevels:
- all
the all example looks odd IMHO.
Supported metric levels:
node, process, container, vm, pod, all, noneUsage: