From 384151cc6460c571d786c559d47d0b480457f3bf Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Thu, 23 Apr 2026 11:47:34 +0200 Subject: [PATCH 1/9] Add opt-in Vec TTL via MetricVec and TTLRegistry Introduce NewMetricVecWithTTL and per-child lastAccessed tracking so stale label sets can be omitted from Collect and removed with CleanupExpired. Store vec children as pointers to satisfy atomic copying rules. Keep default NewCounterVec/NewGaugeVec/NewHistogramVec on NewMetricVec without TTL for backwards compatibility. Opt-in TTLRegistry wraps a dedicated Registry, exposes New*Vec constructors with a fixed ttl, and runs CleanupExpired before each Gather. Relates to https://github.com/prometheus/client_golang/issues/1983 Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/counter.go | 30 ++++-- prometheus/gauge.go | 26 +++-- prometheus/histogram.go | 16 ++- prometheus/ttl_registry.go | 124 ++++++++++++++++++++++ prometheus/ttl_registry_test.go | 52 ++++++++++ prometheus/vec.go | 179 ++++++++++++++++++++++++++++---- prometheus/vec_test.go | 158 ++++++++++++++++++++++++++++ 7 files changed, 546 insertions(+), 39 deletions(-) create mode 100644 prometheus/ttl_registry.go create mode 100644 prometheus/ttl_registry_test.go diff --git a/prometheus/counter.go b/prometheus/counter.go index 7d963d3af..26b53400e 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -201,6 +201,12 @@ func NewCounterVec(opts CounterOpts, labelNames []string) *CounterVec { // NewCounterVec creates a new CounterVec based on the provided CounterVecOpts. func (v2) NewCounterVec(opts CounterVecOpts) *CounterVec { + return newCounterVecWithTTL(opts, 0) +} + +// newCounterVecWithTTL creates a CounterVec. ttl must be >= 0; ttl == 0 disables +// TTL behavior (identical to NewMetricVec). +func newCounterVecWithTTL(opts CounterVecOpts, ttl time.Duration) *CounterVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -211,16 +217,22 @@ func (v2) NewCounterVec(opts CounterVecOpts) *CounterVec { if opts.now == nil { opts.now = time.Now } + newMetric := func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} + result.init(result) // Init self-collection. + result.createdTs = timestamppb.New(opts.now()) + return result + } + if ttl > 0 { + return &CounterVec{ + MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), + } + } return &CounterVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} - result.init(result) // Init self-collection. - result.createdTs = timestamppb.New(opts.now()) - return result - }), + MetricVec: NewMetricVec(desc, newMetric), } } diff --git a/prometheus/gauge.go b/prometheus/gauge.go index 41e54bf27..adebbf673 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -159,6 +159,10 @@ func NewGaugeVec(opts GaugeOpts, labelNames []string) *GaugeVec { // NewGaugeVec creates a new GaugeVec based on the provided GaugeVecOpts. func (v2) NewGaugeVec(opts GaugeVecOpts) *GaugeVec { + return newGaugeVecWithTTL(opts, 0) +} + +func newGaugeVecWithTTL(opts GaugeVecOpts, ttl time.Duration) *GaugeVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -166,15 +170,21 @@ func (v2) NewGaugeVec(opts GaugeVecOpts) *GaugeVec { opts.ConstLabels, WithUnit(opts.Unit), ) + newMetric := func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} + result.init(result) // Init self-collection. + return result + } + if ttl > 0 { + return &GaugeVec{ + MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), + } + } return &GaugeVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} - result.init(result) // Init self-collection. - return result - }), + MetricVec: NewMetricVec(desc, newMetric), } } diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 0e788f715..433e75195 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -1189,6 +1189,10 @@ func NewHistogramVec(opts HistogramOpts, labelNames []string) *HistogramVec { // NewHistogramVec creates a new HistogramVec based on the provided HistogramVecOpts. func (v2) NewHistogramVec(opts HistogramVecOpts) *HistogramVec { + return newHistogramVecWithTTL(opts, 0) +} + +func newHistogramVecWithTTL(opts HistogramVecOpts, ttl time.Duration) *HistogramVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -1196,10 +1200,16 @@ func (v2) NewHistogramVec(opts HistogramVecOpts) *HistogramVec { opts.ConstLabels, WithUnit(opts.Unit), ) + newMetric := func(lvs ...string) Metric { + return newHistogram(desc, opts.HistogramOpts, lvs...) + } + if ttl > 0 { + return &HistogramVec{ + MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), + } + } return &HistogramVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - return newHistogram(desc, opts.HistogramOpts, lvs...) - }), + MetricVec: NewMetricVec(desc, newMetric), } } diff --git a/prometheus/ttl_registry.go b/prometheus/ttl_registry.go new file mode 100644 index 000000000..616b0d049 --- /dev/null +++ b/prometheus/ttl_registry.go @@ -0,0 +1,124 @@ +// Copyright 2014 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "sync" + "time" + + dto "github.com/prometheus/client_model/go" +) + +// TTLRegistry is a dedicated Prometheus registry for metrics that need +// time-to-live behavior on *Vec children. It embeds a plain *Registry and adds: +// - Vec constructors that enable per-child TTL (same semantics as MetricVec +// built with NewMetricVecWithTTL). +// - Automatic CleanupExpired on all Vecs created through this registry before +// each Gather, so memory can be reclaimed even when scrapes are infrequent +// (see discussion in https://github.com/prometheus/client_golang/issues/1983). +// +// Default prometheus.NewRegistry, NewCounterVec, and Opts are unchanged; use +// TTLRegistry only when you explicitly opt in to Vec TTL. +type TTLRegistry struct { + *Registry + + ttl time.Duration + + mu sync.Mutex + vecs []*MetricVec +} + +// NewTTLRegistry returns a registry backed by a new empty *Registry. ttl must +// be greater than zero; it applies to every Vec created via this registry's +// constructor methods. +func NewTTLRegistry(ttl time.Duration) *TTLRegistry { + if ttl <= 0 { + panic("NewTTLRegistry: ttl must be > 0") + } + return &TTLRegistry{ + Registry: NewRegistry(), + ttl: ttl, + } +} + +func (r *TTLRegistry) track(mv *MetricVec) { + r.mu.Lock() + defer r.mu.Unlock() + r.vecs = append(r.vecs, mv) +} + +func (r *TTLRegistry) runCleanup() { + r.mu.Lock() + vecs := append([]*MetricVec(nil), r.vecs...) + r.mu.Unlock() + for _, mv := range vecs { + mv.CleanupExpired() + } +} + +// Gather implements Gatherer. It runs CleanupExpired on all Vecs created +// through this TTLRegistry, then delegates to the embedded Registry. +func (r *TTLRegistry) Gather() ([]*dto.MetricFamily, error) { + r.runCleanup() + return r.Registry.Gather() +} + +// NewCounterVec is like prometheus.NewCounterVec but enables Vec TTL using this +// registry's ttl, registers the Vec, and tracks it for Gather-time cleanup. +func (r *TTLRegistry) NewCounterVec(opts CounterOpts, labelNames []string) *CounterVec { + return r.NewCounterVecOpts(CounterVecOpts{ + CounterOpts: opts, + VariableLabels: UnconstrainedLabels(labelNames), + }) +} + +// NewCounterVecOpts is like V2.NewCounterVec with TTL and automatic registration. +func (r *TTLRegistry) NewCounterVecOpts(opts CounterVecOpts) *CounterVec { + cv := newCounterVecWithTTL(opts, r.ttl) + r.MustRegister(cv) + r.track(cv.MetricVec) + return cv +} + +// NewGaugeVec is like prometheus.NewGaugeVec with TTL, registration, and tracking. +func (r *TTLRegistry) NewGaugeVec(opts GaugeOpts, labelNames []string) *GaugeVec { + return r.NewGaugeVecOpts(GaugeVecOpts{ + GaugeOpts: opts, + VariableLabels: UnconstrainedLabels(labelNames), + }) +} + +// NewGaugeVecOpts is like V2.NewGaugeVec with TTL and automatic registration. +func (r *TTLRegistry) NewGaugeVecOpts(opts GaugeVecOpts) *GaugeVec { + gv := newGaugeVecWithTTL(opts, r.ttl) + r.MustRegister(gv) + r.track(gv.MetricVec) + return gv +} + +// NewHistogramVec is like prometheus.NewHistogramVec with TTL, registration, and tracking. +func (r *TTLRegistry) NewHistogramVec(opts HistogramOpts, labelNames []string) *HistogramVec { + return r.NewHistogramVecOpts(HistogramVecOpts{ + HistogramOpts: opts, + VariableLabels: UnconstrainedLabels(labelNames), + }) +} + +// NewHistogramVecOpts is like V2.NewHistogramVec with TTL and automatic registration. +func (r *TTLRegistry) NewHistogramVecOpts(opts HistogramVecOpts) *HistogramVec { + hv := newHistogramVecWithTTL(opts, r.ttl) + r.MustRegister(hv) + r.track(hv.MetricVec) + return hv +} diff --git a/prometheus/ttl_registry_test.go b/prometheus/ttl_registry_test.go new file mode 100644 index 000000000..7627a78d7 --- /dev/null +++ b/prometheus/ttl_registry_test.go @@ -0,0 +1,52 @@ +// Copyright 2014 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "testing" + "time" +) + +func TestNewTTLRegistryPanicsOnNonPositiveTTL(t *testing.T) { + defer func() { + if recover() == nil { + t.Fatal("expected panic for ttl <= 0") + } + }() + NewTTLRegistry(0) +} + +func TestTTLRegistryGatherRunsCleanup(t *testing.T) { + ttl := 80 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "ttl_reg_gather", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric before sleep, got %d", n) + } + + time.Sleep(ttl + 40*time.Millisecond) + + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after Gather cleanup, got %d", n) + } +} diff --git a/prometheus/vec.go b/prometheus/vec.go index b405a64de..5538f66dd 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -16,6 +16,8 @@ package prometheus import ( "fmt" "sync" + "sync/atomic" + "time" "github.com/prometheus/common/model" ) @@ -47,7 +49,7 @@ type MetricVec struct { func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { return &MetricVec{ metricMap: &metricMap{ - metrics: map[uint64][]metricWithLabelValues{}, + metrics: map[uint64][]*metricWithLabelValues{}, desc: desc, newMetric: newMetric, }, @@ -56,6 +58,30 @@ func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { } } +// NewMetricVecWithTTL returns an initialized MetricVec with TTL-based expiration. +// Children that have not been accessed (via GetMetricWith or GetMetricWithLabelValues) +// for longer than ttl will be excluded from Collect and can be cleaned up via +// CleanupExpired. If ttl is 0, this behaves identically to NewMetricVec. +func NewMetricVecWithTTL(desc *Desc, newMetric func(lvs ...string) Metric, ttl time.Duration) *MetricVec { + return &MetricVec{ + metricMap: &metricMap{ + metrics: map[uint64][]*metricWithLabelValues{}, + desc: desc, + newMetric: newMetric, + ttl: ttl, + }, + hashAdd: hashAdd, + hashAddByte: hashAddByte, + } +} + +// CleanupExpired removes all children that have not been accessed within the +// configured TTL. It returns the number of children removed. If TTL is not +// configured (zero), this is a no-op and returns 0. +func (m *MetricVec) CleanupExpired() int { + return m.cleanupExpired() +} + // DeleteLabelValues removes the metric where the variable labels are the same // as those passed in as labels (same order as the VariableLabels in Desc). It // returns true if a metric was deleted. @@ -304,8 +330,9 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { // metricWithLabelValues provides the metric and its label values for // disambiguation on hash collision. type metricWithLabelValues struct { - values []string - metric Metric + values []string + metric Metric + lastAccessed atomic.Int64 // unix timestamp in milliseconds; only used when TTL > 0 } // curriedLabelValue sets the curried value for a label at the given index. @@ -318,9 +345,10 @@ type curriedLabelValue struct { // metricVecs. type metricMap struct { mtx sync.RWMutex // Protects metrics. - metrics map[uint64][]metricWithLabelValues + metrics map[uint64][]*metricWithLabelValues desc *Desc newMetric func(labelValues ...string) Metric + ttl time.Duration // if > 0, enables TTL-based expiration } // Describe implements Collector. It will send exactly one Desc to the provided @@ -334,9 +362,17 @@ func (m *metricMap) Collect(ch chan<- Metric) { m.mtx.RLock() defer m.mtx.RUnlock() + var deadline int64 + if m.ttl > 0 { + deadline = time.Now().Add(-m.ttl).UnixMilli() + } + for _, metrics := range m.metrics { - for _, metric := range metrics { - ch <- metric.metric + for i := range metrics { + if m.ttl > 0 && metrics[i].lastAccessed.Load() < deadline { + continue + } + ch <- metrics[i].metric } } } @@ -351,6 +387,93 @@ func (m *metricMap) Reset() { } } +// touchByHash updates lastAccessed for a metric found by hash and label values. +// Acquires RLock internally. +func (m *metricMap) touchByHash(h uint64, lvs []string, curry []curriedLabelValue) { + m.mtx.RLock() + defer m.mtx.RUnlock() + m.touchByHashRLocked(h, lvs, curry) +} + +func (m *metricMap) touchByHashRLocked(h uint64, lvs []string, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *metricMap) touchByHashLocked(h uint64, lvs []string, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *metricMap) touchByHashLabels(h uint64, labels Labels, curry []curriedLabelValue) { + m.mtx.RLock() + defer m.mtx.RUnlock() + m.touchByHashLabelsRLocked(h, labels, curry) +} + +func (m *metricMap) touchByHashLabelsRLocked(h uint64, labels Labels, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *metricMap) touchByHashLabelsLocked(h uint64, labels Labels, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +// cleanupExpired removes all children whose lastAccessed is older than TTL. +func (m *metricMap) cleanupExpired() int { + if m.ttl <= 0 { + return 0 + } + + deadline := time.Now().Add(-m.ttl).UnixMilli() + m.mtx.Lock() + defer m.mtx.Unlock() + + var numDeleted int + for h, metrics := range m.metrics { + remaining := metrics[:0] + for i := range metrics { + if metrics[i].lastAccessed.Load() >= deadline { + remaining = append(remaining, metrics[i]) + } else { + numDeleted++ + } + } + if len(remaining) == 0 { + delete(m.metrics, h) + } else { + m.metrics[h] = remaining + } + } + return numDeleted +} + // deleteByHashWithLabelValues removes the metric from the hash bucket h. If // there are multiple matches in the bucket, use lvs to select a metric and // remove only that metric. @@ -373,7 +496,7 @@ func (m *metricMap) deleteByHashWithLabelValues( if len(metrics) > 1 { old := metrics m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = metricWithLabelValues{} + old[len(old)-1] = nil } else { delete(m.metrics, h) } @@ -401,7 +524,7 @@ func (m *metricMap) deleteByHashWithLabels( if len(metrics) > 1 { old := metrics m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = metricWithLabelValues{} + old[len(old)-1] = nil } else { delete(m.metrics, h) } @@ -431,10 +554,10 @@ func (m *metricMap) deleteByLabels(labels Labels, curry []curriedLabelValue) int // findMetricWithPartialLabel returns the index of the matching metric or // len(metrics) if not found. func findMetricWithPartialLabels( - desc *Desc, metrics []metricWithLabelValues, labels Labels, curry []curriedLabelValue, + desc *Desc, metrics []*metricWithLabelValues, labels Labels, curry []curriedLabelValue, ) int { - for i, metric := range metrics { - if matchPartialLabels(desc, metric.values, labels, curry) { + for i := range metrics { + if matchPartialLabels(desc, metrics[i].values, labels, curry) { return i } } @@ -495,6 +618,9 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( metric, ok := m.getMetricWithHashAndLabelValues(hash, lvs, curry) m.mtx.RUnlock() if ok { + if m.ttl > 0 { + m.touchByHash(hash, lvs, curry) + } return metric } @@ -504,7 +630,13 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( if !ok { inlinedLVs := inlineLabelValues(lvs, curry) metric = m.newMetric(inlinedLVs...) - m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: inlinedLVs, metric: metric}) + entry := &metricWithLabelValues{values: inlinedLVs, metric: metric} + if m.ttl > 0 { + entry.lastAccessed.Store(time.Now().UnixMilli()) + } + m.metrics[hash] = append(m.metrics[hash], entry) + } else if m.ttl > 0 { + m.touchByHashLocked(hash, lvs, curry) } return metric } @@ -520,6 +652,9 @@ func (m *metricMap) getOrCreateMetricWithLabels( metric, ok := m.getMetricWithHashAndLabels(hash, labels, curry) m.mtx.RUnlock() if ok { + if m.ttl > 0 { + m.touchByHashLabels(hash, labels, curry) + } return metric } @@ -529,7 +664,13 @@ func (m *metricMap) getOrCreateMetricWithLabels( if !ok { lvs := extractLabelValues(m.desc, labels, curry) metric = m.newMetric(lvs...) - m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: lvs, metric: metric}) + entry := &metricWithLabelValues{values: lvs, metric: metric} + if m.ttl > 0 { + entry.lastAccessed.Store(time.Now().UnixMilli()) + } + m.metrics[hash] = append(m.metrics[hash], entry) + } else if m.ttl > 0 { + m.touchByHashLabelsLocked(hash, labels, curry) } return metric } @@ -565,10 +706,10 @@ func (m *metricMap) getMetricWithHashAndLabels( // findMetricWithLabelValues returns the index of the matching metric or // len(metrics) if not found. func findMetricWithLabelValues( - metrics []metricWithLabelValues, lvs []string, curry []curriedLabelValue, + metrics []*metricWithLabelValues, lvs []string, curry []curriedLabelValue, ) int { - for i, metric := range metrics { - if matchLabelValues(metric.values, lvs, curry) { + for i := range metrics { + if matchLabelValues(metrics[i].values, lvs, curry) { return i } } @@ -578,10 +719,10 @@ func findMetricWithLabelValues( // findMetricWithLabels returns the index of the matching metric or len(metrics) // if not found. func findMetricWithLabels( - desc *Desc, metrics []metricWithLabelValues, labels Labels, curry []curriedLabelValue, + desc *Desc, metrics []*metricWithLabelValues, labels Labels, curry []curriedLabelValue, ) int { - for i, metric := range metrics { - if matchLabels(desc, metric.values, labels, curry) { + for i := range metrics { + if matchLabels(desc, metrics[i].values, labels, curry) { return i } } diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 03223f2f6..c08482448 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -18,6 +18,7 @@ import ( "reflect" "strconv" "testing" + "time" dto "github.com/prometheus/client_model/go" ) @@ -1004,3 +1005,160 @@ func benchmarkMetricVecWithLabelValues(b *testing.B, labels map[string][]string) vec.WithLabelValues(values...) } } + +func collectCount(c Collector) int { + ch := make(chan Metric, 100) + c.Collect(ch) + close(ch) + n := 0 + for range ch { + n++ + } + return n +} + +func TestTTLCounterVec(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "test_ttl_counter", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + vec.WithLabelValues("404").Add(1) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } + + cleaned := vec.CleanupExpired() + if cleaned != 2 { + t.Fatalf("expected 2 cleaned, got %d", cleaned) + } +} + +func TestTTLGaugeVec(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewGaugeVec(GaugeOpts{ + Name: "test_ttl_gauge", + Help: "test", + }, []string{"method"}) + + vec.WithLabelValues("GET").Set(10) + vec.WithLabelValues("POST").Set(20) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + // Touch only one + vec.WithLabelValues("GET").Set(30) + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric after partial TTL, got %d", n) + } + + cleaned := vec.CleanupExpired() + if cleaned != 1 { + t.Fatalf("expected 1 cleaned, got %d", cleaned) + } +} + +func TestTTLHistogramVec(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewHistogramVec(HistogramOpts{ + Name: "test_ttl_histo", + Help: "test", + }, []string{"status"}) + + vec.WithLabelValues("ok").Observe(0.5) + vec.WithLabelValues("err").Observe(1.5) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } +} + +func TestTTLRefreshPreventsExpiration(t *testing.T) { + ttl := 150 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "test_ttl_refresh", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + + // Keep refreshing before TTL expires + for i := 0; i < 5; i++ { + time.Sleep(80 * time.Millisecond) + vec.WithLabelValues("200").Add(1) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric still alive, got %d", n) + } +} + +func TestTTLZeroMeansNoExpiration(t *testing.T) { + vec := NewCounterVec(CounterOpts{ + Name: "test_no_ttl", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + + cleaned := vec.CleanupExpired() + if cleaned != 0 { + t.Fatalf("expected 0 cleaned with no TTL, got %d", cleaned) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric, got %d", n) + } +} + +func TestTTLWithGetMetricWith(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewGaugeVec(GaugeOpts{ + Name: "test_ttl_getmetricwith", + Help: "test", + }, []string{"method"}) + + g, err := vec.GetMetricWith(Labels{"method": "GET"}) + if err != nil { + t.Fatal(err) + } + g.Set(42) + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 after TTL, got %d", n) + } + + // Re-access refreshes + g, _ = vec.GetMetricWith(Labels{"method": "GET"}) + g.Set(99) + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 after re-access, got %d", n) + } +} From cf4b9e9d79f4542ae16d5b6dfe5d5db5565893c0 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 19 May 2026 12:23:36 +0200 Subject: [PATCH 2/9] Separate TTL Vec path and use synctest in TTL tests Move TTL logic to ttlMetricMap so metricMap stays unchanged for default Vecs. Wrap TTL tests with synctest instead of real sleeps. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/ttl_registry_test.go | 45 ++--- prometheus/ttl_vec.go | 323 ++++++++++++++++++++++++++++++++ prometheus/vec.go | 233 ++++++++--------------- prometheus/vec_test.go | 195 ++++++++++--------- 4 files changed, 531 insertions(+), 265 deletions(-) create mode 100644 prometheus/ttl_vec.go diff --git a/prometheus/ttl_registry_test.go b/prometheus/ttl_registry_test.go index 7627a78d7..7f1757e4d 100644 --- a/prometheus/ttl_registry_test.go +++ b/prometheus/ttl_registry_test.go @@ -15,6 +15,7 @@ package prometheus import ( "testing" + "testing/synctest" "time" ) @@ -28,25 +29,27 @@ func TestNewTTLRegistryPanicsOnNonPositiveTTL(t *testing.T) { } func TestTTLRegistryGatherRunsCleanup(t *testing.T) { - ttl := 80 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "ttl_reg_gather", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric before sleep, got %d", n) - } - - time.Sleep(ttl + 40*time.Millisecond) - - if _, err := reg.Gather(); err != nil { - t.Fatal(err) - } - - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after Gather cleanup, got %d", n) - } + synctest.Test(t, func(t *testing.T) { + ttl := 80 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "ttl_reg_gather", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric before sleep, got %d", n) + } + + time.Sleep(ttl + 40*time.Millisecond) + + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after Gather cleanup, got %d", n) + } + }) } diff --git a/prometheus/ttl_vec.go b/prometheus/ttl_vec.go new file mode 100644 index 000000000..3fe0f84af --- /dev/null +++ b/prometheus/ttl_vec.go @@ -0,0 +1,323 @@ +// Copyright 2014 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "sync" + "sync/atomic" + "time" +) + +// ttlMetricWithLabelValues is the TTL variant of metricWithLabelValues. +type ttlMetricWithLabelValues struct { + values []string + metric Metric + lastAccessed atomic.Int64 // unix timestamp in milliseconds +} + +// ttlMetricMap backs MetricVec instances created with NewMetricVecWithTTL. +// It is separate from metricMap so the default Vec path stays unchanged. +type ttlMetricMap struct { + mtx sync.RWMutex + metrics map[uint64][]*ttlMetricWithLabelValues + desc *Desc + newMetric func(labelValues ...string) Metric + ttl time.Duration +} + +func (m *ttlMetricMap) Describe(ch chan<- *Desc) { + ch <- m.desc +} + +func (m *ttlMetricMap) Collect(ch chan<- Metric) { + m.mtx.RLock() + defer m.mtx.RUnlock() + + deadline := time.Now().Add(-m.ttl).UnixMilli() + for _, metrics := range m.metrics { + for i := range metrics { + if metrics[i].lastAccessed.Load() < deadline { + continue + } + ch <- metrics[i].metric + } + } +} + +func (m *ttlMetricMap) Reset() { + m.mtx.Lock() + defer m.mtx.Unlock() + + for h := range m.metrics { + delete(m.metrics, h) + } +} + +func (m *ttlMetricMap) touchByHash(h uint64, lvs []string, curry []curriedLabelValue) { + m.mtx.RLock() + defer m.mtx.RUnlock() + m.touchByHashRLocked(h, lvs, curry) +} + +func (m *ttlMetricMap) touchByHashRLocked(h uint64, lvs []string, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *ttlMetricMap) touchByHashLocked(h uint64, lvs []string, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *ttlMetricMap) touchByHashLabels(h uint64, labels Labels, curry []curriedLabelValue) { + m.mtx.RLock() + defer m.mtx.RUnlock() + m.touchByHashLabelsRLocked(h, labels, curry) +} + +func (m *ttlMetricMap) touchByHashLabelsRLocked(h uint64, labels Labels, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *ttlMetricMap) touchByHashLabelsLocked(h uint64, labels Labels, curry []curriedLabelValue) { + now := time.Now().UnixMilli() + metrics, ok := m.metrics[h] + if !ok { + return + } + if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { + metrics[i].lastAccessed.Store(now) + } +} + +func (m *ttlMetricMap) cleanupExpired() int { + deadline := time.Now().Add(-m.ttl).UnixMilli() + m.mtx.Lock() + defer m.mtx.Unlock() + + var numDeleted int + for h, metrics := range m.metrics { + remaining := metrics[:0] + for i := range metrics { + if metrics[i].lastAccessed.Load() >= deadline { + remaining = append(remaining, metrics[i]) + } else { + numDeleted++ + } + } + if len(remaining) == 0 { + delete(m.metrics, h) + } else { + m.metrics[h] = remaining + } + } + return numDeleted +} + +func (m *ttlMetricMap) deleteByHashWithLabelValues( + h uint64, lvs []string, curry []curriedLabelValue, +) bool { + m.mtx.Lock() + defer m.mtx.Unlock() + + metrics, ok := m.metrics[h] + if !ok { + return false + } + + i := findTTLMetricWithLabelValues(metrics, lvs, curry) + if i >= len(metrics) { + return false + } + + if len(metrics) > 1 { + old := metrics + m.metrics[h] = append(metrics[:i], metrics[i+1:]...) + old[len(old)-1] = nil + } else { + delete(m.metrics, h) + } + return true +} + +func (m *ttlMetricMap) deleteByHashWithLabels( + h uint64, labels Labels, curry []curriedLabelValue, +) bool { + m.mtx.Lock() + defer m.mtx.Unlock() + + metrics, ok := m.metrics[h] + if !ok { + return false + } + i := findTTLMetricWithLabels(m.desc, metrics, labels, curry) + if i >= len(metrics) { + return false + } + + if len(metrics) > 1 { + old := metrics + m.metrics[h] = append(metrics[:i], metrics[i+1:]...) + old[len(old)-1] = nil + } else { + delete(m.metrics, h) + } + return true +} + +func (m *ttlMetricMap) deleteByLabels(labels Labels, curry []curriedLabelValue) int { + m.mtx.Lock() + defer m.mtx.Unlock() + + var numDeleted int + + for h, metrics := range m.metrics { + i := findTTLMetricWithPartialLabels(m.desc, metrics, labels, curry) + if i >= len(metrics) { + continue + } + delete(m.metrics, h) + numDeleted++ + } + + return numDeleted +} + +func findTTLMetricWithPartialLabels( + desc *Desc, metrics []*ttlMetricWithLabelValues, labels Labels, curry []curriedLabelValue, +) int { + for i := range metrics { + if matchPartialLabels(desc, metrics[i].values, labels, curry) { + return i + } + } + return len(metrics) +} + +func (m *ttlMetricMap) getOrCreateMetricWithLabelValues( + hash uint64, lvs []string, curry []curriedLabelValue, +) Metric { + m.mtx.RLock() + metric, ok := m.getMetricWithHashAndLabelValues(hash, lvs, curry) + m.mtx.RUnlock() + if ok { + m.touchByHash(hash, lvs, curry) + return metric + } + + m.mtx.Lock() + defer m.mtx.Unlock() + metric, ok = m.getMetricWithHashAndLabelValues(hash, lvs, curry) + if !ok { + inlinedLVs := inlineLabelValues(lvs, curry) + metric = m.newMetric(inlinedLVs...) + entry := &ttlMetricWithLabelValues{values: inlinedLVs, metric: metric} + entry.lastAccessed.Store(time.Now().UnixMilli()) + m.metrics[hash] = append(m.metrics[hash], entry) + } else { + m.touchByHashLocked(hash, lvs, curry) + } + return metric +} + +func (m *ttlMetricMap) getOrCreateMetricWithLabels( + hash uint64, labels Labels, curry []curriedLabelValue, +) Metric { + m.mtx.RLock() + metric, ok := m.getMetricWithHashAndLabels(hash, labels, curry) + m.mtx.RUnlock() + if ok { + m.touchByHashLabels(hash, labels, curry) + return metric + } + + m.mtx.Lock() + defer m.mtx.Unlock() + metric, ok = m.getMetricWithHashAndLabels(hash, labels, curry) + if !ok { + lvs := extractLabelValues(m.desc, labels, curry) + metric = m.newMetric(lvs...) + entry := &ttlMetricWithLabelValues{values: lvs, metric: metric} + entry.lastAccessed.Store(time.Now().UnixMilli()) + m.metrics[hash] = append(m.metrics[hash], entry) + } else { + m.touchByHashLabelsLocked(hash, labels, curry) + } + return metric +} + +func (m *ttlMetricMap) getMetricWithHashAndLabelValues( + h uint64, lvs []string, curry []curriedLabelValue, +) (Metric, bool) { + metrics, ok := m.metrics[h] + if ok { + if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { + return metrics[i].metric, true + } + } + return nil, false +} + +func (m *ttlMetricMap) getMetricWithHashAndLabels( + h uint64, labels Labels, curry []curriedLabelValue, +) (Metric, bool) { + metrics, ok := m.metrics[h] + if ok { + if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { + return metrics[i].metric, true + } + } + return nil, false +} + +func findTTLMetricWithLabelValues( + metrics []*ttlMetricWithLabelValues, lvs []string, curry []curriedLabelValue, +) int { + for i := range metrics { + if matchLabelValues(metrics[i].values, lvs, curry) { + return i + } + } + return len(metrics) +} + +func findTTLMetricWithLabels( + desc *Desc, metrics []*ttlMetricWithLabelValues, labels Labels, curry []curriedLabelValue, +) int { + for i := range metrics { + if matchLabels(desc, metrics[i].values, labels, curry) { + return i + } + } + return len(metrics) +} diff --git a/prometheus/vec.go b/prometheus/vec.go index 5538f66dd..1f563b4b8 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -16,7 +16,6 @@ package prometheus import ( "fmt" "sync" - "sync/atomic" "time" "github.com/prometheus/common/model" @@ -37,6 +36,7 @@ import ( // panic instead of returning errors. See also the MetricVec example. type MetricVec struct { *metricMap + ttlMap *ttlMetricMap curry []curriedLabelValue @@ -49,7 +49,7 @@ type MetricVec struct { func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { return &MetricVec{ metricMap: &metricMap{ - metrics: map[uint64][]*metricWithLabelValues{}, + metrics: map[uint64][]metricWithLabelValues{}, desc: desc, newMetric: newMetric, }, @@ -64,8 +64,8 @@ func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { // CleanupExpired. If ttl is 0, this behaves identically to NewMetricVec. func NewMetricVecWithTTL(desc *Desc, newMetric func(lvs ...string) Metric, ttl time.Duration) *MetricVec { return &MetricVec{ - metricMap: &metricMap{ - metrics: map[uint64][]*metricWithLabelValues{}, + ttlMap: &ttlMetricMap{ + metrics: map[uint64][]*ttlMetricWithLabelValues{}, desc: desc, newMetric: newMetric, ttl: ttl, @@ -79,7 +79,17 @@ func NewMetricVecWithTTL(desc *Desc, newMetric func(lvs ...string) Metric, ttl t // configured TTL. It returns the number of children removed. If TTL is not // configured (zero), this is a no-op and returns 0. func (m *MetricVec) CleanupExpired() int { - return m.cleanupExpired() + if m.ttlMap == nil { + return 0 + } + return m.ttlMap.cleanupExpired() +} + +func (m *MetricVec) vecDesc() *Desc { + if m.ttlMap != nil { + return m.ttlMap.desc + } + return m.desc } // DeleteLabelValues removes the metric where the variable labels are the same @@ -98,13 +108,16 @@ func (m *MetricVec) CleanupExpired() int { // with a performance overhead (for creating and processing the Labels map). // See also the CounterVec example. func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { - lvs = constrainLabelValues(m.desc, lvs, m.curry) + lvs = constrainLabelValues(m.vecDesc(), lvs, m.curry) h, err := m.hashLabelValues(lvs) if err != nil { return false } + if m.ttlMap != nil { + return m.ttlMap.deleteByHashWithLabelValues(h, lvs, m.curry) + } return m.deleteByHashWithLabelValues(h, lvs, m.curry) } @@ -119,7 +132,7 @@ func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { // This method is used for the same purpose as DeleteLabelValues(...string). See // there for pros and cons of the two methods. func (m *MetricVec) Delete(labels Labels) bool { - labels, closer := constrainLabels(m.desc, labels) + labels, closer := constrainLabels(m.vecDesc(), labels) defer closer() h, err := m.hashLabels(labels) @@ -127,6 +140,9 @@ func (m *MetricVec) Delete(labels Labels) bool { return false } + if m.ttlMap != nil { + return m.ttlMap.deleteByHashWithLabels(h, labels, m.curry) + } return m.deleteByHashWithLabels(h, labels, m.curry) } @@ -137,9 +153,12 @@ func (m *MetricVec) Delete(labels Labels) bool { // Note that curried labels will never be matched if deleting from the curried vector. // To match curried labels with DeletePartialMatch, it must be called on the base vector. func (m *MetricVec) DeletePartialMatch(labels Labels) int { - labels, closer := constrainLabels(m.desc, labels) + labels, closer := constrainLabels(m.vecDesc(), labels) defer closer() + if m.ttlMap != nil { + return m.ttlMap.deleteByLabels(labels, m.curry) + } return m.deleteByLabels(labels, m.curry) } @@ -147,13 +166,31 @@ func (m *MetricVec) DeletePartialMatch(labels Labels) int { // show up in GoDoc. // Describe implements Collector. -func (m *MetricVec) Describe(ch chan<- *Desc) { m.metricMap.Describe(ch) } +func (m *MetricVec) Describe(ch chan<- *Desc) { + if m.ttlMap != nil { + m.ttlMap.Describe(ch) + return + } + m.metricMap.Describe(ch) +} // Collect implements Collector. -func (m *MetricVec) Collect(ch chan<- Metric) { m.metricMap.Collect(ch) } +func (m *MetricVec) Collect(ch chan<- Metric) { + if m.ttlMap != nil { + m.ttlMap.Collect(ch) + return + } + m.metricMap.Collect(ch) +} // Reset deletes all metrics in this vector. -func (m *MetricVec) Reset() { m.metricMap.Reset() } +func (m *MetricVec) Reset() { + if m.ttlMap != nil { + m.ttlMap.Reset() + return + } + m.metricMap.Reset() +} // CurryWith returns a vector curried with the provided labels, i.e. the // returned vector has those labels pre-set for all labeled operations performed @@ -178,7 +215,7 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { oldCurry = m.curry iCurry int ) - for i, labelName := range m.desc.variableLabels.names { + for i, labelName := range m.vecDesc().variableLabels.names { val, ok := labels[labelName] if iCurry < len(oldCurry) && oldCurry[iCurry].index == i { if ok { @@ -192,7 +229,7 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { } newCurry = append(newCurry, curriedLabelValue{ i, - m.desc.variableLabels.constrain(labelName, val), + m.vecDesc().variableLabels.constrain(labelName, val), }) } } @@ -202,6 +239,7 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { return &MetricVec{ metricMap: m.metricMap, + ttlMap: m.ttlMap, curry: newCurry, hashAdd: m.hashAdd, hashAddByte: m.hashAddByte, @@ -238,12 +276,15 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { // a wrapper around MetricVec, implementing a vector for a specific Metric // implementation, for example GaugeVec. func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) { - lvs = constrainLabelValues(m.desc, lvs, m.curry) + lvs = constrainLabelValues(m.vecDesc(), lvs, m.curry) h, err := m.hashLabelValues(lvs) if err != nil { return nil, err } + if m.ttlMap != nil { + return m.ttlMap.getOrCreateMetricWithLabelValues(h, lvs, m.curry), nil + } return m.getOrCreateMetricWithLabelValues(h, lvs, m.curry), nil } @@ -264,7 +305,7 @@ func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) { // around MetricVec, implementing a vector for a specific Metric implementation, // for example GaugeVec. func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) { - labels, closer := constrainLabels(m.desc, labels) + labels, closer := constrainLabels(m.vecDesc(), labels) defer closer() h, err := m.hashLabels(labels) @@ -272,11 +313,14 @@ func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) { return nil, err } + if m.ttlMap != nil { + return m.ttlMap.getOrCreateMetricWithLabels(h, labels, m.curry), nil + } return m.getOrCreateMetricWithLabels(h, labels, m.curry), nil } func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { - if err := validateLabelValues(vals, len(m.desc.variableLabels.names)-len(m.curry)); err != nil { + if err := validateLabelValues(vals, len(m.vecDesc().variableLabels.names)-len(m.curry)); err != nil { return 0, err } @@ -285,7 +329,7 @@ func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { curry = m.curry iVals, iCurry int ) - for i := 0; i < len(m.desc.variableLabels.names); i++ { + for i := 0; i < len(m.vecDesc().variableLabels.names); i++ { if iCurry < len(curry) && curry[iCurry].index == i { h = m.hashAdd(h, curry[iCurry].value) iCurry++ @@ -299,7 +343,7 @@ func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { } func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { - if err := validateValuesInLabels(labels, len(m.desc.variableLabels.names)-len(m.curry)); err != nil { + if err := validateValuesInLabels(labels, len(m.vecDesc().variableLabels.names)-len(m.curry)); err != nil { return 0, err } @@ -308,7 +352,7 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { curry = m.curry iCurry int ) - for i, labelName := range m.desc.variableLabels.names { + for i, labelName := range m.vecDesc().variableLabels.names { val, ok := labels[labelName] if iCurry < len(curry) && curry[iCurry].index == i { if ok { @@ -330,9 +374,8 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { // metricWithLabelValues provides the metric and its label values for // disambiguation on hash collision. type metricWithLabelValues struct { - values []string - metric Metric - lastAccessed atomic.Int64 // unix timestamp in milliseconds; only used when TTL > 0 + values []string + metric Metric } // curriedLabelValue sets the curried value for a label at the given index. @@ -345,10 +388,9 @@ type curriedLabelValue struct { // metricVecs. type metricMap struct { mtx sync.RWMutex // Protects metrics. - metrics map[uint64][]*metricWithLabelValues + metrics map[uint64][]metricWithLabelValues desc *Desc newMetric func(labelValues ...string) Metric - ttl time.Duration // if > 0, enables TTL-based expiration } // Describe implements Collector. It will send exactly one Desc to the provided @@ -362,17 +404,9 @@ func (m *metricMap) Collect(ch chan<- Metric) { m.mtx.RLock() defer m.mtx.RUnlock() - var deadline int64 - if m.ttl > 0 { - deadline = time.Now().Add(-m.ttl).UnixMilli() - } - for _, metrics := range m.metrics { - for i := range metrics { - if m.ttl > 0 && metrics[i].lastAccessed.Load() < deadline { - continue - } - ch <- metrics[i].metric + for _, metric := range metrics { + ch <- metric.metric } } } @@ -387,93 +421,6 @@ func (m *metricMap) Reset() { } } -// touchByHash updates lastAccessed for a metric found by hash and label values. -// Acquires RLock internally. -func (m *metricMap) touchByHash(h uint64, lvs []string, curry []curriedLabelValue) { - m.mtx.RLock() - defer m.mtx.RUnlock() - m.touchByHashRLocked(h, lvs, curry) -} - -func (m *metricMap) touchByHashRLocked(h uint64, lvs []string, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *metricMap) touchByHashLocked(h uint64, lvs []string, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *metricMap) touchByHashLabels(h uint64, labels Labels, curry []curriedLabelValue) { - m.mtx.RLock() - defer m.mtx.RUnlock() - m.touchByHashLabelsRLocked(h, labels, curry) -} - -func (m *metricMap) touchByHashLabelsRLocked(h uint64, labels Labels, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *metricMap) touchByHashLabelsLocked(h uint64, labels Labels, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -// cleanupExpired removes all children whose lastAccessed is older than TTL. -func (m *metricMap) cleanupExpired() int { - if m.ttl <= 0 { - return 0 - } - - deadline := time.Now().Add(-m.ttl).UnixMilli() - m.mtx.Lock() - defer m.mtx.Unlock() - - var numDeleted int - for h, metrics := range m.metrics { - remaining := metrics[:0] - for i := range metrics { - if metrics[i].lastAccessed.Load() >= deadline { - remaining = append(remaining, metrics[i]) - } else { - numDeleted++ - } - } - if len(remaining) == 0 { - delete(m.metrics, h) - } else { - m.metrics[h] = remaining - } - } - return numDeleted -} - // deleteByHashWithLabelValues removes the metric from the hash bucket h. If // there are multiple matches in the bucket, use lvs to select a metric and // remove only that metric. @@ -496,7 +443,7 @@ func (m *metricMap) deleteByHashWithLabelValues( if len(metrics) > 1 { old := metrics m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = nil + old[len(old)-1] = metricWithLabelValues{} } else { delete(m.metrics, h) } @@ -524,7 +471,7 @@ func (m *metricMap) deleteByHashWithLabels( if len(metrics) > 1 { old := metrics m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = nil + old[len(old)-1] = metricWithLabelValues{} } else { delete(m.metrics, h) } @@ -554,10 +501,10 @@ func (m *metricMap) deleteByLabels(labels Labels, curry []curriedLabelValue) int // findMetricWithPartialLabel returns the index of the matching metric or // len(metrics) if not found. func findMetricWithPartialLabels( - desc *Desc, metrics []*metricWithLabelValues, labels Labels, curry []curriedLabelValue, + desc *Desc, metrics []metricWithLabelValues, labels Labels, curry []curriedLabelValue, ) int { - for i := range metrics { - if matchPartialLabels(desc, metrics[i].values, labels, curry) { + for i, metric := range metrics { + if matchPartialLabels(desc, metric.values, labels, curry) { return i } } @@ -618,9 +565,6 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( metric, ok := m.getMetricWithHashAndLabelValues(hash, lvs, curry) m.mtx.RUnlock() if ok { - if m.ttl > 0 { - m.touchByHash(hash, lvs, curry) - } return metric } @@ -630,13 +574,7 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( if !ok { inlinedLVs := inlineLabelValues(lvs, curry) metric = m.newMetric(inlinedLVs...) - entry := &metricWithLabelValues{values: inlinedLVs, metric: metric} - if m.ttl > 0 { - entry.lastAccessed.Store(time.Now().UnixMilli()) - } - m.metrics[hash] = append(m.metrics[hash], entry) - } else if m.ttl > 0 { - m.touchByHashLocked(hash, lvs, curry) + m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: inlinedLVs, metric: metric}) } return metric } @@ -652,9 +590,6 @@ func (m *metricMap) getOrCreateMetricWithLabels( metric, ok := m.getMetricWithHashAndLabels(hash, labels, curry) m.mtx.RUnlock() if ok { - if m.ttl > 0 { - m.touchByHashLabels(hash, labels, curry) - } return metric } @@ -664,13 +599,7 @@ func (m *metricMap) getOrCreateMetricWithLabels( if !ok { lvs := extractLabelValues(m.desc, labels, curry) metric = m.newMetric(lvs...) - entry := &metricWithLabelValues{values: lvs, metric: metric} - if m.ttl > 0 { - entry.lastAccessed.Store(time.Now().UnixMilli()) - } - m.metrics[hash] = append(m.metrics[hash], entry) - } else if m.ttl > 0 { - m.touchByHashLabelsLocked(hash, labels, curry) + m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: lvs, metric: metric}) } return metric } @@ -706,10 +635,10 @@ func (m *metricMap) getMetricWithHashAndLabels( // findMetricWithLabelValues returns the index of the matching metric or // len(metrics) if not found. func findMetricWithLabelValues( - metrics []*metricWithLabelValues, lvs []string, curry []curriedLabelValue, + metrics []metricWithLabelValues, lvs []string, curry []curriedLabelValue, ) int { - for i := range metrics { - if matchLabelValues(metrics[i].values, lvs, curry) { + for i, metric := range metrics { + if matchLabelValues(metric.values, lvs, curry) { return i } } @@ -719,10 +648,10 @@ func findMetricWithLabelValues( // findMetricWithLabels returns the index of the matching metric or len(metrics) // if not found. func findMetricWithLabels( - desc *Desc, metrics []*metricWithLabelValues, labels Labels, curry []curriedLabelValue, + desc *Desc, metrics []metricWithLabelValues, labels Labels, curry []curriedLabelValue, ) int { - for i := range metrics { - if matchLabels(desc, metrics[i].values, labels, curry) { + for i, metric := range metrics { + if matchLabels(desc, metric.values, labels, curry) { return i } } diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index c08482448..cd43da30a 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -18,6 +18,7 @@ import ( "reflect" "strconv" "testing" + "testing/synctest" "time" dto "github.com/prometheus/client_model/go" @@ -1018,102 +1019,110 @@ func collectCount(c Collector) int { } func TestTTLCounterVec(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "test_ttl_counter", - Help: "test", - }, []string{"code"}) + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "test_ttl_counter", + Help: "test", + }, []string{"code"}) - vec.WithLabelValues("200").Add(1) - vec.WithLabelValues("404").Add(1) + vec.WithLabelValues("200").Add(1) + vec.WithLabelValues("404").Add(1) - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } - time.Sleep(ttl + 50*time.Millisecond) + time.Sleep(ttl + 50*time.Millisecond) - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after TTL, got %d", n) - } + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } - cleaned := vec.CleanupExpired() - if cleaned != 2 { - t.Fatalf("expected 2 cleaned, got %d", cleaned) - } + cleaned := vec.CleanupExpired() + if cleaned != 2 { + t.Fatalf("expected 2 cleaned, got %d", cleaned) + } + }) } func TestTTLGaugeVec(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewGaugeVec(GaugeOpts{ - Name: "test_ttl_gauge", - Help: "test", - }, []string{"method"}) + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewGaugeVec(GaugeOpts{ + Name: "test_ttl_gauge", + Help: "test", + }, []string{"method"}) - vec.WithLabelValues("GET").Set(10) - vec.WithLabelValues("POST").Set(20) + vec.WithLabelValues("GET").Set(10) + vec.WithLabelValues("POST").Set(20) - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } - time.Sleep(ttl + 50*time.Millisecond) - // Touch only one - vec.WithLabelValues("GET").Set(30) + time.Sleep(ttl + 50*time.Millisecond) + // Touch only one + vec.WithLabelValues("GET").Set(30) - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric after partial TTL, got %d", n) - } + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric after partial TTL, got %d", n) + } - cleaned := vec.CleanupExpired() - if cleaned != 1 { - t.Fatalf("expected 1 cleaned, got %d", cleaned) - } + cleaned := vec.CleanupExpired() + if cleaned != 1 { + t.Fatalf("expected 1 cleaned, got %d", cleaned) + } + }) } func TestTTLHistogramVec(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewHistogramVec(HistogramOpts{ - Name: "test_ttl_histo", - Help: "test", - }, []string{"status"}) + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewHistogramVec(HistogramOpts{ + Name: "test_ttl_histo", + Help: "test", + }, []string{"status"}) - vec.WithLabelValues("ok").Observe(0.5) - vec.WithLabelValues("err").Observe(1.5) + vec.WithLabelValues("ok").Observe(0.5) + vec.WithLabelValues("err").Observe(1.5) - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } - time.Sleep(ttl + 50*time.Millisecond) + time.Sleep(ttl + 50*time.Millisecond) - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after TTL, got %d", n) - } + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } + }) } func TestTTLRefreshPreventsExpiration(t *testing.T) { - ttl := 150 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "test_ttl_refresh", - Help: "test", - }, []string{"code"}) + synctest.Test(t, func(t *testing.T) { + ttl := 150 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "test_ttl_refresh", + Help: "test", + }, []string{"code"}) - vec.WithLabelValues("200").Add(1) - - // Keep refreshing before TTL expires - for i := 0; i < 5; i++ { - time.Sleep(80 * time.Millisecond) vec.WithLabelValues("200").Add(1) - } - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric still alive, got %d", n) - } + // Keep refreshing before TTL expires + for i := 0; i < 5; i++ { + time.Sleep(80 * time.Millisecond) + vec.WithLabelValues("200").Add(1) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric still alive, got %d", n) + } + }) } func TestTTLZeroMeansNoExpiration(t *testing.T) { @@ -1135,30 +1144,32 @@ func TestTTLZeroMeansNoExpiration(t *testing.T) { } func TestTTLWithGetMetricWith(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewGaugeVec(GaugeOpts{ - Name: "test_ttl_getmetricwith", - Help: "test", - }, []string{"method"}) - - g, err := vec.GetMetricWith(Labels{"method": "GET"}) - if err != nil { - t.Fatal(err) - } - g.Set(42) + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewGaugeVec(GaugeOpts{ + Name: "test_ttl_getmetricwith", + Help: "test", + }, []string{"method"}) + + g, err := vec.GetMetricWith(Labels{"method": "GET"}) + if err != nil { + t.Fatal(err) + } + g.Set(42) - time.Sleep(ttl + 50*time.Millisecond) + time.Sleep(ttl + 50*time.Millisecond) - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 after TTL, got %d", n) - } + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 after TTL, got %d", n) + } - // Re-access refreshes - g, _ = vec.GetMetricWith(Labels{"method": "GET"}) - g.Set(99) + // Re-access refreshes + g, _ = vec.GetMetricWith(Labels{"method": "GET"}) + g.Set(99) - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 after re-access, got %d", n) - } + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 after re-access, got %d", n) + } + }) } From e8d0bdf260625461ba2e11154b73b26699e18866 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 19 May 2026 14:52:31 +0200 Subject: [PATCH 3/9] Fix TTL edge cases from review feedback Handle ttl<=0 in NewMetricVecWithTTL, nil slice tails in cleanupExpired for GC, and untrack Vecs on TTLRegistry.Unregister. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/ttl_registry.go | 41 +++++++++++++++++++++++++++++++++ prometheus/ttl_registry_test.go | 40 ++++++++++++++++++++++++++++++++ prometheus/ttl_vec.go | 4 ++++ prometheus/vec.go | 9 +++++++- prometheus/vec_test.go | 17 ++++++++++++++ 5 files changed, 110 insertions(+), 1 deletion(-) diff --git a/prometheus/ttl_registry.go b/prometheus/ttl_registry.go index 616b0d049..fa30f7c0a 100644 --- a/prometheus/ttl_registry.go +++ b/prometheus/ttl_registry.go @@ -58,6 +58,47 @@ func (r *TTLRegistry) track(mv *MetricVec) { r.vecs = append(r.vecs, mv) } +func (r *TTLRegistry) untrack(mv *MetricVec) { + r.mu.Lock() + defer r.mu.Unlock() + for i, v := range r.vecs { + if v == mv { + last := len(r.vecs) - 1 + r.vecs[i] = r.vecs[last] + r.vecs[last] = nil + r.vecs = r.vecs[:last] + return + } + } +} + +func metricVecFromCollector(c Collector) *MetricVec { + switch v := c.(type) { + case *CounterVec: + return v.MetricVec + case *GaugeVec: + return v.MetricVec + case *HistogramVec: + return v.MetricVec + case *MetricVec: + return v + default: + return nil + } +} + +// Unregister implements Registerer. It untracks Vecs created through this +// registry so they are no longer retained or cleaned up on Gather. +func (r *TTLRegistry) Unregister(c Collector) bool { + ok := r.Registry.Unregister(c) + if ok { + if mv := metricVecFromCollector(c); mv != nil { + r.untrack(mv) + } + } + return ok +} + func (r *TTLRegistry) runCleanup() { r.mu.Lock() vecs := append([]*MetricVec(nil), r.vecs...) diff --git a/prometheus/ttl_registry_test.go b/prometheus/ttl_registry_test.go index 7f1757e4d..7c43c933f 100644 --- a/prometheus/ttl_registry_test.go +++ b/prometheus/ttl_registry_test.go @@ -28,6 +28,46 @@ func TestNewTTLRegistryPanicsOnNonPositiveTTL(t *testing.T) { NewTTLRegistry(0) } +func TestTTLRegistryUnregisterUntracks(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 80 * time.Millisecond + reg := NewTTLRegistry(ttl) + vec := reg.NewCounterVec(CounterOpts{ + Name: "ttl_reg_unregister", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + if !reg.Unregister(vec) { + t.Fatal("expected Unregister to succeed") + } + + time.Sleep(ttl + 40*time.Millisecond) + + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + + // Gather should not have run CleanupExpired on the unregistered vec. + if cleaned := vec.CleanupExpired(); cleaned != 1 { + t.Fatalf("expected 1 expired child after explicit cleanup, got %d", cleaned) + } + + vec2 := reg.NewCounterVec(CounterOpts{ + Name: "ttl_reg_still_tracked", + Help: "test", + }, []string{"code"}) + vec2.WithLabelValues("200").Add(1) + time.Sleep(ttl + 40*time.Millisecond) + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + if cleaned := vec2.CleanupExpired(); cleaned != 0 { + t.Fatalf("expected tracked vec cleaned on Gather, got %d remaining", cleaned) + } + }) +} + func TestTTLRegistryGatherRunsCleanup(t *testing.T) { synctest.Test(t, func(t *testing.T) { ttl := 80 * time.Millisecond diff --git a/prometheus/ttl_vec.go b/prometheus/ttl_vec.go index 3fe0f84af..b33a6fddc 100644 --- a/prometheus/ttl_vec.go +++ b/prometheus/ttl_vec.go @@ -127,6 +127,7 @@ func (m *ttlMetricMap) cleanupExpired() int { var numDeleted int for h, metrics := range m.metrics { + origLen := len(metrics) remaining := metrics[:0] for i := range metrics { if metrics[i].lastAccessed.Load() >= deadline { @@ -138,6 +139,9 @@ func (m *ttlMetricMap) cleanupExpired() int { if len(remaining) == 0 { delete(m.metrics, h) } else { + for i := len(remaining); i < origLen; i++ { + metrics[i] = nil + } m.metrics[h] = remaining } } diff --git a/prometheus/vec.go b/prometheus/vec.go index 1f563b4b8..44ed8a49a 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -61,8 +61,15 @@ func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { // NewMetricVecWithTTL returns an initialized MetricVec with TTL-based expiration. // Children that have not been accessed (via GetMetricWith or GetMetricWithLabelValues) // for longer than ttl will be excluded from Collect and can be cleaned up via -// CleanupExpired. If ttl is 0, this behaves identically to NewMetricVec. +// CleanupExpired. If ttl is 0, this behaves identically to NewMetricVec. A +// negative ttl is invalid and will cause a panic. func NewMetricVecWithTTL(desc *Desc, newMetric func(lvs ...string) Metric, ttl time.Duration) *MetricVec { + if ttl < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", ttl)) + } + if ttl == 0 { + return NewMetricVec(desc, newMetric) + } return &MetricVec{ ttlMap: &ttlMetricMap{ metrics: map[uint64][]*ttlMetricWithLabelValues{}, diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index cd43da30a..ada97032e 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -1125,6 +1125,23 @@ func TestTTLRefreshPreventsExpiration(t *testing.T) { }) } +func TestNewMetricVecWithTTLZeroAndNegative(t *testing.T) { + desc := NewDesc("test", "help", []string{"l"}, nil) + newMetric := func(lvs ...string) Metric { return &counter{} } + + mv0 := NewMetricVecWithTTL(desc, newMetric, 0) + if mv0.ttlMap != nil { + t.Fatal("ttl==0 should use plain MetricVec without ttlMap") + } + + defer func() { + if recover() == nil { + t.Fatal("expected panic for negative ttl") + } + }() + NewMetricVecWithTTL(desc, newMetric, -time.Second) +} + func TestTTLZeroMeansNoExpiration(t *testing.T) { vec := NewCounterVec(CounterOpts{ Name: "test_no_ttl", From 49d10c367fa147187bc20a32ef2796f34422012b Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 11:13:27 +0200 Subject: [PATCH 4/9] Remove TTLRegistry and duplicated ttlMetricMap path Registries in client_golang are for Register/Gather, not metric construction. Drop TTLRegistry and the parallel ttl_vec code path so a follow-up can reintroduce TTL via Opts without coupling constructors to a registry. Addresses review feedback on TTLRegistry API and code duplication. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/counter.go | 30 +-- prometheus/gauge.go | 26 +-- prometheus/histogram.go | 36 ++-- prometheus/registry.go | 2 - prometheus/ttl_registry.go | 165 ---------------- prometheus/ttl_registry_test.go | 95 ---------- prometheus/ttl_vec.go | 327 -------------------------------- prometheus/vec.go | 107 ++--------- prometheus/vec_test.go | 186 ------------------ 9 files changed, 45 insertions(+), 929 deletions(-) delete mode 100644 prometheus/ttl_registry.go delete mode 100644 prometheus/ttl_registry_test.go delete mode 100644 prometheus/ttl_vec.go diff --git a/prometheus/counter.go b/prometheus/counter.go index 26b53400e..7d963d3af 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -201,12 +201,6 @@ func NewCounterVec(opts CounterOpts, labelNames []string) *CounterVec { // NewCounterVec creates a new CounterVec based on the provided CounterVecOpts. func (v2) NewCounterVec(opts CounterVecOpts) *CounterVec { - return newCounterVecWithTTL(opts, 0) -} - -// newCounterVecWithTTL creates a CounterVec. ttl must be >= 0; ttl == 0 disables -// TTL behavior (identical to NewMetricVec). -func newCounterVecWithTTL(opts CounterVecOpts, ttl time.Duration) *CounterVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -217,22 +211,16 @@ func newCounterVecWithTTL(opts CounterVecOpts, ttl time.Duration) *CounterVec { if opts.now == nil { opts.now = time.Now } - newMetric := func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} - result.init(result) // Init self-collection. - result.createdTs = timestamppb.New(opts.now()) - return result - } - if ttl > 0 { - return &CounterVec{ - MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), - } - } return &CounterVec{ - MetricVec: NewMetricVec(desc, newMetric), + MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} + result.init(result) // Init self-collection. + result.createdTs = timestamppb.New(opts.now()) + return result + }), } } diff --git a/prometheus/gauge.go b/prometheus/gauge.go index adebbf673..41e54bf27 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -159,10 +159,6 @@ func NewGaugeVec(opts GaugeOpts, labelNames []string) *GaugeVec { // NewGaugeVec creates a new GaugeVec based on the provided GaugeVecOpts. func (v2) NewGaugeVec(opts GaugeVecOpts) *GaugeVec { - return newGaugeVecWithTTL(opts, 0) -} - -func newGaugeVecWithTTL(opts GaugeVecOpts, ttl time.Duration) *GaugeVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -170,21 +166,15 @@ func newGaugeVecWithTTL(opts GaugeVecOpts, ttl time.Duration) *GaugeVec { opts.ConstLabels, WithUnit(opts.Unit), ) - newMetric := func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} - result.init(result) // Init self-collection. - return result - } - if ttl > 0 { - return &GaugeVec{ - MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), - } - } return &GaugeVec{ - MetricVec: NewMetricVec(desc, newMetric), + MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} + result.init(result) // Init self-collection. + return result + }), } } diff --git a/prometheus/histogram.go b/prometheus/histogram.go index d23602f3a..0e788f715 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -970,7 +970,7 @@ func (h *histogram) maybeReset( // We are using the possibly mocked h.now() rather than // time.Since(h.lastResetTime) to enable testing. if h.nativeHistogramMinResetDuration == 0 || // No reset configured. - h.resetScheduled || // Do not interfere if a reset is already scheduled. + h.resetScheduled || // Do not interefere if a reset is already scheduled. h.now().Sub(h.lastResetTime) < h.nativeHistogramMinResetDuration { return false } @@ -1057,8 +1057,8 @@ func (h *histogram) maybeWidenZeroBucket(hot, cold *histogramCounts) bool { atomic.StoreUint64(&cold.nativeHistogramZeroThresholdBits, math.Float64bits(newZeroThreshold)) // ...and then merge the newly deleted buckets into the wider zero // bucket. - mergeAndDeleteOrAddAndReset := func(hotBuckets, coldBuckets *sync.Map) func(k, v any) bool { - return func(k, v any) bool { + mergeAndDeleteOrAddAndReset := func(hotBuckets, coldBuckets *sync.Map) func(k, v interface{}) bool { + return func(k, v interface{}) bool { key := k.(int) bucket := v.(*int64) if key == smallestKey { @@ -1111,8 +1111,8 @@ func (h *histogram) doubleBucketWidth(hot, cold *histogramCounts) { // ...adjust the schema in the cold counts, too... atomic.StoreInt32(&cold.nativeHistogramSchema, coldSchema) // ...and then merge the cold buckets into the wider hot buckets. - merge := func(hotBuckets *sync.Map) func(k, v any) bool { - return func(k, v any) bool { + merge := func(hotBuckets *sync.Map) func(k, v interface{}) bool { + return func(k, v interface{}) bool { key := k.(int) bucket := v.(*int64) // Adjust key to match the bucket to merge into. @@ -1189,10 +1189,6 @@ func NewHistogramVec(opts HistogramOpts, labelNames []string) *HistogramVec { // NewHistogramVec creates a new HistogramVec based on the provided HistogramVecOpts. func (v2) NewHistogramVec(opts HistogramVecOpts) *HistogramVec { - return newHistogramVecWithTTL(opts, 0) -} - -func newHistogramVecWithTTL(opts HistogramVecOpts, ttl time.Duration) *HistogramVec { desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -1200,16 +1196,10 @@ func newHistogramVecWithTTL(opts HistogramVecOpts, ttl time.Duration) *Histogram opts.ConstLabels, WithUnit(opts.Unit), ) - newMetric := func(lvs ...string) Metric { - return newHistogram(desc, opts.HistogramOpts, lvs...) - } - if ttl > 0 { - return &HistogramVec{ - MetricVec: NewMetricVecWithTTL(desc, newMetric, ttl), - } - } return &HistogramVec{ - MetricVec: NewMetricVec(desc, newMetric), + MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { + return newHistogram(desc, opts.HistogramOpts, lvs...) + }), } } @@ -1491,7 +1481,7 @@ func pickSchema(bucketFactor float64) int32 { func makeBuckets(buckets *sync.Map) ([]*dto.BucketSpan, []int64) { var ii []int - buckets.Range(func(k, v any) bool { + buckets.Range(func(k, v interface{}) bool { ii = append(ii, k.(int)) return true }) @@ -1568,8 +1558,8 @@ func addToBucket(buckets *sync.Map, key int, increment int64) bool { // according to the buckets ranged through. It then resets all buckets ranged // through to 0 (but leaves them in place so that they don't need to get // recreated on the next scrape). -func addAndReset(hotBuckets *sync.Map, bucketNumber *uint32) func(k, v any) bool { - return func(k, v any) bool { +func addAndReset(hotBuckets *sync.Map, bucketNumber *uint32) func(k, v interface{}) bool { + return func(k, v interface{}) bool { bucket := v.(*int64) if addToBucket(hotBuckets, k.(int), atomic.LoadInt64(bucket)) { atomic.AddUint32(bucketNumber, 1) @@ -1580,7 +1570,7 @@ func addAndReset(hotBuckets *sync.Map, bucketNumber *uint32) func(k, v any) bool } func deleteSyncMap(m *sync.Map) { - m.Range(func(k, v any) bool { + m.Range(func(k, v interface{}) bool { m.Delete(k) return true }) @@ -1588,7 +1578,7 @@ func deleteSyncMap(m *sync.Map) { func findSmallestKey(m *sync.Map) int { result := math.MaxInt32 - m.Range(func(k, v any) bool { + m.Range(func(k, v interface{}) bool { key := k.(int) if key < result { result = key diff --git a/prometheus/registry.go b/prometheus/registry.go index ed0681c8b..8dd906c6b 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -641,12 +641,10 @@ func WriteToTextfile(filename string, g Gatherer) error { mfs, err := g.Gather() if err != nil { - tmp.Close() return err } for _, mf := range mfs { if _, err := expfmt.MetricFamilyToText(tmp, mf); err != nil { - tmp.Close() return err } } diff --git a/prometheus/ttl_registry.go b/prometheus/ttl_registry.go deleted file mode 100644 index fa30f7c0a..000000000 --- a/prometheus/ttl_registry.go +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright 2014 The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prometheus - -import ( - "sync" - "time" - - dto "github.com/prometheus/client_model/go" -) - -// TTLRegistry is a dedicated Prometheus registry for metrics that need -// time-to-live behavior on *Vec children. It embeds a plain *Registry and adds: -// - Vec constructors that enable per-child TTL (same semantics as MetricVec -// built with NewMetricVecWithTTL). -// - Automatic CleanupExpired on all Vecs created through this registry before -// each Gather, so memory can be reclaimed even when scrapes are infrequent -// (see discussion in https://github.com/prometheus/client_golang/issues/1983). -// -// Default prometheus.NewRegistry, NewCounterVec, and Opts are unchanged; use -// TTLRegistry only when you explicitly opt in to Vec TTL. -type TTLRegistry struct { - *Registry - - ttl time.Duration - - mu sync.Mutex - vecs []*MetricVec -} - -// NewTTLRegistry returns a registry backed by a new empty *Registry. ttl must -// be greater than zero; it applies to every Vec created via this registry's -// constructor methods. -func NewTTLRegistry(ttl time.Duration) *TTLRegistry { - if ttl <= 0 { - panic("NewTTLRegistry: ttl must be > 0") - } - return &TTLRegistry{ - Registry: NewRegistry(), - ttl: ttl, - } -} - -func (r *TTLRegistry) track(mv *MetricVec) { - r.mu.Lock() - defer r.mu.Unlock() - r.vecs = append(r.vecs, mv) -} - -func (r *TTLRegistry) untrack(mv *MetricVec) { - r.mu.Lock() - defer r.mu.Unlock() - for i, v := range r.vecs { - if v == mv { - last := len(r.vecs) - 1 - r.vecs[i] = r.vecs[last] - r.vecs[last] = nil - r.vecs = r.vecs[:last] - return - } - } -} - -func metricVecFromCollector(c Collector) *MetricVec { - switch v := c.(type) { - case *CounterVec: - return v.MetricVec - case *GaugeVec: - return v.MetricVec - case *HistogramVec: - return v.MetricVec - case *MetricVec: - return v - default: - return nil - } -} - -// Unregister implements Registerer. It untracks Vecs created through this -// registry so they are no longer retained or cleaned up on Gather. -func (r *TTLRegistry) Unregister(c Collector) bool { - ok := r.Registry.Unregister(c) - if ok { - if mv := metricVecFromCollector(c); mv != nil { - r.untrack(mv) - } - } - return ok -} - -func (r *TTLRegistry) runCleanup() { - r.mu.Lock() - vecs := append([]*MetricVec(nil), r.vecs...) - r.mu.Unlock() - for _, mv := range vecs { - mv.CleanupExpired() - } -} - -// Gather implements Gatherer. It runs CleanupExpired on all Vecs created -// through this TTLRegistry, then delegates to the embedded Registry. -func (r *TTLRegistry) Gather() ([]*dto.MetricFamily, error) { - r.runCleanup() - return r.Registry.Gather() -} - -// NewCounterVec is like prometheus.NewCounterVec but enables Vec TTL using this -// registry's ttl, registers the Vec, and tracks it for Gather-time cleanup. -func (r *TTLRegistry) NewCounterVec(opts CounterOpts, labelNames []string) *CounterVec { - return r.NewCounterVecOpts(CounterVecOpts{ - CounterOpts: opts, - VariableLabels: UnconstrainedLabels(labelNames), - }) -} - -// NewCounterVecOpts is like V2.NewCounterVec with TTL and automatic registration. -func (r *TTLRegistry) NewCounterVecOpts(opts CounterVecOpts) *CounterVec { - cv := newCounterVecWithTTL(opts, r.ttl) - r.MustRegister(cv) - r.track(cv.MetricVec) - return cv -} - -// NewGaugeVec is like prometheus.NewGaugeVec with TTL, registration, and tracking. -func (r *TTLRegistry) NewGaugeVec(opts GaugeOpts, labelNames []string) *GaugeVec { - return r.NewGaugeVecOpts(GaugeVecOpts{ - GaugeOpts: opts, - VariableLabels: UnconstrainedLabels(labelNames), - }) -} - -// NewGaugeVecOpts is like V2.NewGaugeVec with TTL and automatic registration. -func (r *TTLRegistry) NewGaugeVecOpts(opts GaugeVecOpts) *GaugeVec { - gv := newGaugeVecWithTTL(opts, r.ttl) - r.MustRegister(gv) - r.track(gv.MetricVec) - return gv -} - -// NewHistogramVec is like prometheus.NewHistogramVec with TTL, registration, and tracking. -func (r *TTLRegistry) NewHistogramVec(opts HistogramOpts, labelNames []string) *HistogramVec { - return r.NewHistogramVecOpts(HistogramVecOpts{ - HistogramOpts: opts, - VariableLabels: UnconstrainedLabels(labelNames), - }) -} - -// NewHistogramVecOpts is like V2.NewHistogramVec with TTL and automatic registration. -func (r *TTLRegistry) NewHistogramVecOpts(opts HistogramVecOpts) *HistogramVec { - hv := newHistogramVecWithTTL(opts, r.ttl) - r.MustRegister(hv) - r.track(hv.MetricVec) - return hv -} diff --git a/prometheus/ttl_registry_test.go b/prometheus/ttl_registry_test.go deleted file mode 100644 index 7c43c933f..000000000 --- a/prometheus/ttl_registry_test.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2014 The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prometheus - -import ( - "testing" - "testing/synctest" - "time" -) - -func TestNewTTLRegistryPanicsOnNonPositiveTTL(t *testing.T) { - defer func() { - if recover() == nil { - t.Fatal("expected panic for ttl <= 0") - } - }() - NewTTLRegistry(0) -} - -func TestTTLRegistryUnregisterUntracks(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 80 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "ttl_reg_unregister", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - if !reg.Unregister(vec) { - t.Fatal("expected Unregister to succeed") - } - - time.Sleep(ttl + 40*time.Millisecond) - - if _, err := reg.Gather(); err != nil { - t.Fatal(err) - } - - // Gather should not have run CleanupExpired on the unregistered vec. - if cleaned := vec.CleanupExpired(); cleaned != 1 { - t.Fatalf("expected 1 expired child after explicit cleanup, got %d", cleaned) - } - - vec2 := reg.NewCounterVec(CounterOpts{ - Name: "ttl_reg_still_tracked", - Help: "test", - }, []string{"code"}) - vec2.WithLabelValues("200").Add(1) - time.Sleep(ttl + 40*time.Millisecond) - if _, err := reg.Gather(); err != nil { - t.Fatal(err) - } - if cleaned := vec2.CleanupExpired(); cleaned != 0 { - t.Fatalf("expected tracked vec cleaned on Gather, got %d remaining", cleaned) - } - }) -} - -func TestTTLRegistryGatherRunsCleanup(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 80 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "ttl_reg_gather", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric before sleep, got %d", n) - } - - time.Sleep(ttl + 40*time.Millisecond) - - if _, err := reg.Gather(); err != nil { - t.Fatal(err) - } - - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after Gather cleanup, got %d", n) - } - }) -} diff --git a/prometheus/ttl_vec.go b/prometheus/ttl_vec.go deleted file mode 100644 index b33a6fddc..000000000 --- a/prometheus/ttl_vec.go +++ /dev/null @@ -1,327 +0,0 @@ -// Copyright 2014 The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package prometheus - -import ( - "sync" - "sync/atomic" - "time" -) - -// ttlMetricWithLabelValues is the TTL variant of metricWithLabelValues. -type ttlMetricWithLabelValues struct { - values []string - metric Metric - lastAccessed atomic.Int64 // unix timestamp in milliseconds -} - -// ttlMetricMap backs MetricVec instances created with NewMetricVecWithTTL. -// It is separate from metricMap so the default Vec path stays unchanged. -type ttlMetricMap struct { - mtx sync.RWMutex - metrics map[uint64][]*ttlMetricWithLabelValues - desc *Desc - newMetric func(labelValues ...string) Metric - ttl time.Duration -} - -func (m *ttlMetricMap) Describe(ch chan<- *Desc) { - ch <- m.desc -} - -func (m *ttlMetricMap) Collect(ch chan<- Metric) { - m.mtx.RLock() - defer m.mtx.RUnlock() - - deadline := time.Now().Add(-m.ttl).UnixMilli() - for _, metrics := range m.metrics { - for i := range metrics { - if metrics[i].lastAccessed.Load() < deadline { - continue - } - ch <- metrics[i].metric - } - } -} - -func (m *ttlMetricMap) Reset() { - m.mtx.Lock() - defer m.mtx.Unlock() - - for h := range m.metrics { - delete(m.metrics, h) - } -} - -func (m *ttlMetricMap) touchByHash(h uint64, lvs []string, curry []curriedLabelValue) { - m.mtx.RLock() - defer m.mtx.RUnlock() - m.touchByHashRLocked(h, lvs, curry) -} - -func (m *ttlMetricMap) touchByHashRLocked(h uint64, lvs []string, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *ttlMetricMap) touchByHashLocked(h uint64, lvs []string, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *ttlMetricMap) touchByHashLabels(h uint64, labels Labels, curry []curriedLabelValue) { - m.mtx.RLock() - defer m.mtx.RUnlock() - m.touchByHashLabelsRLocked(h, labels, curry) -} - -func (m *ttlMetricMap) touchByHashLabelsRLocked(h uint64, labels Labels, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *ttlMetricMap) touchByHashLabelsLocked(h uint64, labels Labels, curry []curriedLabelValue) { - now := time.Now().UnixMilli() - metrics, ok := m.metrics[h] - if !ok { - return - } - if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { - metrics[i].lastAccessed.Store(now) - } -} - -func (m *ttlMetricMap) cleanupExpired() int { - deadline := time.Now().Add(-m.ttl).UnixMilli() - m.mtx.Lock() - defer m.mtx.Unlock() - - var numDeleted int - for h, metrics := range m.metrics { - origLen := len(metrics) - remaining := metrics[:0] - for i := range metrics { - if metrics[i].lastAccessed.Load() >= deadline { - remaining = append(remaining, metrics[i]) - } else { - numDeleted++ - } - } - if len(remaining) == 0 { - delete(m.metrics, h) - } else { - for i := len(remaining); i < origLen; i++ { - metrics[i] = nil - } - m.metrics[h] = remaining - } - } - return numDeleted -} - -func (m *ttlMetricMap) deleteByHashWithLabelValues( - h uint64, lvs []string, curry []curriedLabelValue, -) bool { - m.mtx.Lock() - defer m.mtx.Unlock() - - metrics, ok := m.metrics[h] - if !ok { - return false - } - - i := findTTLMetricWithLabelValues(metrics, lvs, curry) - if i >= len(metrics) { - return false - } - - if len(metrics) > 1 { - old := metrics - m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = nil - } else { - delete(m.metrics, h) - } - return true -} - -func (m *ttlMetricMap) deleteByHashWithLabels( - h uint64, labels Labels, curry []curriedLabelValue, -) bool { - m.mtx.Lock() - defer m.mtx.Unlock() - - metrics, ok := m.metrics[h] - if !ok { - return false - } - i := findTTLMetricWithLabels(m.desc, metrics, labels, curry) - if i >= len(metrics) { - return false - } - - if len(metrics) > 1 { - old := metrics - m.metrics[h] = append(metrics[:i], metrics[i+1:]...) - old[len(old)-1] = nil - } else { - delete(m.metrics, h) - } - return true -} - -func (m *ttlMetricMap) deleteByLabels(labels Labels, curry []curriedLabelValue) int { - m.mtx.Lock() - defer m.mtx.Unlock() - - var numDeleted int - - for h, metrics := range m.metrics { - i := findTTLMetricWithPartialLabels(m.desc, metrics, labels, curry) - if i >= len(metrics) { - continue - } - delete(m.metrics, h) - numDeleted++ - } - - return numDeleted -} - -func findTTLMetricWithPartialLabels( - desc *Desc, metrics []*ttlMetricWithLabelValues, labels Labels, curry []curriedLabelValue, -) int { - for i := range metrics { - if matchPartialLabels(desc, metrics[i].values, labels, curry) { - return i - } - } - return len(metrics) -} - -func (m *ttlMetricMap) getOrCreateMetricWithLabelValues( - hash uint64, lvs []string, curry []curriedLabelValue, -) Metric { - m.mtx.RLock() - metric, ok := m.getMetricWithHashAndLabelValues(hash, lvs, curry) - m.mtx.RUnlock() - if ok { - m.touchByHash(hash, lvs, curry) - return metric - } - - m.mtx.Lock() - defer m.mtx.Unlock() - metric, ok = m.getMetricWithHashAndLabelValues(hash, lvs, curry) - if !ok { - inlinedLVs := inlineLabelValues(lvs, curry) - metric = m.newMetric(inlinedLVs...) - entry := &ttlMetricWithLabelValues{values: inlinedLVs, metric: metric} - entry.lastAccessed.Store(time.Now().UnixMilli()) - m.metrics[hash] = append(m.metrics[hash], entry) - } else { - m.touchByHashLocked(hash, lvs, curry) - } - return metric -} - -func (m *ttlMetricMap) getOrCreateMetricWithLabels( - hash uint64, labels Labels, curry []curriedLabelValue, -) Metric { - m.mtx.RLock() - metric, ok := m.getMetricWithHashAndLabels(hash, labels, curry) - m.mtx.RUnlock() - if ok { - m.touchByHashLabels(hash, labels, curry) - return metric - } - - m.mtx.Lock() - defer m.mtx.Unlock() - metric, ok = m.getMetricWithHashAndLabels(hash, labels, curry) - if !ok { - lvs := extractLabelValues(m.desc, labels, curry) - metric = m.newMetric(lvs...) - entry := &ttlMetricWithLabelValues{values: lvs, metric: metric} - entry.lastAccessed.Store(time.Now().UnixMilli()) - m.metrics[hash] = append(m.metrics[hash], entry) - } else { - m.touchByHashLabelsLocked(hash, labels, curry) - } - return metric -} - -func (m *ttlMetricMap) getMetricWithHashAndLabelValues( - h uint64, lvs []string, curry []curriedLabelValue, -) (Metric, bool) { - metrics, ok := m.metrics[h] - if ok { - if i := findTTLMetricWithLabelValues(metrics, lvs, curry); i < len(metrics) { - return metrics[i].metric, true - } - } - return nil, false -} - -func (m *ttlMetricMap) getMetricWithHashAndLabels( - h uint64, labels Labels, curry []curriedLabelValue, -) (Metric, bool) { - metrics, ok := m.metrics[h] - if ok { - if i := findTTLMetricWithLabels(m.desc, metrics, labels, curry); i < len(metrics) { - return metrics[i].metric, true - } - } - return nil, false -} - -func findTTLMetricWithLabelValues( - metrics []*ttlMetricWithLabelValues, lvs []string, curry []curriedLabelValue, -) int { - for i := range metrics { - if matchLabelValues(metrics[i].values, lvs, curry) { - return i - } - } - return len(metrics) -} - -func findTTLMetricWithLabels( - desc *Desc, metrics []*ttlMetricWithLabelValues, labels Labels, curry []curriedLabelValue, -) int { - for i := range metrics { - if matchLabels(desc, metrics[i].values, labels, curry) { - return i - } - } - return len(metrics) -} diff --git a/prometheus/vec.go b/prometheus/vec.go index b94bf3e82..b405a64de 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -16,7 +16,6 @@ package prometheus import ( "fmt" "sync" - "time" "github.com/prometheus/common/model" ) @@ -36,7 +35,6 @@ import ( // panic instead of returning errors. See also the MetricVec example. type MetricVec struct { *metricMap - ttlMap *ttlMetricMap curry []curriedLabelValue @@ -58,47 +56,6 @@ func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { } } -// NewMetricVecWithTTL returns an initialized MetricVec with TTL-based expiration. -// Children that have not been accessed (via GetMetricWith or GetMetricWithLabelValues) -// for longer than ttl will be excluded from Collect and can be cleaned up via -// CleanupExpired. If ttl is 0, this behaves identically to NewMetricVec. A -// negative ttl is invalid and will cause a panic. -func NewMetricVecWithTTL(desc *Desc, newMetric func(lvs ...string) Metric, ttl time.Duration) *MetricVec { - if ttl < 0 { - panic(fmt.Sprintf("invalid negative ttl: %v", ttl)) - } - if ttl == 0 { - return NewMetricVec(desc, newMetric) - } - return &MetricVec{ - ttlMap: &ttlMetricMap{ - metrics: map[uint64][]*ttlMetricWithLabelValues{}, - desc: desc, - newMetric: newMetric, - ttl: ttl, - }, - hashAdd: hashAdd, - hashAddByte: hashAddByte, - } -} - -// CleanupExpired removes all children that have not been accessed within the -// configured TTL. It returns the number of children removed. If TTL is not -// configured (zero), this is a no-op and returns 0. -func (m *MetricVec) CleanupExpired() int { - if m.ttlMap == nil { - return 0 - } - return m.ttlMap.cleanupExpired() -} - -func (m *MetricVec) vecDesc() *Desc { - if m.ttlMap != nil { - return m.ttlMap.desc - } - return m.desc -} - // DeleteLabelValues removes the metric where the variable labels are the same // as those passed in as labels (same order as the VariableLabels in Desc). It // returns true if a metric was deleted. @@ -115,16 +72,13 @@ func (m *MetricVec) vecDesc() *Desc { // with a performance overhead (for creating and processing the Labels map). // See also the CounterVec example. func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { - lvs = constrainLabelValues(m.vecDesc(), lvs, m.curry) + lvs = constrainLabelValues(m.desc, lvs, m.curry) h, err := m.hashLabelValues(lvs) if err != nil { return false } - if m.ttlMap != nil { - return m.ttlMap.deleteByHashWithLabelValues(h, lvs, m.curry) - } return m.deleteByHashWithLabelValues(h, lvs, m.curry) } @@ -139,7 +93,7 @@ func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { // This method is used for the same purpose as DeleteLabelValues(...string). See // there for pros and cons of the two methods. func (m *MetricVec) Delete(labels Labels) bool { - labels, closer := constrainLabels(m.vecDesc(), labels) + labels, closer := constrainLabels(m.desc, labels) defer closer() h, err := m.hashLabels(labels) @@ -147,9 +101,6 @@ func (m *MetricVec) Delete(labels Labels) bool { return false } - if m.ttlMap != nil { - return m.ttlMap.deleteByHashWithLabels(h, labels, m.curry) - } return m.deleteByHashWithLabels(h, labels, m.curry) } @@ -160,12 +111,9 @@ func (m *MetricVec) Delete(labels Labels) bool { // Note that curried labels will never be matched if deleting from the curried vector. // To match curried labels with DeletePartialMatch, it must be called on the base vector. func (m *MetricVec) DeletePartialMatch(labels Labels) int { - labels, closer := constrainLabels(m.vecDesc(), labels) + labels, closer := constrainLabels(m.desc, labels) defer closer() - if m.ttlMap != nil { - return m.ttlMap.deleteByLabels(labels, m.curry) - } return m.deleteByLabels(labels, m.curry) } @@ -173,31 +121,13 @@ func (m *MetricVec) DeletePartialMatch(labels Labels) int { // show up in GoDoc. // Describe implements Collector. -func (m *MetricVec) Describe(ch chan<- *Desc) { - if m.ttlMap != nil { - m.ttlMap.Describe(ch) - return - } - m.metricMap.Describe(ch) -} +func (m *MetricVec) Describe(ch chan<- *Desc) { m.metricMap.Describe(ch) } // Collect implements Collector. -func (m *MetricVec) Collect(ch chan<- Metric) { - if m.ttlMap != nil { - m.ttlMap.Collect(ch) - return - } - m.metricMap.Collect(ch) -} +func (m *MetricVec) Collect(ch chan<- Metric) { m.metricMap.Collect(ch) } // Reset deletes all metrics in this vector. -func (m *MetricVec) Reset() { - if m.ttlMap != nil { - m.ttlMap.Reset() - return - } - m.metricMap.Reset() -} +func (m *MetricVec) Reset() { m.metricMap.Reset() } // CurryWith returns a vector curried with the provided labels, i.e. the // returned vector has those labels pre-set for all labeled operations performed @@ -222,7 +152,7 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { oldCurry = m.curry iCurry int ) - for i, labelName := range m.vecDesc().variableLabels.names { + for i, labelName := range m.desc.variableLabels.names { val, ok := labels[labelName] if iCurry < len(oldCurry) && oldCurry[iCurry].index == i { if ok { @@ -236,7 +166,7 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { } newCurry = append(newCurry, curriedLabelValue{ i, - m.vecDesc().variableLabels.constrain(labelName, val), + m.desc.variableLabels.constrain(labelName, val), }) } } @@ -246,7 +176,6 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { return &MetricVec{ metricMap: m.metricMap, - ttlMap: m.ttlMap, curry: newCurry, hashAdd: m.hashAdd, hashAddByte: m.hashAddByte, @@ -283,15 +212,12 @@ func (m *MetricVec) CurryWith(labels Labels) (*MetricVec, error) { // a wrapper around MetricVec, implementing a vector for a specific Metric // implementation, for example GaugeVec. func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) { - lvs = constrainLabelValues(m.vecDesc(), lvs, m.curry) + lvs = constrainLabelValues(m.desc, lvs, m.curry) h, err := m.hashLabelValues(lvs) if err != nil { return nil, err } - if m.ttlMap != nil { - return m.ttlMap.getOrCreateMetricWithLabelValues(h, lvs, m.curry), nil - } return m.getOrCreateMetricWithLabelValues(h, lvs, m.curry), nil } @@ -312,7 +238,7 @@ func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) { // around MetricVec, implementing a vector for a specific Metric implementation, // for example GaugeVec. func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) { - labels, closer := constrainLabels(m.vecDesc(), labels) + labels, closer := constrainLabels(m.desc, labels) defer closer() h, err := m.hashLabels(labels) @@ -320,14 +246,11 @@ func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) { return nil, err } - if m.ttlMap != nil { - return m.ttlMap.getOrCreateMetricWithLabels(h, labels, m.curry), nil - } return m.getOrCreateMetricWithLabels(h, labels, m.curry), nil } func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { - if err := validateLabelValues(vals, len(m.vecDesc().variableLabels.names)-len(m.curry)); err != nil { + if err := validateLabelValues(vals, len(m.desc.variableLabels.names)-len(m.curry)); err != nil { return 0, err } @@ -336,7 +259,7 @@ func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { curry = m.curry iVals, iCurry int ) - for i := 0; i < len(m.vecDesc().variableLabels.names); i++ { + for i := 0; i < len(m.desc.variableLabels.names); i++ { if iCurry < len(curry) && curry[iCurry].index == i { h = m.hashAdd(h, curry[iCurry].value) iCurry++ @@ -350,7 +273,7 @@ func (m *MetricVec) hashLabelValues(vals []string) (uint64, error) { } func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { - if err := validateValuesInLabels(labels, len(m.vecDesc().variableLabels.names)-len(m.curry)); err != nil { + if err := validateValuesInLabels(labels, len(m.desc.variableLabels.names)-len(m.curry)); err != nil { return 0, err } @@ -359,7 +282,7 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) { curry = m.curry iCurry int ) - for i, labelName := range m.vecDesc().variableLabels.names { + for i, labelName := range m.desc.variableLabels.names { val, ok := labels[labelName] if iCurry < len(curry) && curry[iCurry].index == i { if ok { @@ -736,7 +659,7 @@ func inlineLabelValues(lvs []string, curry []curriedLabelValue) []string { } var labelsPool = &sync.Pool{ - New: func() any { + New: func() interface{} { return make(Labels) }, } diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index ada97032e..03223f2f6 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -18,8 +18,6 @@ import ( "reflect" "strconv" "testing" - "testing/synctest" - "time" dto "github.com/prometheus/client_model/go" ) @@ -1006,187 +1004,3 @@ func benchmarkMetricVecWithLabelValues(b *testing.B, labels map[string][]string) vec.WithLabelValues(values...) } } - -func collectCount(c Collector) int { - ch := make(chan Metric, 100) - c.Collect(ch) - close(ch) - n := 0 - for range ch { - n++ - } - return n -} - -func TestTTLCounterVec(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "test_ttl_counter", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - vec.WithLabelValues("404").Add(1) - - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } - - time.Sleep(ttl + 50*time.Millisecond) - - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after TTL, got %d", n) - } - - cleaned := vec.CleanupExpired() - if cleaned != 2 { - t.Fatalf("expected 2 cleaned, got %d", cleaned) - } - }) -} - -func TestTTLGaugeVec(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewGaugeVec(GaugeOpts{ - Name: "test_ttl_gauge", - Help: "test", - }, []string{"method"}) - - vec.WithLabelValues("GET").Set(10) - vec.WithLabelValues("POST").Set(20) - - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } - - time.Sleep(ttl + 50*time.Millisecond) - // Touch only one - vec.WithLabelValues("GET").Set(30) - - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric after partial TTL, got %d", n) - } - - cleaned := vec.CleanupExpired() - if cleaned != 1 { - t.Fatalf("expected 1 cleaned, got %d", cleaned) - } - }) -} - -func TestTTLHistogramVec(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewHistogramVec(HistogramOpts{ - Name: "test_ttl_histo", - Help: "test", - }, []string{"status"}) - - vec.WithLabelValues("ok").Observe(0.5) - vec.WithLabelValues("err").Observe(1.5) - - if n := collectCount(vec); n != 2 { - t.Fatalf("expected 2 metrics, got %d", n) - } - - time.Sleep(ttl + 50*time.Millisecond) - - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 metrics after TTL, got %d", n) - } - }) -} - -func TestTTLRefreshPreventsExpiration(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 150 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewCounterVec(CounterOpts{ - Name: "test_ttl_refresh", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - - // Keep refreshing before TTL expires - for i := 0; i < 5; i++ { - time.Sleep(80 * time.Millisecond) - vec.WithLabelValues("200").Add(1) - } - - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric still alive, got %d", n) - } - }) -} - -func TestNewMetricVecWithTTLZeroAndNegative(t *testing.T) { - desc := NewDesc("test", "help", []string{"l"}, nil) - newMetric := func(lvs ...string) Metric { return &counter{} } - - mv0 := NewMetricVecWithTTL(desc, newMetric, 0) - if mv0.ttlMap != nil { - t.Fatal("ttl==0 should use plain MetricVec without ttlMap") - } - - defer func() { - if recover() == nil { - t.Fatal("expected panic for negative ttl") - } - }() - NewMetricVecWithTTL(desc, newMetric, -time.Second) -} - -func TestTTLZeroMeansNoExpiration(t *testing.T) { - vec := NewCounterVec(CounterOpts{ - Name: "test_no_ttl", - Help: "test", - }, []string{"code"}) - - vec.WithLabelValues("200").Add(1) - - cleaned := vec.CleanupExpired() - if cleaned != 0 { - t.Fatalf("expected 0 cleaned with no TTL, got %d", cleaned) - } - - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 metric, got %d", n) - } -} - -func TestTTLWithGetMetricWith(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - ttl := 100 * time.Millisecond - reg := NewTTLRegistry(ttl) - vec := reg.NewGaugeVec(GaugeOpts{ - Name: "test_ttl_getmetricwith", - Help: "test", - }, []string{"method"}) - - g, err := vec.GetMetricWith(Labels{"method": "GET"}) - if err != nil { - t.Fatal(err) - } - g.Set(42) - - time.Sleep(ttl + 50*time.Millisecond) - - if n := collectCount(vec); n != 0 { - t.Fatalf("expected 0 after TTL, got %d", n) - } - - // Re-access refreshes - g, _ = vec.GetMetricWith(Labels{"method": "GET"}) - g.Set(99) - - if n := collectCount(vec); n != 1 { - t.Fatalf("expected 1 after re-access, got %d", n) - } - }) -} From f366f3e2ab836d1530c79510ef2521575dbde9b3 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 11:13:42 +0200 Subject: [PATCH 5/9] Configure Vec TTL via MetricVecOpts on a unified metricMap Replace NewMetricVecWithTTL / dual ttlMap branching with MetricVecOpts.TTL and a single metricMap path. Collect skips expired ttlMetric children; CleanupExpired reclaims them. Default TTL of 0 keeps the hot path free of expiration work. Addresses review feedback on new*WithTTL helpers and the ttlMap anti-pattern. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/ttl.go | 30 ++++++++++++ prometheus/vec.go | 113 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 prometheus/ttl.go diff --git a/prometheus/ttl.go b/prometheus/ttl.go new file mode 100644 index 000000000..603bdc095 --- /dev/null +++ b/prometheus/ttl.go @@ -0,0 +1,30 @@ +// Copyright 2026 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +// ExpiredCleaner is implemented by collectors that support TTL-based cleanup of +// unused children (for example MetricVec with a non-zero Opts.TTL). +// Registry.Gather calls CleanupExpired on registered collectors that implement +// this interface so expired children can be reclaimed even when Collect alone +// would only skip them. +type ExpiredCleaner interface { + CleanupExpired() int +} + +// ttlMetric is implemented by decorator wrappers that track last access time. +type ttlMetric interface { + Metric + lastAccessed() int64 + touch() +} diff --git a/prometheus/vec.go b/prometheus/vec.go index b405a64de..312d23267 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -16,6 +16,7 @@ package prometheus import ( "fmt" "sync" + "time" "github.com/prometheus/common/model" ) @@ -43,19 +44,64 @@ type MetricVec struct { hashAddByte func(h uint64, b byte) uint64 } -// NewMetricVec returns an initialized metricVec. +// MetricVecOpts bundles the options to create a MetricVec. +type MetricVecOpts struct { + Desc *Desc + NewMetric func(lvs ...string) Metric + // TTL, if greater than zero, enables per-child expiration. Children that + // have not been accessed for longer than TTL are omitted from Collect and + // can be removed via CleanupExpired (also invoked automatically by + // Registry.Gather for collectors that implement ExpiredCleaner). + // + // A negative TTL is invalid and causes a panic. TTL of zero disables + // expiration (identical to NewMetricVec). + // + // Access includes GetMetricWith / GetMetricWithLabelValues and, when using + // the built-in CounterVec / GaugeVec / HistogramVec with TTL, mutating + // methods on cached children (Inc, Add, Set, Observe, …). Caching a child + // and never calling those methods (nor looking it up again) lets the child + // expire; the cached handle then behaves like after Delete — updates are + // not exported until the label set is looked up again. See Delete docs. + // + // If metrics are never scraped, call CleanupExpired periodically (or rely + // on Gather) so expired children can be reclaimed; there is no background + // goroutine. + TTL time.Duration +} + +// NewMetricVec returns an initialized MetricVec with no TTL. func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { + return V2.NewMetricVec(MetricVecOpts{Desc: desc, NewMetric: newMetric}) +} + +// NewMetricVec returns an initialized MetricVec. See MetricVecOpts. +func (v2) NewMetricVec(opts MetricVecOpts) *MetricVec { + if opts.TTL < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", opts.TTL)) + } return &MetricVec{ metricMap: &metricMap{ metrics: map[uint64][]metricWithLabelValues{}, - desc: desc, - newMetric: newMetric, + desc: opts.Desc, + newMetric: opts.NewMetric, + ttl: opts.TTL, }, hashAdd: hashAdd, hashAddByte: hashAddByte, } } +// CleanupExpired removes all children that have not been accessed within the +// configured TTL. It returns the number of children removed. If TTL is not +// configured (zero), this is a no-op and returns 0. +// +// Registry.Gather invokes CleanupExpired for collectors that implement +// ExpiredCleaner. If scrapes are rare or absent, call CleanupExpired +// periodically yourself; client_golang does not start a background cleaner. +func (m *MetricVec) CleanupExpired() int { + return m.metricMap.cleanupExpired() +} + // DeleteLabelValues removes the metric where the variable labels are the same // as those passed in as labels (same order as the VariableLabels in Desc). It // returns true if a metric was deleted. @@ -71,6 +117,10 @@ func NewMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec { // latter has a much more readable (albeit more verbose) syntax, but it comes // with a performance overhead (for creating and processing the Labels map). // See also the CounterVec example. +// +// Callers that cache a child and keep using it after deletion (or after +// CleanupExpired under TTL) update a detached metric that is no longer +// exported until the same label set is looked up again. func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { lvs = constrainLabelValues(m.desc, lvs, m.curry) @@ -321,6 +371,7 @@ type metricMap struct { metrics map[uint64][]metricWithLabelValues desc *Desc newMetric func(labelValues ...string) Metric + ttl time.Duration // 0 disables TTL; see MetricVecOpts.TTL. } // Describe implements Collector. It will send exactly one Desc to the provided @@ -334,13 +385,59 @@ func (m *metricMap) Collect(ch chan<- Metric) { m.mtx.RLock() defer m.mtx.RUnlock() + var deadline int64 + if m.ttl > 0 { + deadline = time.Now().Add(-m.ttl).UnixMilli() + } for _, metrics := range m.metrics { for _, metric := range metrics { + if m.ttl > 0 { + if tm, ok := metric.metric.(ttlMetric); ok && tm.lastAccessed() < deadline { + continue + } + } ch <- metric.metric } } } +func (m *metricMap) cleanupExpired() int { + if m.ttl <= 0 { + return 0 + } + deadline := time.Now().Add(-m.ttl).UnixMilli() + m.mtx.Lock() + defer m.mtx.Unlock() + + var numDeleted int + for h, metrics := range m.metrics { + origLen := len(metrics) + remaining := metrics[:0] + for i := range metrics { + if tm, ok := metrics[i].metric.(ttlMetric); ok && tm.lastAccessed() < deadline { + numDeleted++ + continue + } + remaining = append(remaining, metrics[i]) + } + if len(remaining) == 0 { + delete(m.metrics, h) + } else { + for i := len(remaining); i < origLen; i++ { + metrics[i] = metricWithLabelValues{} + } + m.metrics[h] = remaining + } + } + return numDeleted +} + +func touchIfTTL(metric Metric) { + if tm, ok := metric.(ttlMetric); ok { + tm.touch() + } +} + // Reset deletes all metrics in this vector. func (m *metricMap) Reset() { m.mtx.Lock() @@ -495,6 +592,9 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( metric, ok := m.getMetricWithHashAndLabelValues(hash, lvs, curry) m.mtx.RUnlock() if ok { + if m.ttl > 0 { + touchIfTTL(metric) + } return metric } @@ -505,6 +605,8 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( inlinedLVs := inlineLabelValues(lvs, curry) metric = m.newMetric(inlinedLVs...) m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: inlinedLVs, metric: metric}) + } else if m.ttl > 0 { + touchIfTTL(metric) } return metric } @@ -520,6 +622,9 @@ func (m *metricMap) getOrCreateMetricWithLabels( metric, ok := m.getMetricWithHashAndLabels(hash, labels, curry) m.mtx.RUnlock() if ok { + if m.ttl > 0 { + touchIfTTL(metric) + } return metric } @@ -530,6 +635,8 @@ func (m *metricMap) getOrCreateMetricWithLabels( lvs := extractLabelValues(m.desc, labels, curry) metric = m.newMetric(lvs...) m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: lvs, metric: metric}) + } else if m.ttl > 0 { + touchIfTTL(metric) } return metric } From 8d33a81114230be754f8e182ddd5d6c3fd1a9038 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 11:14:08 +0200 Subject: [PATCH 6/9] Refresh TTL on cached child mutators via decorators Wrap Counter/Gauge/Histogram children when *VecOpts.TTL > 0 so Inc, Add, Set, Observe (and exemplar variants) update lastAccessed. Typical cachedObs := vec.WithLabelValues(...); cachedObs.Observe(x) flows keep series alive without re-hashing labels. Also plumb TTL through CounterVecOpts, GaugeVecOpts, and HistogramVecOpts instead of separate *WithTTL constructors. Addresses review feedback on cached child correctness and Opts-based configuration. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/counter.go | 33 ++++++++--- prometheus/gauge.go | 30 +++++++--- prometheus/histogram.go | 20 ++++++- prometheus/ttl.go | 120 ++++++++++++++++++++++++++++++++++++++++ prometheus/vec_test.go | 24 ++++---- 5 files changed, 198 insertions(+), 29 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index 7d963d3af..5a0ef1114 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -15,6 +15,7 @@ package prometheus import ( "errors" + "fmt" "math" "sync/atomic" "time" @@ -70,6 +71,11 @@ type CounterVecOpts struct { // of labels. Each label value will be constrained with the optional Constraint // function, if provided. VariableLabels ConstrainableLabels + + // TTL, if greater than zero, enables per-child expiration for this vector. + // See MetricVecOpts.TTL for semantics, orphaned-handle behavior, and + // cleanup via Registry.Gather / CleanupExpired. + TTL time.Duration } // NewCounter creates a new Counter based on the provided CounterOpts. @@ -211,15 +217,26 @@ func (v2) NewCounterVec(opts CounterVecOpts) *CounterVec { if opts.now == nil { opts.now = time.Now } - return &CounterVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} - result.init(result) // Init self-collection. - result.createdTs = timestamppb.New(opts.now()) + if opts.TTL < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", opts.TTL)) + } + newMetric := func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &counter{desc: desc, labelPairs: MakeLabelPairs(desc, lvs), now: opts.now} + result.init(result) // Init self-collection. + result.createdTs = timestamppb.New(opts.now()) + if opts.TTL <= 0 { return result + } + return newTTLCounter(result) + } + return &CounterVec{ + MetricVec: V2.NewMetricVec(MetricVecOpts{ + Desc: desc, + NewMetric: newMetric, + TTL: opts.TTL, }), } } diff --git a/prometheus/gauge.go b/prometheus/gauge.go index 41e54bf27..7cb5be192 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -14,6 +14,7 @@ package prometheus import ( + "fmt" "math" "sync/atomic" "time" @@ -65,6 +66,10 @@ type GaugeVecOpts struct { // of labels. Each label value will be constrained with the optional Constraint // function, if provided. VariableLabels ConstrainableLabels + + // TTL, if greater than zero, enables per-child expiration for this vector. + // See MetricVecOpts.TTL for semantics. + TTL time.Duration } // NewGauge creates a new Gauge based on the provided GaugeOpts. @@ -166,14 +171,25 @@ func (v2) NewGaugeVec(opts GaugeVecOpts) *GaugeVec { opts.ConstLabels, WithUnit(opts.Unit), ) - return &GaugeVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - if len(lvs) != len(desc.variableLabels.names) { - panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) - } - result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} - result.init(result) // Init self-collection. + if opts.TTL < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", opts.TTL)) + } + newMetric := func(lvs ...string) Metric { + if len(lvs) != len(desc.variableLabels.names) { + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, lvs)) + } + result := &gauge{desc: desc, labelPairs: MakeLabelPairs(desc, lvs)} + result.init(result) // Init self-collection. + if opts.TTL <= 0 { return result + } + return newTTLGauge(result) + } + return &GaugeVec{ + MetricVec: V2.NewMetricVec(MetricVecOpts{ + Desc: desc, + NewMetric: newMetric, + TTL: opts.TTL, }), } } diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 0e788f715..f685cbb29 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -515,6 +515,10 @@ type HistogramVecOpts struct { // of labels. Each label value will be constrained with the optional Constraint // function, if provided. VariableLabels ConstrainableLabels + + // TTL, if greater than zero, enables per-child expiration for this vector. + // See MetricVecOpts.TTL for semantics. + TTL time.Duration } // NewHistogram creates a new Histogram based on the provided HistogramOpts. It @@ -1196,9 +1200,21 @@ func (v2) NewHistogramVec(opts HistogramVecOpts) *HistogramVec { opts.ConstLabels, WithUnit(opts.Unit), ) + if opts.TTL < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", opts.TTL)) + } + newMetric := func(lvs ...string) Metric { + h := newHistogram(desc, opts.HistogramOpts, lvs...) + if opts.TTL <= 0 { + return h + } + return newTTLHistogram(h) + } return &HistogramVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - return newHistogram(desc, opts.HistogramOpts, lvs...) + MetricVec: V2.NewMetricVec(MetricVecOpts{ + Desc: desc, + NewMetric: newMetric, + TTL: opts.TTL, }), } } diff --git a/prometheus/ttl.go b/prometheus/ttl.go index 603bdc095..e588af0c7 100644 --- a/prometheus/ttl.go +++ b/prometheus/ttl.go @@ -13,6 +13,11 @@ package prometheus +import ( + "sync/atomic" + "time" +) + // ExpiredCleaner is implemented by collectors that support TTL-based cleanup of // unused children (for example MetricVec with a non-zero Opts.TTL). // Registry.Gather calls CleanupExpired on registered collectors that implement @@ -28,3 +33,118 @@ type ttlMetric interface { lastAccessed() int64 touch() } + +func nowUnixMilli() int64 { + return time.Now().UnixMilli() +} + +// --- Counter wrapper --- + +type ttlCounter struct { + Counter + lastAccessedTs atomic.Int64 +} + +func newTTLCounter(c Counter) *ttlCounter { + tc := &ttlCounter{Counter: c} + tc.lastAccessedTs.Store(nowUnixMilli()) + return tc +} + +func (c *ttlCounter) Inc() { + c.Counter.Inc() + c.lastAccessedTs.Store(nowUnixMilli()) +} + +func (c *ttlCounter) Add(v float64) { + c.Counter.Add(v) + c.lastAccessedTs.Store(nowUnixMilli()) +} + +func (c *ttlCounter) AddWithExemplar(v float64, e Labels) { + if ea, ok := c.Counter.(ExemplarAdder); ok { + ea.AddWithExemplar(v, e) + } else { + c.Counter.Add(v) + } + c.lastAccessedTs.Store(nowUnixMilli()) +} + +func (c *ttlCounter) lastAccessed() int64 { return c.lastAccessedTs.Load() } +func (c *ttlCounter) touch() { c.lastAccessedTs.Store(nowUnixMilli()) } + +// --- Gauge wrapper --- + +type ttlGauge struct { + Gauge + lastAccessedTs atomic.Int64 +} + +func newTTLGauge(g Gauge) *ttlGauge { + tg := &ttlGauge{Gauge: g} + tg.lastAccessedTs.Store(nowUnixMilli()) + return tg +} + +func (g *ttlGauge) Set(v float64) { + g.Gauge.Set(v) + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) Inc() { + g.Gauge.Inc() + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) Dec() { + g.Gauge.Dec() + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) Add(v float64) { + g.Gauge.Add(v) + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) Sub(v float64) { + g.Gauge.Sub(v) + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) SetToCurrentTime() { + g.Gauge.SetToCurrentTime() + g.lastAccessedTs.Store(nowUnixMilli()) +} + +func (g *ttlGauge) lastAccessed() int64 { return g.lastAccessedTs.Load() } +func (g *ttlGauge) touch() { g.lastAccessedTs.Store(nowUnixMilli()) } + +// --- Histogram wrapper --- + +type ttlHistogram struct { + Histogram + lastAccessedTs atomic.Int64 +} + +func newTTLHistogram(h Histogram) *ttlHistogram { + th := &ttlHistogram{Histogram: h} + th.lastAccessedTs.Store(nowUnixMilli()) + return th +} + +func (h *ttlHistogram) Observe(v float64) { + h.Histogram.Observe(v) + h.lastAccessedTs.Store(nowUnixMilli()) +} + +func (h *ttlHistogram) ObserveWithExemplar(v float64, e Labels) { + if eo, ok := h.Histogram.(ExemplarObserver); ok { + eo.ObserveWithExemplar(v, e) + } else { + h.Histogram.Observe(v) + } + h.lastAccessedTs.Store(nowUnixMilli()) +} + +func (h *ttlHistogram) lastAccessed() int64 { return h.lastAccessedTs.Load() } +func (h *ttlHistogram) touch() { h.lastAccessedTs.Store(nowUnixMilli()) } diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 03223f2f6..fffbfd063 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -48,11 +48,11 @@ func TestDeleteWithCollisions(t *testing.T) { func TestDeleteWithConstraints(t *testing.T) { vec := V2.NewGaugeVec(GaugeVecOpts{ - GaugeOpts{ + GaugeOpts: GaugeOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "l1"}, {Name: "l2", Constraint: func(s string) string { return "x" + s }}, }, @@ -116,11 +116,11 @@ func TestDeleteLabelValuesWithCollisions(t *testing.T) { func TestDeleteLabelValuesWithConstraints(t *testing.T) { vec := V2.NewGaugeVec(GaugeVecOpts{ - GaugeOpts{ + GaugeOpts: GaugeOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "l1"}, {Name: "l2", Constraint: func(s string) string { return "x" + s }}, }, @@ -168,11 +168,11 @@ func TestDeletePartialMatch(t *testing.T) { func TestDeletePartialMatchWithConstraints(t *testing.T) { vec := V2.NewGaugeVec(GaugeVecOpts{ - GaugeOpts{ + GaugeOpts: GaugeOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "l1"}, {Name: "l2", Constraint: func(s string) string { return "x" + s }}, {Name: "l3"}, @@ -344,11 +344,11 @@ func testMetricVec(t *testing.T, vec *GaugeVec) { func TestMetricVecWithConstraints(t *testing.T) { constraint := func(s string) string { return "x" + s } vec := V2.NewGaugeVec(GaugeVecOpts{ - GaugeOpts{ + GaugeOpts: GaugeOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "l1"}, {Name: "l2", Constraint: constraint}, }, @@ -474,11 +474,11 @@ func TestCurryVecWithConstraints(t *testing.T) { constraint := func(s string) string { return "x" + s } t.Run("constrainedLabels overlap variableLabels", func(t *testing.T) { vec := V2.NewCounterVec(CounterVecOpts{ - CounterOpts{ + CounterOpts: CounterOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "one"}, {Name: "two"}, {Name: "three", Constraint: constraint}, @@ -489,11 +489,11 @@ func TestCurryVecWithConstraints(t *testing.T) { t.Run("constrainedLabels reducing cardinality", func(t *testing.T) { constraint := func(s string) string { return "x" } vec := V2.NewCounterVec(CounterVecOpts{ - CounterOpts{ + CounterOpts: CounterOpts{ Name: "test", Help: "helpless", }, - ConstrainedLabels{ + VariableLabels: ConstrainedLabels{ {Name: "one"}, {Name: "two"}, {Name: "three", Constraint: constraint}, From 12af472b795df512b8d59b5ca3f8a3b685c1210d Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 11:14:21 +0200 Subject: [PATCH 7/9] Cleanup expired Vec children during Registry.Gather Call CleanupExpired on collectors that implement ExpiredCleaner before Collect so scrapes reclaim memory. Document that there is no background TTL goroutine: without Gather (or an explicit CleanupExpired), expired children remain allocated. Document orphaned cached handles after TTL the same way Delete already does. Addresses review feedback on no-scrape leaks, Gather-time cleanup, and orphaned refs documentation. Copyright on new ttl.go is 2026. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/registry.go | 9 ++ prometheus/vec_test.go | 244 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 253 insertions(+) diff --git a/prometheus/registry.go b/prometheus/registry.go index 8dd906c6b..5b58c533f 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -432,6 +432,9 @@ func (r *Registry) MustGather() []*dto.MetricFamily { } // Gather implements Gatherer. +// +// Before Collect, Gather calls CleanupExpired on any registered collector that +// implements ExpiredCleaner (for example a MetricVec with a non-zero TTL). func (r *Registry) Gather() ([]*dto.MetricFamily, error) { r.mtx.RLock() @@ -476,8 +479,14 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { for { select { case collector := <-checkedCollectors: + if cleaner, ok := collector.(ExpiredCleaner); ok { + cleaner.CleanupExpired() + } safeErrs.Append((safeCollect(collector, checkedMetricChan))) case collector := <-uncheckedCollectors: + if cleaner, ok := collector.(ExpiredCleaner); ok { + cleaner.CleanupExpired() + } safeErrs.Append(safeCollect(collector, uncheckedMetricChan)) default: return diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index fffbfd063..510b3db34 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -18,6 +18,8 @@ import ( "reflect" "strconv" "testing" + "testing/synctest" + "time" dto "github.com/prometheus/client_model/go" ) @@ -1004,3 +1006,245 @@ func benchmarkMetricVecWithLabelValues(b *testing.B, labels map[string][]string) vec.WithLabelValues(values...) } } + +func collectCount(c Collector) int { + ch := make(chan Metric, 100) + c.Collect(ch) + close(ch) + n := 0 + for range ch { + n++ + } + return n +} + +func TestTTLCounterVec(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewCounterVec(CounterVecOpts{ + CounterOpts: CounterOpts{Name: "test_ttl_counter", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: ttl, + }) + + vec.WithLabelValues("200").Add(1) + vec.WithLabelValues("404").Add(1) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } + + cleaned := vec.CleanupExpired() + if cleaned != 2 { + t.Fatalf("expected 2 cleaned, got %d", cleaned) + } + }) +} + +func TestTTLGaugeVec(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewGaugeVec(GaugeVecOpts{ + GaugeOpts: GaugeOpts{Name: "test_ttl_gauge", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"method"}), + TTL: ttl, + }) + + vec.WithLabelValues("GET").Set(10) + vec.WithLabelValues("POST").Set(20) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + // Touch only one + vec.WithLabelValues("GET").Set(30) + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric after partial TTL, got %d", n) + } + + cleaned := vec.CleanupExpired() + if cleaned != 1 { + t.Fatalf("expected 1 cleaned, got %d", cleaned) + } + }) +} + +func TestTTLHistogramVec(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewHistogramVec(HistogramVecOpts{ + HistogramOpts: HistogramOpts{Name: "test_ttl_histo", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"status"}), + TTL: ttl, + }) + + vec.WithLabelValues("ok").Observe(0.5) + vec.WithLabelValues("err").Observe(1.5) + + if n := collectCount(vec); n != 2 { + t.Fatalf("expected 2 metrics, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after TTL, got %d", n) + } + }) +} + +func TestTTLCachedChildKeepsAlive(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewHistogramVec(HistogramVecOpts{ + HistogramOpts: HistogramOpts{Name: "test_ttl_cached", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"status"}), + TTL: ttl, + }) + + cached := vec.WithLabelValues("ok") + cached.Observe(0.5) + + // Hot path: only Observe on the cached child (no WithLabelValues). + for i := 0; i < 5; i++ { + time.Sleep(80 * time.Millisecond) + cached.Observe(0.1) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric still alive via cached Observe, got %d", n) + } + }) +} + +func TestTTLRefreshPreventsExpiration(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 150 * time.Millisecond + vec := V2.NewCounterVec(CounterVecOpts{ + CounterOpts: CounterOpts{Name: "test_ttl_refresh", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: ttl, + }) + + cached := vec.WithLabelValues("200") + cached.Add(1) + + for i := 0; i < 5; i++ { + time.Sleep(80 * time.Millisecond) + cached.Add(1) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric still alive, got %d", n) + } + }) +} + +func TestMetricVecOptsTTLZeroAndNegative(t *testing.T) { + desc := NewDesc("test", "help", []string{"l"}, nil) + newMetric := func(lvs ...string) Metric { return &counter{} } + + mv0 := V2.NewMetricVec(MetricVecOpts{Desc: desc, NewMetric: newMetric, TTL: 0}) + if mv0.ttl != 0 { + t.Fatal("ttl==0 should leave metricMap.ttl at 0") + } + if cleaned := mv0.CleanupExpired(); cleaned != 0 { + t.Fatalf("expected 0 cleaned, got %d", cleaned) + } + + defer func() { + if recover() == nil { + t.Fatal("expected panic for negative ttl") + } + }() + V2.NewMetricVec(MetricVecOpts{Desc: desc, NewMetric: newMetric, TTL: -time.Second}) +} + +func TestTTLZeroMeansNoExpiration(t *testing.T) { + vec := NewCounterVec(CounterOpts{ + Name: "test_no_ttl", + Help: "test", + }, []string{"code"}) + + vec.WithLabelValues("200").Add(1) + + cleaned := vec.CleanupExpired() + if cleaned != 0 { + t.Fatalf("expected 0 cleaned with no TTL, got %d", cleaned) + } + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric, got %d", n) + } +} + +func TestTTLWithGetMetricWith(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewGaugeVec(GaugeVecOpts{ + GaugeOpts: GaugeOpts{Name: "test_ttl_getmetricwith", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"method"}), + TTL: ttl, + }) + + g, err := vec.GetMetricWith(Labels{"method": "GET"}) + if err != nil { + t.Fatal(err) + } + g.Set(42) + + time.Sleep(ttl + 50*time.Millisecond) + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 after TTL, got %d", n) + } + + g, _ = vec.GetMetricWith(Labels{"method": "GET"}) + g.Set(99) + + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 after re-access, got %d", n) + } + }) +} + +func TestRegistryGatherCleansExpired(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 80 * time.Millisecond + reg := NewRegistry() + vec := V2.NewCounterVec(CounterVecOpts{ + CounterOpts: CounterOpts{Name: "ttl_reg_gather", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: ttl, + }) + reg.MustRegister(vec) + + vec.WithLabelValues("200").Add(1) + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric before sleep, got %d", n) + } + + time.Sleep(ttl + 40*time.Millisecond) + + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 metrics after Gather cleanup, got %d", n) + } + if cleaned := vec.CleanupExpired(); cleaned != 0 { + t.Fatalf("expected nothing left to clean, got %d", cleaned) + } + }) +} + From 649400b93cc305e6acd5a11ea21a4f7606a88ce9 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 11:25:57 +0200 Subject: [PATCH 8/9] Harden TTL Gather path and cover new edge cases Only run CleanupExpired during Gather when ttlEnabled is set, panic if a custom MetricVec with TTL returns a non-TTL Metric, add SummaryVec TTL wrappers, and document wrapper/allocation trade-offs. Tests cover no-wrapper for TTL==0, orphaned cached handles, custom-metric panic, SummaryVec, and Gather skipping non-TTL collectors. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/registry.go | 9 ++- prometheus/summary.go | 20 ++++- prometheus/ttl.go | 36 ++++++++- prometheus/vec.go | 39 +++++++-- prometheus/vec_test.go | 180 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 267 insertions(+), 17 deletions(-) diff --git a/prometheus/registry.go b/prometheus/registry.go index 5b58c533f..b00dc3f8e 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -433,8 +433,9 @@ func (r *Registry) MustGather() []*dto.MetricFamily { // Gather implements Gatherer. // -// Before Collect, Gather calls CleanupExpired on any registered collector that -// implements ExpiredCleaner (for example a MetricVec with a non-zero TTL). +// Before Collect, Gather calls CleanupExpired on registered collectors that +// implement ExpiredCleaner and have TTL enabled, so expired Vec children can be +// reclaimed on scrape without touching non-TTL collectors. func (r *Registry) Gather() ([]*dto.MetricFamily, error) { r.mtx.RLock() @@ -479,12 +480,12 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { for { select { case collector := <-checkedCollectors: - if cleaner, ok := collector.(ExpiredCleaner); ok { + if cleaner, ok := collector.(ttlEnabledCollector); ok && cleaner.ttlEnabled() { cleaner.CleanupExpired() } safeErrs.Append((safeCollect(collector, checkedMetricChan))) case collector := <-uncheckedCollectors: - if cleaner, ok := collector.(ExpiredCleaner); ok { + if cleaner, ok := collector.(ttlEnabledCollector); ok && cleaner.ttlEnabled() { cleaner.CleanupExpired() } safeErrs.Append(safeCollect(collector, uncheckedMetricChan)) diff --git a/prometheus/summary.go b/prometheus/summary.go index c12b8d13d..1186f4eb7 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -164,6 +164,10 @@ type SummaryVecOpts struct { // of labels. Each label value will be constrained with the optional Constraint // function, if provided. VariableLabels ConstrainableLabels + + // TTL, if greater than zero, enables per-child expiration for this vector. + // See MetricVecOpts.TTL for semantics. + TTL time.Duration } // Problem with the sliding-window decay algorithm... The Merge method of @@ -577,6 +581,9 @@ func (v2) NewSummaryVec(opts SummaryVecOpts) *SummaryVec { panic(errQuantileLabelNotAllowed) } } + if opts.TTL < 0 { + panic(fmt.Sprintf("invalid negative ttl: %v", opts.TTL)) + } desc := V2.NewDesc( BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), opts.Help, @@ -584,9 +591,18 @@ func (v2) NewSummaryVec(opts SummaryVecOpts) *SummaryVec { opts.ConstLabels, WithUnit(opts.Unit), ) + newMetric := func(lvs ...string) Metric { + s := newSummary(desc, opts.SummaryOpts, lvs...) + if opts.TTL <= 0 { + return s + } + return newTTLSummary(s) + } return &SummaryVec{ - MetricVec: NewMetricVec(desc, func(lvs ...string) Metric { - return newSummary(desc, opts.SummaryOpts, lvs...) + MetricVec: V2.NewMetricVec(MetricVecOpts{ + Desc: desc, + NewMetric: newMetric, + TTL: opts.TTL, }), } } diff --git a/prometheus/ttl.go b/prometheus/ttl.go index e588af0c7..1b0222bf5 100644 --- a/prometheus/ttl.go +++ b/prometheus/ttl.go @@ -20,13 +20,22 @@ import ( // ExpiredCleaner is implemented by collectors that support TTL-based cleanup of // unused children (for example MetricVec with a non-zero Opts.TTL). -// Registry.Gather calls CleanupExpired on registered collectors that implement -// this interface so expired children can be reclaimed even when Collect alone -// would only skip them. +// +// Registry.Gather only invokes CleanupExpired on collectors that also report +// TTL as enabled (see ttlEnabled), so vectors with TTL == 0 are not touched on +// the Gather hot path. type ExpiredCleaner interface { CleanupExpired() int } +// ttlEnabledCollector is the Gather-time check for automatic TTL cleanup. +// ttlEnabled is unexported so only types in this package (e.g. *MetricVec and +// the built-in *Vec types) can opt into automatic cleanup. +type ttlEnabledCollector interface { + ExpiredCleaner + ttlEnabled() bool +} + // ttlMetric is implemented by decorator wrappers that track last access time. type ttlMetric interface { Metric @@ -148,3 +157,24 @@ func (h *ttlHistogram) ObserveWithExemplar(v float64, e Labels) { func (h *ttlHistogram) lastAccessed() int64 { return h.lastAccessedTs.Load() } func (h *ttlHistogram) touch() { h.lastAccessedTs.Store(nowUnixMilli()) } + +// --- Summary wrapper --- + +type ttlSummary struct { + Summary + lastAccessedTs atomic.Int64 +} + +func newTTLSummary(s Summary) *ttlSummary { + ts := &ttlSummary{Summary: s} + ts.lastAccessedTs.Store(nowUnixMilli()) + return ts +} + +func (s *ttlSummary) Observe(v float64) { + s.Summary.Observe(v) + s.lastAccessedTs.Store(nowUnixMilli()) +} + +func (s *ttlSummary) lastAccessed() int64 { return s.lastAccessedTs.Load() } +func (s *ttlSummary) touch() { s.lastAccessedTs.Store(nowUnixMilli()) } diff --git a/prometheus/vec.go b/prometheus/vec.go index 312d23267..d93c1bd03 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -57,11 +57,19 @@ type MetricVecOpts struct { // expiration (identical to NewMetricVec). // // Access includes GetMetricWith / GetMetricWithLabelValues and, when using - // the built-in CounterVec / GaugeVec / HistogramVec with TTL, mutating - // methods on cached children (Inc, Add, Set, Observe, …). Caching a child - // and never calling those methods (nor looking it up again) lets the child - // expire; the cached handle then behaves like after Delete — updates are - // not exported until the label set is looked up again. See Delete docs. + // the built-in CounterVec / GaugeVec / HistogramVec / SummaryVec with TTL, + // mutating methods on cached children (Inc, Add, Set, Observe, …). Caching + // a child and never calling those methods (nor looking it up again) lets + // the child expire; the cached handle then behaves like after Delete — + // updates are not exported until the label set is looked up again. See + // Delete docs. + // + // When TTL > 0, NewMetric must return a Metric that implements the internal + // TTL touch hooks (the built-in *Vec constructors wrap children for you). + // Passing a plain Metric panics when the child is created. + // + // TTL > 0 adds a small per-child wrapper allocation and a timestamp update + // on each mutating call; TTL == 0 keeps the default Vec path with neither. // // If metrics are never scraped, call CleanupExpired periodically (or rely // on Gather) so expired children can be reclaimed; there is no background @@ -95,13 +103,17 @@ func (v2) NewMetricVec(opts MetricVecOpts) *MetricVec { // configured TTL. It returns the number of children removed. If TTL is not // configured (zero), this is a no-op and returns 0. // -// Registry.Gather invokes CleanupExpired for collectors that implement -// ExpiredCleaner. If scrapes are rare or absent, call CleanupExpired -// periodically yourself; client_golang does not start a background cleaner. +// Registry.Gather invokes CleanupExpired only for collectors with TTL enabled. +// If scrapes are rare or absent, call CleanupExpired periodically yourself; +// client_golang does not start a background cleaner. func (m *MetricVec) CleanupExpired() int { return m.metricMap.cleanupExpired() } +func (m *MetricVec) ttlEnabled() bool { + return m.metricMap.ttl > 0 +} + // DeleteLabelValues removes the metric where the variable labels are the same // as those passed in as labels (same order as the VariableLabels in Desc). It // returns true if a metric was deleted. @@ -604,6 +616,7 @@ func (m *metricMap) getOrCreateMetricWithLabelValues( if !ok { inlinedLVs := inlineLabelValues(lvs, curry) metric = m.newMetric(inlinedLVs...) + m.requireTTLMetric(metric) m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: inlinedLVs, metric: metric}) } else if m.ttl > 0 { touchIfTTL(metric) @@ -634,6 +647,7 @@ func (m *metricMap) getOrCreateMetricWithLabels( if !ok { lvs := extractLabelValues(m.desc, labels, curry) metric = m.newMetric(lvs...) + m.requireTTLMetric(metric) m.metrics[hash] = append(m.metrics[hash], metricWithLabelValues{values: lvs, metric: metric}) } else if m.ttl > 0 { touchIfTTL(metric) @@ -641,6 +655,15 @@ func (m *metricMap) getOrCreateMetricWithLabels( return metric } +func (m *metricMap) requireTTLMetric(metric Metric) { + if m.ttl <= 0 { + return + } + if _, ok := metric.(ttlMetric); !ok { + panic("MetricVec with TTL > 0 requires NewMetric to return a TTL-aware Metric; use CounterVec/GaugeVec/HistogramVec/SummaryVec Opts.TTL or wrap the Metric yourself") + } +} + // getMetricWithHashAndLabelValues gets a metric while handling possible // collisions in the hash space. Must be called while holding the read mutex. func (m *metricMap) getMetricWithHashAndLabelValues( diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 510b3db34..3dc84aa67 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -1248,3 +1248,183 @@ func TestRegistryGatherCleansExpired(t *testing.T) { }) } +func TestTTLZeroHasNoWrapper(t *testing.T) { + vec := NewCounterVec(CounterOpts{Name: "ttl_zero_wrap", Help: "test"}, []string{"code"}) + c := vec.WithLabelValues("200") + if _, ok := c.(*ttlCounter); ok { + t.Fatal("TTL==0 must not wrap children in ttlCounter") + } + if _, ok := c.(*counter); !ok { + t.Fatalf("TTL==0 child should be *counter, got %T", c) + } + if vec.ttlEnabled() { + t.Fatal("TTL==0 vector must not report ttlEnabled") + } +} + +func TestTTLWrapsChildren(t *testing.T) { + vec := V2.NewCounterVec(CounterVecOpts{ + CounterOpts: CounterOpts{Name: "ttl_wrap", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: time.Minute, + }) + c := vec.WithLabelValues("200") + if _, ok := c.(*ttlCounter); !ok { + t.Fatalf("TTL>0 child should be *ttlCounter, got %T", c) + } + if !vec.ttlEnabled() { + t.Fatal("TTL>0 vector must report ttlEnabled") + } + + gvec := V2.NewGaugeVec(GaugeVecOpts{ + GaugeOpts: GaugeOpts{Name: "ttl_wrap_g", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: time.Minute, + }) + if _, ok := gvec.WithLabelValues("200").(*ttlGauge); !ok { + t.Fatal("expected *ttlGauge") + } + + hvec := V2.NewHistogramVec(HistogramVecOpts{ + HistogramOpts: HistogramOpts{Name: "ttl_wrap_h", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: time.Minute, + }) + if _, ok := hvec.WithLabelValues("200").(*ttlHistogram); !ok { + t.Fatal("expected *ttlHistogram") + } + + svec := V2.NewSummaryVec(SummaryVecOpts{ + SummaryOpts: SummaryOpts{Name: "ttl_wrap_s", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: time.Minute, + }) + if _, ok := svec.WithLabelValues("200").(*ttlSummary); !ok { + t.Fatal("expected *ttlSummary") + } +} + +func TestTTLCustomMetricVecRequiresTTLMetric(t *testing.T) { + desc := NewDesc("ttl_custom", "help", []string{"l"}, nil) + mv := V2.NewMetricVec(MetricVecOpts{ + Desc: desc, + NewMetric: func(lvs ...string) Metric { return &counter{} }, + TTL: time.Minute, + }) + defer func() { + if recover() == nil { + t.Fatal("expected panic when NewMetric does not return a ttlMetric") + } + }() + _, _ = mv.GetMetricWithLabelValues("x") +} + +func TestTTLOrphanedCachedHandleAfterCleanup(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 50 * time.Millisecond + vec := V2.NewCounterVec(CounterVecOpts{ + CounterOpts: CounterOpts{Name: "ttl_orphan", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: ttl, + }) + + cached := vec.WithLabelValues("200") + cached.Add(1) + + time.Sleep(ttl + 20*time.Millisecond) + if n := vec.CleanupExpired(); n != 1 { + t.Fatalf("expected 1 cleaned, got %d", n) + } + + // Cached handle is detached: updates are not exported. + cached.Add(5) + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 exported after orphan update, got %d", n) + } + + // Re-lookup creates a fresh child. + fresh := vec.WithLabelValues("200") + if fresh == cached { + t.Fatal("expected a new child metric after cleanup") + } + fresh.Add(1) + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 after re-lookup, got %d", n) + } + }) +} + +func TestTTLSummaryVec(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ttl := 100 * time.Millisecond + vec := V2.NewSummaryVec(SummaryVecOpts{ + SummaryOpts: SummaryOpts{Name: "test_ttl_summary", Help: "test"}, + VariableLabels: UnconstrainedLabels([]string{"code"}), + TTL: ttl, + }) + + cached := vec.WithLabelValues("ok") + cached.Observe(0.5) + + for i := 0; i < 5; i++ { + time.Sleep(80 * time.Millisecond) + cached.Observe(0.1) + } + if n := collectCount(vec); n != 1 { + t.Fatalf("expected 1 metric via cached Observe, got %d", n) + } + + time.Sleep(ttl + 50*time.Millisecond) + if n := collectCount(vec); n != 0 { + t.Fatalf("expected 0 after idle TTL, got %d", n) + } + }) +} + +// cleanupCallSpy tracks whether Gather invoked CleanupExpired. +type cleanupCallSpy struct { + selfCollector + desc *Desc + calls int + enableTTL bool +} + +func (s *cleanupCallSpy) Desc() *Desc { return s.desc } +func (s *cleanupCallSpy) Write(out *dto.Metric) error { + return populateMetric(GaugeValue, 0, nil, nil, out, nil) +} +func (s *cleanupCallSpy) CleanupExpired() int { + s.calls++ + return 0 +} +func (s *cleanupCallSpy) ttlEnabled() bool { return s.enableTTL } + +func TestRegistryGatherSkipsCleanupWhenTTLDisabled(t *testing.T) { + reg := NewRegistry() + + disabled := &cleanupCallSpy{desc: NewDesc("spy_disabled", "help", nil, nil), enableTTL: false} + disabled.init(disabled) + enabled := &cleanupCallSpy{desc: NewDesc("spy_enabled", "help", nil, nil), enableTTL: true} + enabled.init(enabled) + + reg.MustRegister(disabled, enabled) + if _, err := reg.Gather(); err != nil { + t.Fatal(err) + } + if disabled.calls != 0 { + t.Fatalf("Gather must not CleanupExpired when ttlEnabled is false, got %d calls", disabled.calls) + } + if enabled.calls != 1 { + t.Fatalf("Gather must CleanupExpired when ttlEnabled is true, got %d calls", enabled.calls) + } + + // Non-TTL built-in vecs must not report ttlEnabled. + plain := NewCounterVec(CounterOpts{Name: "plain_gather", Help: "test"}, []string{"c"}) + if _, ok := any(plain).(ttlEnabledCollector); !ok { + t.Fatal("CounterVec should satisfy ttlEnabledCollector via MetricVec") + } + if plain.ttlEnabled() { + t.Fatal("plain CounterVec must not be ttlEnabled") + } +} + From 2b5c23bfc5393f9225d8683f0695ebf743c28262 Mon Sep 17 00:00:00 2001 From: Julien Pinsonneau Date: Tue, 21 Jul 2026 12:12:27 +0200 Subject: [PATCH 9/9] Fix lint and license headers for TTL changes Drop year from ttl.go copyright (repo policy for 2026+), use promoted metricMap selectors for staticcheck QF1008, and gofumpt vec_test helpers. Signed-off-by: Julien Pinsonneau Made-with: Cursor --- prometheus/ttl.go | 2 +- prometheus/vec.go | 4 ++-- prometheus/vec_test.go | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/prometheus/ttl.go b/prometheus/ttl.go index 1b0222bf5..ebe153598 100644 --- a/prometheus/ttl.go +++ b/prometheus/ttl.go @@ -1,4 +1,4 @@ -// Copyright 2026 The Prometheus Authors +// Copyright The Prometheus Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/prometheus/vec.go b/prometheus/vec.go index d93c1bd03..60acf006a 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -107,11 +107,11 @@ func (v2) NewMetricVec(opts MetricVecOpts) *MetricVec { // If scrapes are rare or absent, call CleanupExpired periodically yourself; // client_golang does not start a background cleaner. func (m *MetricVec) CleanupExpired() int { - return m.metricMap.cleanupExpired() + return m.cleanupExpired() } func (m *MetricVec) ttlEnabled() bool { - return m.metricMap.ttl > 0 + return m.ttl > 0 } // DeleteLabelValues removes the metric where the variable labels are the same diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 3dc84aa67..1a6504c06 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -1390,13 +1390,16 @@ type cleanupCallSpy struct { } func (s *cleanupCallSpy) Desc() *Desc { return s.desc } + func (s *cleanupCallSpy) Write(out *dto.Metric) error { return populateMetric(GaugeValue, 0, nil, nil, out, nil) } + func (s *cleanupCallSpy) CleanupExpired() int { s.calls++ return 0 } + func (s *cleanupCallSpy) ttlEnabled() bool { return s.enableTTL } func TestRegistryGatherSkipsCleanupWhenTTLDisabled(t *testing.T) { @@ -1427,4 +1430,3 @@ func TestRegistryGatherSkipsCleanupWhenTTLDisabled(t *testing.T) { t.Fatal("plain CounterVec must not be ttlEnabled") } } -