feat: add current metrics gauge - #715
Conversation
Signed-off-by: YuMin Kim <gimyumin40@gmail.com>
pedro-stanaka
left a comment
There was a problem hiding this comment.
The changes are too broad and introduce breaking changes to the users of project as a library. Out of curiosity, how do you use the statsd-exporter in your work/day-to-day?
| metricsCurrent = promauto.NewGaugeVec( | ||
| prometheus.GaugeOpts{ | ||
| Name: "statsd_exporter_metrics_current", | ||
| Help: "The current number of metric vectors with active time series.", | ||
| }, | ||
| []string{"type"}, | ||
| ) |
There was a problem hiding this comment.
Worth tightening this metric name and help text before release. statsd_exporter_metrics_current can be read as the current number of metrics, active time series, or current metric values, but the implementation increments only when a metric vector’s refcount transitions from zero to non-zero. That means one metric name plus label-name set contributes one, regardless of how many label-value series are active. Since scrape-facing metric names become a long-lived operator API, this ambiguity will be hard to fix later.
Suggestion: Consider renaming the metric and updating the help text, tests, and changelog entry together. For example:
| metricsCurrent = promauto.NewGaugeVec( | |
| prometheus.GaugeOpts{ | |
| Name: "statsd_exporter_metrics_current", | |
| Help: "The current number of metric vectors with active time series.", | |
| }, | |
| []string{"type"}, | |
| ) | |
| metricsCurrent = promauto.NewGaugeVec( | |
| prometheus.GaugeOpts{ | |
| Name: "statsd_exporter_active_metric_vectors", | |
| Help: "Current number of metric vectors that have at least one active time series.", | |
| }, | |
| []string{"type"}, | |
| ) |
There was a problem hiding this comment.
Consider keeping the exported Registry API source-compatible while adding this telemetry. The PR changes GetCounter, GetGauge, GetHistogram, GetSummary, and RemoveStaleMetrics to require a metricsCurrent gauge. That will break downstream users of pkg/registry, and it also makes callers pass exporter self-metric plumbing through every lookup and cleanup call. Since Registry already owns Metrics, Vectors, and RefCount, it is a natural place to hold an optional current-gauge field internally.
We should keep the existing Get* and RemoveStaleMetrics signatures stable. Add the current gauge to registry state instead, for example via a new opt-in constructor or setter such as NewRegistryWithMetricsCurrent or SetMetricsCurrent, and have Store/RemoveStaleMetrics use that optional field internally. Existing direct callers would continue to compile and would simply get no current-gauge updates unless they opt in.
| } | ||
| } | ||
|
|
||
| func TestMetricsCurrentTracksActiveMetricVectors(t *testing.T) { |
There was a problem hiding this comment.
We should extend the test after the inactive assertion to re-register the same metric vector and assert that the current gauge returns to 1.
Summary
statsd_exporter_metrics_currentto report the current number of active metric vectors by metric typestatsd_exporter_metrics_totalas the cumulative vector counterFixes #565
Testing
go test ./pkg/registry ./pkg/exporter -run 'TestMetricsCurrentTracksActiveMetricVectors|TestTtlExpiration|TestHashLabelNames' -count=1\n-go test ./pkg/exporter -run TestMetricsCurrentTracksActiveMetricVectors -race -count=1\n-go test ./... -count=1\n-git diff --check