diff --git a/app/health/checker.go b/app/health/checker.go index 7ec4a7448f..58277e99ae 100644 --- a/app/health/checker.go +++ b/app/health/checker.go @@ -22,9 +22,9 @@ const ( maxScrapes = 10 // maxSSEScrapes is the number of scrapes kept for the SSE head delay check (1 hour at 30s intervals). maxSSEScrapes = 120 - // labelsCardinalityThreshold is the threshold for single validator; - // for N validators, the threshold is N * labelsCardinalityThreshold. - labelsCardinalityThreshold = 100 + // seriesCardinalityThreshold is the maximum number of time series per metric family + // for a single validator; for N validators, the threshold is N * seriesCardinalityThreshold. + seriesCardinalityThreshold = 100 // memorySamplePeriod is the period between memory snapshots. memorySamplePeriod = 30 * time.Minute @@ -156,17 +156,9 @@ func (c *Checker) scrape() error { continue } - var maxLabelsCount int - - for _, fam := range fams.GetMetric() { - labelsCount := len(fam.GetLabel()) - if labelsCount > maxLabelsCount { - maxLabelsCount = labelsCount - } - } - - if maxLabelsCount > labelsCardinalityThreshold*c.numValidators { - highCardinalityGauge.WithLabelValues(fams.GetName()).Set(float64(maxLabelsCount)) + seriesCount := len(fams.GetMetric()) + if seriesCount > seriesCardinalityThreshold*max(c.numValidators, 1) { + highCardinalityGauge.WithLabelValues(fams.GetName()).Set(float64(seriesCount)) gatherAgain = true } diff --git a/app/health/checker_internal_test.go b/app/health/checker_internal_test.go new file mode 100644 index 0000000000..fc544502d8 --- /dev/null +++ b/app/health/checker_internal_test.go @@ -0,0 +1,34 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package health + +import ( + "strconv" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" +) + +func TestScrapeHighCardinality(t *testing.T) { + registry := prometheus.NewRegistry() + + highVec := prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_high_cardinality_metric", Help: "test"}, []string{"label"}) + lowVec := prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_low_cardinality_metric", Help: "test"}, []string{"label"}) + registry.MustRegister(highVec, lowVec) + + for i := range seriesCardinalityThreshold + 1 { + highVec.WithLabelValues(strconv.Itoa(i)).Set(1) + } + + lowVec.WithLabelValues("0").Set(1) + + checker := NewChecker(Metadata{}, registry, 1) + require.NoError(t, checker.scrape()) + + require.InDelta(t, float64(seriesCardinalityThreshold+1), + testutil.ToFloat64(highCardinalityGauge.WithLabelValues("test_high_cardinality_metric")), 0) + require.InDelta(t, float64(0), + testutil.ToFloat64(highCardinalityGauge.WithLabelValues("test_low_cardinality_metric")), 0) +} diff --git a/app/promauto/resetgauge.go b/app/promauto/resetgauge.go index 40a4ac3dd1..2ba44cf599 100644 --- a/app/promauto/resetgauge.go +++ b/app/promauto/resetgauge.go @@ -3,19 +3,19 @@ package promauto import ( + "fmt" + "slices" "strings" "sync" "github.com/prometheus/client_golang/prometheus" ) -const separator = "|" - // NewResetGaugeVec creates a new ResetGaugeVec. func NewResetGaugeVec(opts prometheus.GaugeOpts, labelNames []string) *ResetGaugeVec { return &ResetGaugeVec{ inner: NewGaugeVec(opts, labelNames), - labels: make(map[string]bool), + labels: make(map[string][]string), } } @@ -25,45 +25,41 @@ type ResetGaugeVec struct { inner *prometheus.GaugeVec mu sync.Mutex - labels map[string]bool + labels map[string][]string } func (g *ResetGaugeVec) WithLabelValues(lvs ...string) prometheus.Gauge { - for _, lv := range lvs { - if strings.Contains(lv, separator) { - panic("label value cannot contain separator") - } - } - g.mu.Lock() defer g.mu.Unlock() - g.labels[strings.Join(lvs, separator)] = true + g.labels[labelsKey(lvs)] = lvs return g.inner.WithLabelValues(lvs...) } -// Reset deletes all previously set labels that match all the given label values. -// An empty slice will delete all previously set labels. +// Reset deletes all previously set label combinations whose leading label values equal lvs. +// An empty lvs deletes all previously set label combinations. func (g *ResetGaugeVec) Reset(lvs ...string) { g.mu.Lock() defer g.mu.Unlock() - for label := range g.labels { - match := true - - for _, check := range lvs { - if !strings.Contains(label, check) { - match = false - break - } - } - - if !match { + for key, labels := range g.labels { + if len(lvs) > len(labels) || !slices.Equal(labels[:len(lvs)], lvs) { continue } - g.inner.DeleteLabelValues(strings.Split(label, separator)...) - delete(g.labels, label) + g.inner.DeleteLabelValues(labels...) + delete(g.labels, key) + } +} + +// labelsKey returns an unambiguous map key for the label values, +// using length prefixes so values may contain any characters. +func labelsKey(lvs []string) string { + var sb strings.Builder + for _, lv := range lvs { + _, _ = fmt.Fprintf(&sb, "%d:%s", len(lv), lv) } + + return sb.String() } diff --git a/app/promauto/resetgauge_test.go b/app/promauto/resetgauge_test.go index c905f41ca4..d4ca2c6340 100644 --- a/app/promauto/resetgauge_test.go +++ b/app/promauto/resetgauge_test.go @@ -11,12 +11,22 @@ import ( "github.com/obolnetwork/charon/app/promauto" ) -const resetTest = "reset_test" +const ( + resetTest = "reset_test" + resetTestExact = "reset_test_exact" +) + +var ( + testResetGauge = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ + Name: resetTest, + Help: "", + }, []string{"label0", "label1"}) -var testResetGauge = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ - Name: resetTest, - Help: "", -}, []string{"label0", "label1"}) + testResetGaugeExact = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ + Name: resetTestExact, + Help: "", + }, []string{"label0", "label1"}) +) func TestResetGaugeVec(t *testing.T) { registry, err := promauto.NewRegistry(nil) @@ -54,7 +64,33 @@ func TestResetGaugeVec(t *testing.T) { assertVecLen(t, registry, resetTest, 1) } -func assertVecLen(t *testing.T, registry *prometheus.Registry, name string, l int) { //nolint:unparam // abstracting name is fine even though it is always currently constant +func TestResetGaugeVecExactPrefix(t *testing.T) { + registry, err := promauto.NewRegistry(nil) + require.NoError(t, err) + + // Label values may contain any characters, including "|". + testResetGaugeExact.WithLabelValues("with|pipe", "a").Set(1) + assertVecLen(t, registry, resetTestExact, 1) + + testResetGaugeExact.WithLabelValues("foobar", "b").Set(1) + assertVecLen(t, registry, resetTestExact, 2) + + // Substring of a label value must not match. + testResetGaugeExact.Reset("foo") + assertVecLen(t, registry, resetTestExact, 2) + + // Non-leading label values must not match. + testResetGaugeExact.Reset("a") + assertVecLen(t, registry, resetTestExact, 2) + + testResetGaugeExact.Reset("foobar") + assertVecLen(t, registry, resetTestExact, 1) + + testResetGaugeExact.Reset("with|pipe", "a") + assertVecLen(t, registry, resetTestExact, 0) +} + +func assertVecLen(t *testing.T, registry *prometheus.Registry, name string, l int) { t.Helper() metrics, err := registry.Gather() diff --git a/cmd/relay/metrics.go b/cmd/relay/metrics.go index 4a0c5a8dd1..f336fb9403 100644 --- a/cmd/relay/metrics.go +++ b/cmd/relay/metrics.go @@ -50,6 +50,19 @@ var ( }, []string{"peer", "peer_cluster"}) ) +// deletePeerMetrics deletes all metric series of the given peer. The relay accepts +// connections from arbitrary peers and clusters, so series must be deleted on disconnect +// to prevent unbounded growth. +func deletePeerMetrics(peerName string) { + match := prometheus.Labels{"peer": peerName} + + newConnsCounter.DeletePartialMatch(match) + activeConnsCounter.DeletePartialMatch(match) + networkTXCounter.DeletePartialMatch(match) + networkRXCounter.DeletePartialMatch(match) + peerPingLatency.DeletePartialMatch(match) +} + // newBandwidthCounter returns a new bandwidth counter that stops counting when the context is cancelled. func newBandwidthCounter(ctx context.Context, ch chan<- bwTuple) metrics.Reporter { return bwCounter{ diff --git a/cmd/relay/metrics_internal_test.go b/cmd/relay/metrics_internal_test.go new file mode 100644 index 0000000000..4a3060daf2 --- /dev/null +++ b/cmd/relay/metrics_internal_test.go @@ -0,0 +1,38 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package relay + +import ( + "testing" + + promtestutil "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" +) + +func TestDeletePeerMetrics(t *testing.T) { + countersBefore := promtestutil.CollectAndCount(newConnsCounter) + pingsBefore := promtestutil.CollectAndCount(peerPingLatency) + + // One peer with series in multiple clusters, including the empty pre-peerinfo cluster. + newConnsCounter.WithLabelValues("test-peer1", "cluster1").Add(1) + newConnsCounter.WithLabelValues("test-peer1", "cluster2").Add(1) + newConnsCounter.WithLabelValues("test-peer1", "").Add(1) + activeConnsCounter.WithLabelValues("test-peer1", "cluster1").Set(1) + networkTXCounter.WithLabelValues("test-peer1", "cluster1").Add(1) + networkRXCounter.WithLabelValues("test-peer1", "cluster1").Add(1) + peerPingLatency.WithLabelValues("test-peer1", "cluster1").Observe(1) + + newConnsCounter.WithLabelValues("test-peer2", "cluster1").Add(1) + + require.Equal(t, countersBefore+4, promtestutil.CollectAndCount(newConnsCounter)) + require.Equal(t, pingsBefore+1, promtestutil.CollectAndCount(peerPingLatency)) + + deletePeerMetrics("test-peer1") + + require.Equal(t, countersBefore+1, promtestutil.CollectAndCount(newConnsCounter)) + require.Equal(t, pingsBefore, promtestutil.CollectAndCount(peerPingLatency)) + + deletePeerMetrics("test-peer2") + + require.Equal(t, countersBefore, promtestutil.CollectAndCount(newConnsCounter)) +} diff --git a/cmd/relay/p2p.go b/cmd/relay/p2p.go index 3283eeb806..f04bf3f1a8 100644 --- a/cmd/relay/p2p.go +++ b/cmd/relay/p2p.go @@ -163,12 +163,9 @@ func monitorConnections(ctx context.Context, p2pNode host.Host, bwTuples <-chan // Periodically request peerinfo for all peers we have active connections to. for p, state := range peers { if state.Active == 0 { - // No active connections, remove peer from state. + // No active connections, remove peer from state and delete its metric series. delete(peers, p) - - if state.ClusterHash != "" { - activeConnsCounter.WithLabelValues(state.Name, state.ClusterHash).Set(0) - } + deletePeerMetrics(state.Name) continue } diff --git a/core/validatorapi/router.go b/core/validatorapi/router.go index 54f2d49e21..f323cc2976 100644 --- a/core/validatorapi/router.go +++ b/core/validatorapi/router.go @@ -59,6 +59,8 @@ const ( executionPayloadValueHeader = "Eth-Execution-Payload-Value" consensusBlockValueHeader = "Eth-Consensus-Block-Value" defaultRequestTimeout = 10 * time.Second + // maxUserAgentLen bounds the untrusted User-Agent header used as a metric label value. + maxUserAgentLen = 128 ) // Handler defines the request handler providing the business logic @@ -405,6 +407,12 @@ func wrap(endpoint string, handler handlerFunc, encodings []contentType) http.Ha userAgent := r.Header.Get("User-Agent") if userAgent != "" { + if len(userAgent) > maxUserAgentLen { + userAgent = userAgent[:maxUserAgentLen] + } + + userAgent = strings.ToValidUTF8(userAgent, "") + vcUserAgentGauge.Reset() vcUserAgentGauge.WithLabelValues(userAgent).Set(1) }