fix(discovery): strengthen metrics_test presence check + errorlint fix
cognitive committed
Apr 30, 2026 at 04:22 UTC
9befcd9472b7ff7d5d99de9fdeb99debe819cafd
1 file changed
+79
-20
portal/discovery/metrics_test.go
+79
-20
@@ -1,6 +1,7 @@
1
package discovery
2
3
import (
4
+ "errors"
5
"fmt"
6
"testing"
7
"time"
@@ -26,35 +27,93 @@ func metricFamilyByName(t *testing.T, name string) *dto.MetricFamily {
27
return nil
28
}
29
30
+// assertRegisteredWithName verifies that collector c is already registered on
31
+// prometheus.DefaultRegisterer by attempting to re-register it and expecting
32
+// AlreadyRegisteredError. It then confirms that the previously-registered
33
+// collector's first described metric name equals wantName.
34
+//
35
+// This is the only approach that simultaneously proves (a) the collector is on
36
+// the default registry and (b) the registered metric has the expected name, for
37
+// both Vec and non-Vec collectors with no prior observations.
38
+func assertRegisteredWithName(t *testing.T, c prometheus.Collector, wantName string) {
39
+ t.Helper()
40
+ err := prometheus.DefaultRegisterer.Register(c)
41
+ if err == nil {
42
+ // Re-registration succeeded — the collector was NOT on the default registry.
43
+ // Undo the registration so the rest of the test suite is not affected.
44
+ prometheus.DefaultRegisterer.Unregister(c)
45
+ t.Fatalf("metric %q: collector was not registered on DefaultRegisterer before the test", wantName)
46
+ }
47
+ var are prometheus.AlreadyRegisteredError
48
+ if !errors.As(err, &are) {
49
+ t.Fatalf("metric %q: unexpected registration error: %v", wantName, err)
50
+ }
51
+ // are.ExistingCollector is the collector already on the registry.
52
+ // Drain its Describe channel to confirm the expected metric name is present.
53
+ ch := make(chan *prometheus.Desc, 32)
54
+ go func() {
55
+ are.ExistingCollector.Describe(ch)
56
+ close(ch)
57
+ }()
58
+ found := false
59
+ for d := range ch {
60
+ // Desc.String() format: Desc{fqName: "the_name", help: "...", ...}
61
+ s := d.String()
62
+ const marker = `fqName: "`
63
+ idx := 0
64
+ for idx+len(marker) <= len(s) {
65
+ if s[idx:idx+len(marker)] == marker {
66
+ start := idx + len(marker)
67
+ end := start
68
+ for end < len(s) && s[end] != '"' {
69
+ end++
70
+ }
71
+ if s[start:end] == wantName {
72
+ found = true
73
+ }
74
+ break
75
+ }
76
+ idx++
77
+ }
78
+ }
79
+ if !found {
80
+ t.Errorf("metric %q: name not found in described metrics of existing collector", wantName)
81
+ }
82
+}
83
+
84
// TestMetricsRegistryPresence asserts that all 8 Phase-1 metrics are registered
30
-// on the default registry with non-empty HELP text and the expected type.
85
+// on prometheus.DefaultRegisterer. It uses Register→AlreadyRegisteredError so
86
+// Vec metrics with no prior observations are still detected (they are invisible
87
+// to DefaultGatherer.Gather until the first label combination is used).
88
func TestMetricsRegistryPresence(t *testing.T) {
89
want := []struct {
33
- name string
34
- typ dto.MetricType
90
+ name string
91
+ collector prometheus.Collector
92
+ typ dto.MetricType
93
}{
36
- {"portal_discovery_relay_selected_total", dto.MetricType_COUNTER},
37
- {"portal_discovery_relay_pool_size", dto.MetricType_GAUGE},
38
- {"portal_discovery_rtt_seconds", dto.MetricType_HISTOGRAM},
39
- {"portal_discovery_active_tunnels_per_relay", dto.MetricType_GAUGE},
40
- {"portal_discovery_selection_duration_seconds", dto.MetricType_HISTOGRAM},
41
- {"portal_discovery_selection_skipped_total", dto.MetricType_COUNTER},
42
- {"portal_discovery_failures_total", dto.MetricType_COUNTER},
43
- {"portal_discovery_congestion_mode", dto.MetricType_GAUGE},
94
+ {"portal_discovery_relay_selected_total", RelaySelectedTotal, dto.MetricType_COUNTER},
95
+ {"portal_discovery_relay_pool_size", RelayPoolSize, dto.MetricType_GAUGE},
96
+ {"portal_discovery_rtt_seconds", RTTSeconds, dto.MetricType_HISTOGRAM},
97
+ {"portal_discovery_active_tunnels_per_relay", ActiveTunnelsPerRelay, dto.MetricType_GAUGE},
98
+ {"portal_discovery_selection_duration_seconds", SelectionDurationSeconds, dto.MetricType_HISTOGRAM},
99
+ {"portal_discovery_selection_skipped_total", SelectionSkippedTotal, dto.MetricType_COUNTER},
100
+ {"portal_discovery_failures_total", FailuresTotal, dto.MetricType_COUNTER},
101
+ {"portal_discovery_congestion_mode", CongestionMode, dto.MetricType_GAUGE},
102
}
103
104
for _, tc := range want {
47
-
105
t.Run(tc.name, func(t *testing.T) {
106
+ // Primary check: collector is on DefaultRegisterer.
107
+ assertRegisteredWithName(t, tc.collector, tc.name)
108
+ // Secondary check: if the metric has observations, verify HELP and type.
109
mf := metricFamilyByName(t, tc.name)
50
- if mf == nil {
51
- t.Fatalf("metric %q not found in registry", tc.name)
52
- }
53
- if mf.GetHelp() == "" {
54
- t.Errorf("metric %q has empty HELP string", tc.name)
55
- }
56
- if mf.GetType() != tc.typ {
57
- t.Errorf("metric %q: got type %v, want %v", tc.name, mf.GetType(), tc.typ)
110
+ if mf != nil {
111
+ if mf.GetHelp() == "" {
112
+ t.Errorf("metric %q has empty HELP string", tc.name)
113
+ }
114
+ if mf.GetType() != tc.typ {
115
+ t.Errorf("metric %q: got type %v, want %v", tc.name, mf.GetType(), tc.typ)
116
+ }
117
}
118
})
119
}