Skip to content

feat: add current metrics gauge - #715

Open
yuminn-k wants to merge 1 commit into
prometheus:masterfrom
yuminn-k:feat/565-current-metrics-gauge
Open

feat: add current metrics gauge#715
yuminn-k wants to merge 1 commit into
prometheus:masterfrom
yuminn-k:feat/565-current-metrics-gauge

Conversation

@yuminn-k

Copy link
Copy Markdown

Summary

  • add statsd_exporter_metrics_current to report the current number of active metric vectors by metric type
  • keep statsd_exporter_metrics_total as the cumulative vector counter
  • update the current gauge when metric vectors become active again or expire via TTL cleanup

Fixes #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

Signed-off-by: YuMin Kim <gimyumin40@gmail.com>

@pedro-stanaka pedro-stanaka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread main.go
Comment on lines +174 to +180
metricsCurrent = promauto.NewGaugeVec(
prometheus.GaugeOpts{
Name: "statsd_exporter_metrics_current",
Help: "The current number of metric vectors with active time series.",
},
[]string{"type"},
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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"},
)

Comment thread pkg/registry/registry.go

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gauge to track the current number of metrics known [was: metricsCount metric remain the same after metric been deleted by RemoveStaleMetrics]

2 participants