prometheus: purge deleted stream metric series in streams mode (CON-555) - #4742
prometheus: purge deleted stream metric series in streams mode (CON-555)#4742squiidz wants to merge 1 commit into
Conversation
In streams mode, deleting a stream via DELETE /streams/{id} left its
metric series (labeled stream="<id>") registered in the prometheus
exporter with frozen values until the process restarted, so /metrics
accumulated series for every stream ever deleted.
Implement the optional service.MetricsExporterSeriesDeleter interface
on the prometheus exporter by calling DeletePartialMatch on every
registered counter, gauge, summary and histogram vec. The benthos
stream manager invokes it with {stream: <id>} when a stream is deleted
(or replaced by an update).
The exporter-side implementation is inert until the benthos dependency
is bumped to a release containing the stream manager hook; the unit
tests exercise the exporter directly and pass against the currently
pinned version.
| for _, pv := range p.counters { | ||
| pv.ctr.DeletePartialMatch(promLabels) | ||
| } | ||
| for _, pv := range p.gauges { | ||
| pv.ctr.DeletePartialMatch(promLabels) | ||
| } | ||
| for _, pv := range p.timers { | ||
| pv.sum.DeletePartialMatch(promLabels) | ||
| } | ||
| for _, pv := range p.timersHist { | ||
| pv.sum.DeletePartialMatch(promLabels) | ||
| } |
There was a problem hiding this comment.
DeletePartialMatch is applied unconditionally to every registered vec, including vecs that do not declare the label being matched on.
In prometheus/client_golang (pinned here at v1.23.2 per go.mod#L136), MetricVec.DeletePartialMatch → metricMap.deleteByLabels → matchPartialLabels, which looks each requested label key up in the vec's variable label names and skips the comparison entirely when the key is not one of them. If none of the requested keys are variable labels of that vec, the match is vacuously true and every series in the vec is deleted.
Failure scenario: DeleteSeriesPartialMatch({"stream": "foo"}) is called while a metric vec exists that has no stream label — e.g. the zero-label uptime counter created at metrics_prometheus_test.go#L189-L191, or any global/non-stream-scoped vec. All of its series are dropped from /metrics, so deleting one stream silently wipes unrelated counters and gauges. Note this also means the new test's own assert.Contains(t, body, "\nuptime 9") assertion at line 208 should be failing — worth confirming the test actually passes locally before merging.
Suggested fix: record the declared label names on promCounterVec/promGaugeVec/promTimingVec/promTimingHistVec (they already carry count) and skip any vec that does not declare every key present in labels before calling DeletePartialMatch.
|
|
||
| ### Fixed | ||
|
|
||
| - prometheus: In streams mode, deleting a stream now purges its metric series (labeled `stream="<id>"`) from the `/metrics` endpoint instead of exposing them with frozen values until the process restarts. ([@squiidz](https://github.com/squiidz), [#TBD](https://github.com/redpanda-data/connect/pull/TBD)) |
There was a problem hiding this comment.
The changelog entry ships a placeholder link: [#TBD](https://github.com/redpanda-data/connect/pull/TBD) resolves to a 404. Every other entry in this file uses the real PR number (see CHANGELOG.md#L16-L23). Replace TBD with this PR's number (4742) before merging.
7f49bca to
99bca83
Compare
|
|
||
| ### Fixed | ||
|
|
||
| - prometheus: In streams mode, deleting a stream now purges its metric series (labeled `stream="<id>"`) from the `/metrics` endpoint instead of exposing them with frozen values until the process restarts. ([@squiidz](https://github.com/squiidz), [#4742](https://github.com/redpanda-data/connect/pull/4742)) |
There was a problem hiding this comment.
This entry announces user-visible behavior that will not actually happen with the benthos version this PR pins. The commit body itself states the exporter-side implementation "is inert until the benthos dependency is bumped to a release containing the stream manager hook", and this PR changes only CHANGELOG.md and the two prometheus files — there is no go.mod bump, and nothing in this repo calls DeleteSeriesPartialMatch outside the new tests (see
connect/internal/impl/prometheus/metrics_prometheus.go
Lines 526 to 545 in 99bca83
As written, the next release would ship a "Fixed" note for a fix users cannot observe. Suggested fix: either include the benthos dependency bump in this PR so the stream manager actually invokes the deleter, or hold the changelog entry until that bump lands (documenting the current limitation instead, per CONTRIBUTING.md §1.2.3 "Known limitations and edge cases are documented").
Ref: CON-555
Problem
In streams mode, deleting a stream via
DELETE /streams/{id}stops the stream and removes it from the API, but its metric series (labeledstream="<id>") remain registered in the prometheus exporter forever./metricskeeps exposing every deleted stream's counters/gauges/timers with frozen values until the process restarts, growing cardinality without bound for workloads that churn streams.Fix
Implements the optional
service.MetricsExporterSeriesDeleterinterface (added in redpanda-data/benthos#487) on the prometheus exporter by callingDeletePartialMatchon every registered counter, gauge, summary and histogram vec. The benthos stream manager invokes it with{stream: <id>}when a stream is deleted (or replaced by an update — for prometheus that shows up as an ordinary counter reset, which scrapers handle).Depends on redpanda-data/benthos#487: this implementation is inert until the benthos dependency is bumped to a release containing the stream manager hook. The unit tests exercise the exporter directly and pass against the currently pinned version, so this can merge ahead of the bump.
Testing
/metrics, series for other label values and unlabelled series are untouched.go.modreplace, not part of this PR), using the reproduction from the ticket: