From 5e625bbd40e779cee6a5c95bbea9a46a7246b58a Mon Sep 17 00:00:00 2001 From: Cooper Ry Lees Date: Mon, 6 Apr 2026 20:18:08 +0000 Subject: [PATCH 1/4] Prune stale Prometheus metrics for defunct peer pod IPs on teardown After a DaemonSet rolling update, goldpinger retained response-time histogram and error counter series for old pod IPs that no longer exist. These stale single-sample series skewed P95/P99 latency calculations and made transient rollout errors appear permanent. (Fixes #167) The existing destroyPingers path only cleaned UDP-specific per-peer metrics (and only when UDP was enabled). This adds: - DeletePeerMetrics(): removes goldpinger_peers_response_time_s histogram label sets for destroyed peers, called unconditionally on pinger teardown - goldpinger_udp_errors_total cleanup in DeletePeerUDPMetrics(), which was previously missed Testing: - TestDeletePeerMetrics_CleansResponseTimeHistogram: verifies the response-time histogram label set is removed after DeletePeerMetrics() - TestDeletePeerMetrics_LeavesOtherPeersIntact: verifies pruning one peer does not affect another peer's metric series - TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics: extended to also verify goldpinger_udp_errors_total cleanup - All 11 tests pass (go test ./pkg/goldpinger/ -v) Validated on a 6-node IPv6 kubeadm cluster by upgrading goldpinger with a rolling update and confirming /metrics only contains current pod IPs after the rollout completes. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Cooper Ry Lees --- pkg/goldpinger/stats.go | 8 ++++ pkg/goldpinger/stats_test.go | 77 +++++++++++++++++++++++++++++++++++- pkg/goldpinger/updater.go | 4 +- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/pkg/goldpinger/stats.go b/pkg/goldpinger/stats.go index 07d0e43..4085bc8 100644 --- a/pkg/goldpinger/stats.go +++ b/pkg/goldpinger/stats.go @@ -300,6 +300,13 @@ func SetPeerHopCount(hostIP, podIP string, hopCount int32) { ).Set(float64(hopCount)) } +// DeletePeerMetrics removes stale metric labels for a destroyed peer. +// This covers the HTTP ping response-time histogram which is always active. +// Must be called unconditionally when a peer is removed. +func DeletePeerMetrics(hostIP, podIP string) { + goldpingerResponseTimePeersHistogram.DeleteLabelValues(GoldpingerConfig.Hostname, "ping", hostIP, podIP) +} + // DeletePeerUDPMetrics removes stale UDP metric labels for a destroyed peer. // This must be kept in sync with all per-peer UDP metrics to avoid stale // label sets lingering in /metrics after a pod rolls. @@ -309,6 +316,7 @@ func DeletePeerUDPMetrics(hostIP, podIP string) { goldpingerPeersUDPRtt.DeleteLabelValues(GoldpingerConfig.Hostname, hostIP, podIP) goldpingerUDPDuplicatesCounter.DeleteLabelValues(GoldpingerConfig.Hostname, hostIP, podIP) goldpingerUDPOutOfOrderCounter.DeleteLabelValues(GoldpingerConfig.Hostname, hostIP, podIP) + goldpingerUDPErrorsCounter.DeleteLabelValues(GoldpingerConfig.Hostname, pickPodHostIP(podIP, hostIP)) } // ObservePeerUDPRtt records a UDP RTT observation in seconds diff --git a/pkg/goldpinger/stats_test.go b/pkg/goldpinger/stats_test.go index 8b56b91..e6a5a7c 100644 --- a/pkg/goldpinger/stats_test.go +++ b/pkg/goldpinger/stats_test.go @@ -7,6 +7,74 @@ import ( dto "github.com/prometheus/client_model/go" ) +// TestDeletePeerMetrics_CleansResponseTimeHistogram verifies that +// DeletePeerMetrics removes the response-time histogram label set for +// a destroyed peer. This prevents stale pod IPs from lingering in +// /metrics after rolling updates (see #167). +func TestDeletePeerMetrics_CleansResponseTimeHistogram(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() + + hostIP := "10.0.0.1" + podIP := "10.0.0.2" + + // Simulate a ping observation (call_type="ping") + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", hostIP, podIP, + ).Observe(0.005) + + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + } + + DeletePeerMetrics(hostIP, podIP) + + if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 0 { + t.Errorf("response time histogram still has %d label set(s) after DeletePeerMetrics", n) + } +} + +// TestDeletePeerMetrics_LeavesOtherPeersIntact verifies that pruning +// metrics for one peer does not affect a different peer's label set. +func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() + + // Peer A + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", "10.0.0.1", "10.0.0.2", + ).Observe(0.005) + SetPeerLossPct("10.0.0.1", "10.0.0.2", 0) + + // Peer B + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", + ).Observe(0.010) + SetPeerLossPct("10.0.0.3", "10.0.0.4", 1.5) + + // Delete peer A only + DeletePeerMetrics("10.0.0.1", "10.0.0.2") + DeletePeerUDPMetrics("10.0.0.1", "10.0.0.2") + + // Peer B should survive in both metrics + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Error("response time histogram lost all label sets — peer B should still exist") + } + if countMetrics(goldpingerPeersLossPct) == 0 { + t.Error("loss pct gauge lost all label sets — peer B should still exist") + } + + // Clean up peer B so it doesn't leak into other tests + goldpingerResponseTimePeersHistogram.DeleteLabelValues( + GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", + ) + goldpingerPeersLossPct.DeleteLabelValues( + GoldpingerConfig.Hostname, "10.0.0.3", "10.0.0.4", + ) +} + // TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics verifies that // DeletePeerUDPMetrics removes label sets from every per-peer UDP metric. // If a new per-peer metric is added but not cleaned up in @@ -14,8 +82,13 @@ import ( func TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics(t *testing.T) { // Save and restore hostname since we set it for the test origHostname := GoldpingerConfig.Hostname + origUseHostIP := GoldpingerConfig.UseHostIP GoldpingerConfig.Hostname = "test-instance" - defer func() { GoldpingerConfig.Hostname = origHostname }() + GoldpingerConfig.UseHostIP = false + defer func() { + GoldpingerConfig.Hostname = origHostname + GoldpingerConfig.UseHostIP = origUseHostIP + }() hostIP := "10.0.0.1" podIP := "10.0.0.2" @@ -26,6 +99,7 @@ func TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics(t *testing.T) { ObservePeerUDPRtt(hostIP, podIP, 0.001) CountUDPDuplicates(hostIP, podIP, 1) CountUDPOutOfOrder(hostIP, podIP, 1) + CountUDPError(podIP) // UseHostIP=false so target=podIP // Verify they exist before cleanup perPeerCollectors := map[string]prometheus.Collector{ @@ -34,6 +108,7 @@ func TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics(t *testing.T) { "goldpinger_peers_udp_rtt_s": goldpingerPeersUDPRtt, "goldpinger_udp_duplicates_total": goldpingerUDPDuplicatesCounter, "goldpinger_udp_out_of_order_total": goldpingerUDPOutOfOrderCounter, + "goldpinger_udp_errors_total": goldpingerUDPErrorsCounter, } for name, collector := range perPeerCollectors { diff --git a/pkg/goldpinger/updater.go b/pkg/goldpinger/updater.go index 2205da2..cf73457 100644 --- a/pkg/goldpinger/updater.go +++ b/pkg/goldpinger/updater.go @@ -137,7 +137,9 @@ func destroyPingers(pingers map[string]*Pinger, deletedPods map[string]*Goldping // Close the channel to stop pinging close(pinger.stopChan) - // Clean up stale UDP metric labels for this peer + // Clean up stale metric labels for this peer so defunct pod IPs + // don't linger in /metrics after rolling updates (see #167) + DeletePeerMetrics(pod.HostIP, pod.PodIP) if GoldpingerConfig.UDPEnabled { DeletePeerUDPMetrics(pod.HostIP, pod.PodIP) } From e08e21f15c541fcc457cd897119ea05fde0961a5 Mon Sep 17 00:00:00 2001 From: Cooper Ry Lees Date: Mon, 6 Apr 2026 20:21:33 +0000 Subject: [PATCH 2/4] Use table-driven subtests with IPv4, IPv6, and mixed address families Converts the three DeletePeer* tests to table-driven subtests covering IPv4, IPv6 (RFC 3849 2001:db8::/32 documentation prefix), and mixed address-family scenarios. This matches the IPv6-only cluster where the stale metrics bug was originally observed (#167). Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Cooper Ry Lees --- pkg/goldpinger/stats_test.go | 232 +++++++++++++++++++++-------------- 1 file changed, 141 insertions(+), 91 deletions(-) diff --git a/pkg/goldpinger/stats_test.go b/pkg/goldpinger/stats_test.go index e6a5a7c..2bf359f 100644 --- a/pkg/goldpinger/stats_test.go +++ b/pkg/goldpinger/stats_test.go @@ -12,67 +12,108 @@ import ( // a destroyed peer. This prevents stale pod IPs from lingering in // /metrics after rolling updates (see #167). func TestDeletePeerMetrics_CleansResponseTimeHistogram(t *testing.T) { - origHostname := GoldpingerConfig.Hostname - GoldpingerConfig.Hostname = "test-instance" - defer func() { GoldpingerConfig.Hostname = origHostname }() - - hostIP := "10.0.0.1" - podIP := "10.0.0.2" - - // Simulate a ping observation (call_type="ping") - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", hostIP, podIP, - ).Observe(0.005) - - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + tests := []struct { + name string + hostIP string + podIP string + }{ + {"IPv4", "10.0.0.1", "10.0.0.2"}, + {"IPv6", "2001:db8::1", "2001:db8:1::2"}, } - DeletePeerMetrics(hostIP, podIP) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() - if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 0 { - t.Errorf("response time histogram still has %d label set(s) after DeletePeerMetrics", n) + // Simulate a ping observation (call_type="ping") + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.hostIP, tt.podIP, + ).Observe(0.005) + + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + } + + DeletePeerMetrics(tt.hostIP, tt.podIP) + + if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 0 { + t.Errorf("response time histogram still has %d label set(s) after DeletePeerMetrics", n) + } + }) } } // TestDeletePeerMetrics_LeavesOtherPeersIntact verifies that pruning // metrics for one peer does not affect a different peer's label set. func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { - origHostname := GoldpingerConfig.Hostname - GoldpingerConfig.Hostname = "test-instance" - defer func() { GoldpingerConfig.Hostname = origHostname }() - - // Peer A - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.1", "10.0.0.2", - ).Observe(0.005) - SetPeerLossPct("10.0.0.1", "10.0.0.2", 0) - - // Peer B - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", - ).Observe(0.010) - SetPeerLossPct("10.0.0.3", "10.0.0.4", 1.5) - - // Delete peer A only - DeletePeerMetrics("10.0.0.1", "10.0.0.2") - DeletePeerUDPMetrics("10.0.0.1", "10.0.0.2") - - // Peer B should survive in both metrics - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Error("response time histogram lost all label sets — peer B should still exist") - } - if countMetrics(goldpingerPeersLossPct) == 0 { - t.Error("loss pct gauge lost all label sets — peer B should still exist") + tests := []struct { + name string + peerA [2]string // {hostIP, podIP} + peerB [2]string + }{ + { + "IPv4", + [2]string{"10.0.0.1", "10.0.0.2"}, + [2]string{"10.0.0.3", "10.0.0.4"}, + }, + { + "IPv6", + [2]string{"2001:db8::1", "2001:db8:1::2"}, + [2]string{"2001:db8::3", "2001:db8:2::4"}, + }, + { + "MixedV4DeleteV6Survives", + [2]string{"10.0.0.1", "10.0.0.2"}, + [2]string{"2001:db8::3", "2001:db8:2::4"}, + }, + { + "MixedV6DeleteV4Survives", + [2]string{"2001:db8::1", "2001:db8:1::2"}, + [2]string{"10.0.0.3", "10.0.0.4"}, + }, } - // Clean up peer B so it doesn't leak into other tests - goldpingerResponseTimePeersHistogram.DeleteLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", - ) - goldpingerPeersLossPct.DeleteLabelValues( - GoldpingerConfig.Hostname, "10.0.0.3", "10.0.0.4", - ) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() + + // Peer A + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerA[0], tt.peerA[1], + ).Observe(0.005) + SetPeerLossPct(tt.peerA[0], tt.peerA[1], 0) + + // Peer B + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], + ).Observe(0.010) + SetPeerLossPct(tt.peerB[0], tt.peerB[1], 1.5) + + // Delete peer A only + DeletePeerMetrics(tt.peerA[0], tt.peerA[1]) + DeletePeerUDPMetrics(tt.peerA[0], tt.peerA[1]) + + // Peer B should survive in both metrics + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Error("response time histogram lost all label sets — peer B should still exist") + } + if countMetrics(goldpingerPeersLossPct) == 0 { + t.Error("loss pct gauge lost all label sets — peer B should still exist") + } + + // Clean up peer B so it doesn't leak into other tests + goldpingerResponseTimePeersHistogram.DeleteLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], + ) + goldpingerPeersLossPct.DeleteLabelValues( + GoldpingerConfig.Hostname, tt.peerB[0], tt.peerB[1], + ) + }) + } } // TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics verifies that @@ -80,51 +121,60 @@ func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { // If a new per-peer metric is added but not cleaned up in // DeletePeerUDPMetrics, this test will fail. func TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics(t *testing.T) { - // Save and restore hostname since we set it for the test - origHostname := GoldpingerConfig.Hostname - origUseHostIP := GoldpingerConfig.UseHostIP - GoldpingerConfig.Hostname = "test-instance" - GoldpingerConfig.UseHostIP = false - defer func() { - GoldpingerConfig.Hostname = origHostname - GoldpingerConfig.UseHostIP = origUseHostIP - }() - - hostIP := "10.0.0.1" - podIP := "10.0.0.2" - - // Populate all per-peer UDP metrics so they have label values - SetPeerLossPct(hostIP, podIP, 5.0) - SetPeerHopCount(hostIP, podIP, 2) - ObservePeerUDPRtt(hostIP, podIP, 0.001) - CountUDPDuplicates(hostIP, podIP, 1) - CountUDPOutOfOrder(hostIP, podIP, 1) - CountUDPError(podIP) // UseHostIP=false so target=podIP - - // Verify they exist before cleanup - perPeerCollectors := map[string]prometheus.Collector{ - "goldpinger_peers_loss_pct": goldpingerPeersLossPct, - "goldpinger_peers_hop_count": goldpingerPeersHopCount, - "goldpinger_peers_udp_rtt_s": goldpingerPeersUDPRtt, - "goldpinger_udp_duplicates_total": goldpingerUDPDuplicatesCounter, - "goldpinger_udp_out_of_order_total": goldpingerUDPOutOfOrderCounter, - "goldpinger_udp_errors_total": goldpingerUDPErrorsCounter, + tests := []struct { + name string + hostIP string + podIP string + }{ + {"IPv4", "10.0.0.1", "10.0.0.2"}, + {"IPv6", "2001:db8::1", "2001:db8:1::2"}, } - for name, collector := range perPeerCollectors { - if countMetrics(collector) == 0 { - t.Fatalf("metric %s has no label values before cleanup — test setup is broken", name) - } - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + origUseHostIP := GoldpingerConfig.UseHostIP + GoldpingerConfig.Hostname = "test-instance" + GoldpingerConfig.UseHostIP = false + defer func() { + GoldpingerConfig.Hostname = origHostname + GoldpingerConfig.UseHostIP = origUseHostIP + }() - // Run cleanup - DeletePeerUDPMetrics(hostIP, podIP) + // Populate all per-peer UDP metrics so they have label values + SetPeerLossPct(tt.hostIP, tt.podIP, 5.0) + SetPeerHopCount(tt.hostIP, tt.podIP, 2) + ObservePeerUDPRtt(tt.hostIP, tt.podIP, 0.001) + CountUDPDuplicates(tt.hostIP, tt.podIP, 1) + CountUDPOutOfOrder(tt.hostIP, tt.podIP, 1) + CountUDPError(tt.podIP) // UseHostIP=false so target=podIP - // Verify all per-peer metrics are cleaned up - for name, collector := range perPeerCollectors { - if n := countMetrics(collector); n != 0 { - t.Errorf("metric %s still has %d label set(s) after DeletePeerUDPMetrics — add it to the cleanup function", name, n) - } + // Verify they exist before cleanup + perPeerCollectors := map[string]prometheus.Collector{ + "goldpinger_peers_loss_pct": goldpingerPeersLossPct, + "goldpinger_peers_hop_count": goldpingerPeersHopCount, + "goldpinger_peers_udp_rtt_s": goldpingerPeersUDPRtt, + "goldpinger_udp_duplicates_total": goldpingerUDPDuplicatesCounter, + "goldpinger_udp_out_of_order_total": goldpingerUDPOutOfOrderCounter, + "goldpinger_udp_errors_total": goldpingerUDPErrorsCounter, + } + + for name, collector := range perPeerCollectors { + if countMetrics(collector) == 0 { + t.Fatalf("metric %s has no label values before cleanup — test setup is broken", name) + } + } + + // Run cleanup + DeletePeerUDPMetrics(tt.hostIP, tt.podIP) + + // Verify all per-peer metrics are cleaned up + for name, collector := range perPeerCollectors { + if n := countMetrics(collector); n != 0 { + t.Errorf("metric %s still has %d label set(s) after DeletePeerUDPMetrics — add it to the cleanup function", name, n) + } + } + }) } } From 4308eb1fa59754f1353d2bd5cf41637c402eaff1 Mon Sep 17 00:00:00 2001 From: Cooper Ry Lees Date: Wed, 22 Apr 2026 13:51:55 +0000 Subject: [PATCH 3/4] Also prune response-time histogram "check" call_type on peer removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #169 review: DeletePeerMetrics only removed the "ping" call_type label set, leaving stale "check" samples from CheckAllPods in /metrics after a peer rolls. goldpingerResponseTimePeersHistogram is observed at two sites — Pinger (pinger.go:56, "ping") and CheckAllPods via GetLabeledPeersCallsTimer (client.go:209, "check") — so both must be pruned. Missed on the first pass because I traced the Pinger observation (the hot path) and did not separately check for the "check" label value emitted from the /check_all handler. Tests now seed both call_types before teardown so either gap regresses. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Cooper Ry Lees --- pkg/goldpinger/stats.go | 6 ++++-- pkg/goldpinger/stats_test.go | 28 ++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/pkg/goldpinger/stats.go b/pkg/goldpinger/stats.go index 4085bc8..4c7fc65 100644 --- a/pkg/goldpinger/stats.go +++ b/pkg/goldpinger/stats.go @@ -301,10 +301,12 @@ func SetPeerHopCount(hostIP, podIP string, hopCount int32) { } // DeletePeerMetrics removes stale metric labels for a destroyed peer. -// This covers the HTTP ping response-time histogram which is always active. -// Must be called unconditionally when a peer is removed. +// The response-time histogram is observed with two call_type values: +// "ping" (continuous from Pinger) and "check" (from CheckAllPods), so both +// must be pruned. Must be called unconditionally when a peer is removed. func DeletePeerMetrics(hostIP, podIP string) { goldpingerResponseTimePeersHistogram.DeleteLabelValues(GoldpingerConfig.Hostname, "ping", hostIP, podIP) + goldpingerResponseTimePeersHistogram.DeleteLabelValues(GoldpingerConfig.Hostname, "check", hostIP, podIP) } // DeletePeerUDPMetrics removes stale UDP metric labels for a destroyed peer. diff --git a/pkg/goldpinger/stats_test.go b/pkg/goldpinger/stats_test.go index 2bf359f..7a6890e 100644 --- a/pkg/goldpinger/stats_test.go +++ b/pkg/goldpinger/stats_test.go @@ -27,13 +27,16 @@ func TestDeletePeerMetrics_CleansResponseTimeHistogram(t *testing.T) { GoldpingerConfig.Hostname = "test-instance" defer func() { GoldpingerConfig.Hostname = origHostname }() - // Simulate a ping observation (call_type="ping") + // Simulate observations for both call_type values the histogram uses goldpingerResponseTimePeersHistogram.WithLabelValues( GoldpingerConfig.Hostname, "ping", tt.hostIP, tt.podIP, ).Observe(0.005) + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "check", tt.hostIP, tt.podIP, + ).Observe(0.010) - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 2 { + t.Fatalf("response time histogram has %d label sets before cleanup, want 2 — test setup is broken", n) } DeletePeerMetrics(tt.hostIP, tt.podIP) @@ -81,25 +84,31 @@ func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { GoldpingerConfig.Hostname = "test-instance" defer func() { GoldpingerConfig.Hostname = origHostname }() - // Peer A + // Peer A — observe both call types goldpingerResponseTimePeersHistogram.WithLabelValues( GoldpingerConfig.Hostname, "ping", tt.peerA[0], tt.peerA[1], ).Observe(0.005) + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "check", tt.peerA[0], tt.peerA[1], + ).Observe(0.006) SetPeerLossPct(tt.peerA[0], tt.peerA[1], 0) - // Peer B + // Peer B — observe both call types goldpingerResponseTimePeersHistogram.WithLabelValues( GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], ).Observe(0.010) + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "check", tt.peerB[0], tt.peerB[1], + ).Observe(0.011) SetPeerLossPct(tt.peerB[0], tt.peerB[1], 1.5) // Delete peer A only DeletePeerMetrics(tt.peerA[0], tt.peerA[1]) DeletePeerUDPMetrics(tt.peerA[0], tt.peerA[1]) - // Peer B should survive in both metrics - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Error("response time histogram lost all label sets — peer B should still exist") + // Peer B's ping and check histogram entries should both survive + if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 2 { + t.Errorf("response time histogram has %d label set(s), want 2 for peer B (ping+check)", n) } if countMetrics(goldpingerPeersLossPct) == 0 { t.Error("loss pct gauge lost all label sets — peer B should still exist") @@ -109,6 +118,9 @@ func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { goldpingerResponseTimePeersHistogram.DeleteLabelValues( GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], ) + goldpingerResponseTimePeersHistogram.DeleteLabelValues( + GoldpingerConfig.Hostname, "check", tt.peerB[0], tt.peerB[1], + ) goldpingerPeersLossPct.DeleteLabelValues( GoldpingerConfig.Hostname, tt.peerB[0], tt.peerB[1], ) From 4036ef6a9ac2c5addc7665aa030352099c3df9c1 Mon Sep 17 00:00:00 2001 From: Sachin Kamboj Date: Thu, 23 Apr 2026 11:18:17 -0400 Subject: [PATCH 4/4] chore(version): bump to v3.11.2 Signed-off-by: Sachin Kamboj --- Makefile | 2 +- charts/goldpinger/Chart.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 444db21..30322b4 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ name ?= goldpinger -version ?= v3.11.1 +version ?= v3.11.2 bin ?= goldpinger pkg ?= "github.com/bloomberg/goldpinger" tag = $(name):$(version) diff --git a/charts/goldpinger/Chart.yaml b/charts/goldpinger/Chart.yaml index ac356cf..8a88314 100644 --- a/charts/goldpinger/Chart.yaml +++ b/charts/goldpinger/Chart.yaml @@ -1,7 +1,7 @@ apiVersion: v1 name: goldpinger -appVersion: "3.11.1" -version: 1.1.1 +appVersion: "3.11.2" +version: 1.1.2 description: Goldpinger is a tool to help debug, troubleshoot and visualize network connectivity and slowness issues. home: https://github.com/bloomberg/goldpinger sources: