From 93d20ee14a80ed97133f34912b53afeddfc1520c Mon Sep 17 00:00:00 2001 From: corverroos Date: Sun, 30 Jul 2023 11:09:39 +0200 Subject: [PATCH 1/4] app/promauto: support partial resets --- app/metrics.go | 2 +- app/monitoringapi.go | 10 +--------- app/peerinfo/metrics.go | 4 ++-- app/peerinfo/peerinfo.go | 23 ++--------------------- app/promauto/resetgauge.go | 25 +++++++++++++++++++++++-- app/promauto/resetgauge_test.go | 29 ++++++++++++++++++++++++----- core/scheduler/metrics.go | 12 +++--------- p2p/p2p.go | 3 ++- 8 files changed, 58 insertions(+), 50 deletions(-) diff --git a/app/metrics.go b/app/metrics.go index 5e3ccf777..c05e78d0c 100644 --- a/app/metrics.go +++ b/app/metrics.go @@ -75,7 +75,7 @@ var ( Help: "Gauge set to the peer count of the upstream beacon node", }) - beaconNodeVersionGauge = promauto.NewGaugeVec(prometheus.GaugeOpts{ + beaconNodeVersionGauge = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ Namespace: "app", Subsystem: "beacon_node", Name: "version", diff --git a/app/monitoringapi.go b/app/monitoringapi.go index 6dd22b562..c156bd2b7 100644 --- a/app/monitoringapi.go +++ b/app/monitoringapi.go @@ -217,23 +217,15 @@ func beaconNodeSyncing(ctx context.Context, eth2Cl eth2client.NodeSyncingProvide func beaconNodeVersionMetric(ctx context.Context, eth2Cl eth2wrap.Client, clock clockwork.Clock) { nodeVersionTicker := clock.NewTicker(10 * time.Minute) - // TODO(corver): Refactor to use ResetGauge. - var prevNodeVersion string setNodeVersion := func() { version, err := eth2Cl.NodeVersion(ctx) if err != nil { log.Error(ctx, "Failed to get beacon node version", err) return } - if version == prevNodeVersion { - return - } - if prevNodeVersion != "" { - beaconNodeVersionGauge.WithLabelValues(prevNodeVersion).Set(0) - } + beaconNodeVersionGauge.ResetAll() beaconNodeVersionGauge.WithLabelValues(version).Set(1) - prevNodeVersion = version } go func() { diff --git a/app/peerinfo/metrics.go b/app/peerinfo/metrics.go index 3b3f983f9..8ba457f42 100644 --- a/app/peerinfo/metrics.go +++ b/app/peerinfo/metrics.go @@ -17,7 +17,7 @@ var ( ConstLabels: nil, }, []string{"peer"}) - peerVersion = promauto.NewGaugeVec(prometheus.GaugeOpts{ + peerVersion = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ Namespace: "app", Subsystem: "peerinfo", Name: "version", @@ -25,7 +25,7 @@ var ( ConstLabels: nil, }, []string{"peer", "version"}) - peerGitHash = promauto.NewGaugeVec(prometheus.GaugeOpts{ + peerGitHash = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ Namespace: "app", Subsystem: "peerinfo", Name: "git_commit", diff --git a/app/peerinfo/peerinfo.go b/app/peerinfo/peerinfo.go index 4e1fda820..fa573988b 100644 --- a/app/peerinfo/peerinfo.go +++ b/app/peerinfo/peerinfo.go @@ -6,7 +6,6 @@ import ( "bytes" "context" "fmt" - "sync" "testing" "time" @@ -241,13 +240,6 @@ func supportedPeerVersion(peerVersion string, supported []version.SemVer) error // newMetricsSubmitter returns a prometheus metric submitter. func newMetricsSubmitter() metricSubmitter { - var ( - mu sync.Mutex - // TODO(corver): Refactor to use ResetGauge. - prevVersions = make(map[string]string) - prevGitHashes = make(map[string]string) - ) - return func(peerID peer.ID, clockOffset time.Duration, version string, gitHash string, startTime time.Time, ) { @@ -274,20 +266,9 @@ func newMetricsSubmitter() metricSubmitter { } // TODO(corver): Validate version and githash with regex + peerVersion.ResetMatching(peerName) peerVersion.WithLabelValues(peerName, version).Set(1) + peerGitHash.ResetMatching(peerName) peerGitHash.WithLabelValues(peerName, gitHash).Set(1) - - // Clear previous metrics if changed - mu.Lock() - defer mu.Unlock() - - if prev, ok := prevVersions[peerName]; ok && version != prev { - peerVersion.WithLabelValues(peerName, prev).Set(0) - } - if prev, ok := prevGitHashes[peerName]; ok && gitHash != prev { - peerGitHash.WithLabelValues(peerName, prev).Set(0) - } - prevVersions[peerName] = version - prevGitHashes[peerName] = gitHash } } diff --git a/app/promauto/resetgauge.go b/app/promauto/resetgauge.go index e9010b66d..c405bb1f7 100644 --- a/app/promauto/resetgauge.go +++ b/app/promauto/resetgauge.go @@ -43,8 +43,29 @@ func (g *ResetGaugeVec) WithLabelValues(lvs ...string) prometheus.Gauge { return g.inner.WithLabelValues(lvs...) } -// Reset deletes all previously set labels. -func (g *ResetGaugeVec) Reset() { +// ResetMatching deletes all previously set labels that match all the given label values. +func (g *ResetGaugeVec) ResetMatching(lvs ...string) { + g.mu.Lock() + defer g.mu.Unlock() + + for label := range g.labels { + foundAll := true + for _, check := range lvs { + if !strings.Contains(label, check) { + foundAll = false + break + } + } + + if foundAll { + g.inner.DeleteLabelValues(strings.Split(label, separator)...) + delete(g.labels, label) + } + } +} + +// ResetAll deletes all previously set labels. +func (g *ResetGaugeVec) ResetAll() { g.mu.Lock() defer g.mu.Unlock() diff --git a/app/promauto/resetgauge_test.go b/app/promauto/resetgauge_test.go index 217c0021b..9ad791759 100644 --- a/app/promauto/resetgauge_test.go +++ b/app/promauto/resetgauge_test.go @@ -16,22 +16,41 @@ const resetTest = "reset_test" var testResetGauge = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ Name: resetTest, Help: "", -}, []string{"label"}) +}, []string{"label0", "label1"}) func TestResetGaugeVec(t *testing.T) { registry, err := promauto.NewRegistry(nil) require.NoError(t, err) - testResetGauge.WithLabelValues("1").Set(1) + testResetGauge.WithLabelValues("1", "a").Set(0) assertVecLen(t, registry, resetTest, 1) - testResetGauge.WithLabelValues("2").Set(2) + // Same labels, should not increase length + testResetGauge.WithLabelValues("1", "a").Set(1) + assertVecLen(t, registry, resetTest, 1) + + testResetGauge.WithLabelValues("2", "b").Set(2) assertVecLen(t, registry, resetTest, 2) - testResetGauge.Reset() + testResetGauge.ResetAll() assertVecLen(t, registry, resetTest, 0) - testResetGauge.WithLabelValues("3").Set(3) + testResetGauge.WithLabelValues("3", "c").Set(3) + assertVecLen(t, registry, resetTest, 1) + + testResetGauge.WithLabelValues("3", "d").Set(3) + assertVecLen(t, registry, resetTest, 2) + + testResetGauge.WithLabelValues("3", "e").Set(3) + assertVecLen(t, registry, resetTest, 3) + + testResetGauge.WithLabelValues("4", "z").Set(4) + assertVecLen(t, registry, resetTest, 4) + + testResetGauge.ResetMatching("3", "c") + assertVecLen(t, registry, resetTest, 3) + + testResetGauge.ResetMatching("3") assertVecLen(t, registry, resetTest, 1) } diff --git a/core/scheduler/metrics.go b/core/scheduler/metrics.go index 12e36c825..bfc833baf 100644 --- a/core/scheduler/metrics.go +++ b/core/scheduler/metrics.go @@ -49,7 +49,7 @@ var ( Help: "Total balance of a validator by public key", }, []string{"pubkey_full", "pubkey"}) - statusGauge = promauto.NewGaugeVec(prometheus.GaugeOpts{ + statusGauge = promauto.NewResetGaugeVec(prometheus.GaugeOpts{ Namespace: "core", Subsystem: "scheduler", Name: "validator_status", @@ -77,16 +77,10 @@ func instrumentDuty(duty core.Duty, defSet core.DutyDefinitionSet) { // newMetricSubmitter returns a function that sets validator balance and status metric. func newMetricSubmitter() func(pubkey core.PubKey, totalBal eth2p0.Gwei, status string) { - // TODO(corver): Refactor to use ResetGauge. - prevStatus := make(map[core.PubKey]string) - return func(pubkey core.PubKey, totalBal eth2p0.Gwei, status string) { balanceGauge.WithLabelValues(string(pubkey), pubkey.String()).Set(float64(totalBal)) - statusGauge.WithLabelValues(string(pubkey), pubkey.String(), status).Set(1) - if prev, ok := prevStatus[pubkey]; ok && prev != status { // Validator status changed - statusGauge.WithLabelValues(string(pubkey), pubkey.String(), prev).Set(0) - } - prevStatus[pubkey] = status + statusGauge.ResetMatching(string(pubkey), pubkey.String()) + statusGauge.WithLabelValues(string(pubkey), pubkey.String(), status).Set(1) } } diff --git a/p2p/p2p.go b/p2p/p2p.go index 6417d377a..f4d52795c 100644 --- a/p2p/p2p.go +++ b/p2p/p2p.go @@ -307,7 +307,6 @@ func RegisterConnectionLogger(ctx context.Context, tcpNode host.Host, peerIDs [] return case <-ticker.C: // Instrument connection and stream counts. - peerStreamGauge.Reset() // Reset stream gauge to clear previously set protocols. counts := make(map[connKey]int) streams := make(map[streamKey]int) @@ -328,6 +327,8 @@ func RegisterConnectionLogger(ctx context.Context, tcpNode host.Host, peerIDs [] streams[sKey]++ } } + + peerStreamGauge.ResetAll() // Reset stream gauge to clear previously set protocols. for _, pID := range peerIDs { for _, typ := range []string{addrTypeRelay, addrTypeDirect} { cKey := connKey{PeerName: PeerName(pID), Type: typ} From ab08ed0c3d9b968093414dd06108e07e04c93f80 Mon Sep 17 00:00:00 2001 From: corverroos Date: Sun, 30 Jul 2023 11:11:56 +0200 Subject: [PATCH 2/4] app/promauto: support partial resets --- app/promauto/resetgauge.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/promauto/resetgauge.go b/app/promauto/resetgauge.go index c405bb1f7..eec986e1b 100644 --- a/app/promauto/resetgauge.go +++ b/app/promauto/resetgauge.go @@ -44,6 +44,7 @@ func (g *ResetGaugeVec) WithLabelValues(lvs ...string) prometheus.Gauge { } // ResetMatching deletes all previously set labels that match all the given label values. +// An empty slice will delete all previously set labels. func (g *ResetGaugeVec) ResetMatching(lvs ...string) { g.mu.Lock() defer g.mu.Unlock() @@ -66,12 +67,5 @@ func (g *ResetGaugeVec) ResetMatching(lvs ...string) { // ResetAll deletes all previously set labels. func (g *ResetGaugeVec) ResetAll() { - g.mu.Lock() - defer g.mu.Unlock() - - for lv := range g.labels { - g.inner.DeleteLabelValues(strings.Split(lv, separator)...) - } - - g.labels = make(map[string]bool) + g.ResetMatching() } From 7797673b4d021f86009318d19b9f01f255646c46 Mon Sep 17 00:00:00 2001 From: corverroos Date: Sun, 30 Jul 2023 11:13:50 +0200 Subject: [PATCH 3/4] cleanup --- app/monitoringapi.go | 2 +- app/peerinfo/peerinfo.go | 4 ++-- app/promauto/resetgauge.go | 9 ++------- app/promauto/resetgauge_test.go | 6 +++--- core/scheduler/metrics.go | 2 +- p2p/p2p.go | 2 +- 6 files changed, 10 insertions(+), 15 deletions(-) diff --git a/app/monitoringapi.go b/app/monitoringapi.go index c156bd2b7..5e4fb89c2 100644 --- a/app/monitoringapi.go +++ b/app/monitoringapi.go @@ -224,7 +224,7 @@ func beaconNodeVersionMetric(ctx context.Context, eth2Cl eth2wrap.Client, clock return } - beaconNodeVersionGauge.ResetAll() + beaconNodeVersionGauge.Reset() beaconNodeVersionGauge.WithLabelValues(version).Set(1) } diff --git a/app/peerinfo/peerinfo.go b/app/peerinfo/peerinfo.go index fa573988b..58fb9170a 100644 --- a/app/peerinfo/peerinfo.go +++ b/app/peerinfo/peerinfo.go @@ -266,9 +266,9 @@ func newMetricsSubmitter() metricSubmitter { } // TODO(corver): Validate version and githash with regex - peerVersion.ResetMatching(peerName) + peerVersion.Reset(peerName) peerVersion.WithLabelValues(peerName, version).Set(1) - peerGitHash.ResetMatching(peerName) + peerGitHash.Reset(peerName) peerGitHash.WithLabelValues(peerName, gitHash).Set(1) } } diff --git a/app/promauto/resetgauge.go b/app/promauto/resetgauge.go index eec986e1b..910431793 100644 --- a/app/promauto/resetgauge.go +++ b/app/promauto/resetgauge.go @@ -43,9 +43,9 @@ func (g *ResetGaugeVec) WithLabelValues(lvs ...string) prometheus.Gauge { return g.inner.WithLabelValues(lvs...) } -// ResetMatching deletes all previously set labels that match all the given label values. +// Reset deletes all previously set labels that match all the given label values. // An empty slice will delete all previously set labels. -func (g *ResetGaugeVec) ResetMatching(lvs ...string) { +func (g *ResetGaugeVec) Reset(lvs ...string) { g.mu.Lock() defer g.mu.Unlock() @@ -64,8 +64,3 @@ func (g *ResetGaugeVec) ResetMatching(lvs ...string) { } } } - -// ResetAll deletes all previously set labels. -func (g *ResetGaugeVec) ResetAll() { - g.ResetMatching() -} diff --git a/app/promauto/resetgauge_test.go b/app/promauto/resetgauge_test.go index 9ad791759..90694be32 100644 --- a/app/promauto/resetgauge_test.go +++ b/app/promauto/resetgauge_test.go @@ -32,7 +32,7 @@ func TestResetGaugeVec(t *testing.T) { testResetGauge.WithLabelValues("2", "b").Set(2) assertVecLen(t, registry, resetTest, 2) - testResetGauge.ResetAll() + testResetGauge.Reset() assertVecLen(t, registry, resetTest, 0) testResetGauge.WithLabelValues("3", "c").Set(3) @@ -47,10 +47,10 @@ func TestResetGaugeVec(t *testing.T) { testResetGauge.WithLabelValues("4", "z").Set(4) assertVecLen(t, registry, resetTest, 4) - testResetGauge.ResetMatching("3", "c") + testResetGauge.Reset("3", "c") assertVecLen(t, registry, resetTest, 3) - testResetGauge.ResetMatching("3") + testResetGauge.Reset("3") assertVecLen(t, registry, resetTest, 1) } diff --git a/core/scheduler/metrics.go b/core/scheduler/metrics.go index bfc833baf..5745fde81 100644 --- a/core/scheduler/metrics.go +++ b/core/scheduler/metrics.go @@ -80,7 +80,7 @@ func newMetricSubmitter() func(pubkey core.PubKey, totalBal eth2p0.Gwei, status return func(pubkey core.PubKey, totalBal eth2p0.Gwei, status string) { balanceGauge.WithLabelValues(string(pubkey), pubkey.String()).Set(float64(totalBal)) - statusGauge.ResetMatching(string(pubkey), pubkey.String()) + statusGauge.Reset(string(pubkey), pubkey.String()) statusGauge.WithLabelValues(string(pubkey), pubkey.String(), status).Set(1) } } diff --git a/p2p/p2p.go b/p2p/p2p.go index f4d52795c..f380d98e5 100644 --- a/p2p/p2p.go +++ b/p2p/p2p.go @@ -328,7 +328,7 @@ func RegisterConnectionLogger(ctx context.Context, tcpNode host.Host, peerIDs [] } } - peerStreamGauge.ResetAll() // Reset stream gauge to clear previously set protocols. + peerStreamGauge.Reset() // Reset stream gauge to clear previously set protocols. for _, pID := range peerIDs { for _, typ := range []string{addrTypeRelay, addrTypeDirect} { cKey := connKey{PeerName: PeerName(pID), Type: typ} From 959ebb6cc2d6843d004f1f071174d01849618130 Mon Sep 17 00:00:00 2001 From: corverroos Date: Sun, 30 Jul 2023 11:16:53 +0200 Subject: [PATCH 4/4] cleanup --- app/promauto/resetgauge.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/promauto/resetgauge.go b/app/promauto/resetgauge.go index 910431793..ee0ad9e9f 100644 --- a/app/promauto/resetgauge.go +++ b/app/promauto/resetgauge.go @@ -50,17 +50,19 @@ func (g *ResetGaugeVec) Reset(lvs ...string) { defer g.mu.Unlock() for label := range g.labels { - foundAll := true + match := true for _, check := range lvs { if !strings.Contains(label, check) { - foundAll = false + match = false break } } - if foundAll { - g.inner.DeleteLabelValues(strings.Split(label, separator)...) - delete(g.labels, label) + if !match { + continue } + + g.inner.DeleteLabelValues(strings.Split(label, separator)...) + delete(g.labels, label) } }