Skip to content

Commit e013e6d

Browse files
author
Andrew Werner
committed
util/metric: allowed nils in arrays of MetricStruct implementations
It is useful to have arrays of structs just like it is useful to have arrays of values when there are enums. In the face of enums, zero is often an uninteresting value and we do not want to instantiate metrics for it. In this case of individual metrics, this was achieved by leaving the entry nil. This was not permitted in structs and would lead to panics. This commit fixes that. Release note: None
1 parent 9bfd067 commit e013e6d

File tree

2 files changed

+32
-23
lines changed

2 files changed

+32
-23
lines changed

pkg/util/metric/registry.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,18 +114,18 @@ func (r *Registry) AddMetricStruct(metricStruct interface{}) {
114114
func (r *Registry) addMetricValue(
115115
ctx context.Context, val reflect.Value, name string, skipNil bool,
116116
) {
117-
switch typ := val.Interface().(type) {
118-
case Iterable:
119-
if val.Kind() == reflect.Ptr && val.IsNil() {
120-
if skipNil {
121-
if log.V(2) {
122-
log.Infof(ctx, "skipping nil metric field %s", name)
123-
}
124-
} else {
125-
log.Fatalf(ctx, "found nil metric field %s", name)
117+
if val.Kind() == reflect.Ptr && val.IsNil() {
118+
if skipNil {
119+
if log.V(2) {
120+
log.Infof(ctx, "skipping nil metric field %s", name)
126121
}
127-
return
122+
} else {
123+
log.Fatalf(ctx, "found nil metric field %s", name)
128124
}
125+
return
126+
}
127+
switch typ := val.Interface().(type) {
128+
case Iterable:
129129
r.AddMetric(typ)
130130
case Struct:
131131
r.AddMetricStruct(typ)

pkg/util/metric/registry_test.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ func TestRegistry(t *testing.T) {
8888
StructHistogram *Histogram
8989
NestedStructGauge NestedStruct
9090
ArrayStructCounters [4]*Counter
91+
// Ensure that nil struct values in arrays are safe.
92+
NestedStructArray [2]*NestedStruct
9193
// A few extra ones: either not exported, or not metric objects.
9294
privateStructGauge *Gauge
9395
privateStructGauge64 *GaugeFloat64
@@ -113,6 +115,12 @@ func TestRegistry(t *testing.T) {
113115
nil, // skipped
114116
NewCounter(Metadata{Name: "array.struct.counter.3"}),
115117
},
118+
NestedStructArray: [2]*NestedStruct{
119+
0: nil, // skipped
120+
1: {
121+
NestedStructGauge: NewGauge(Metadata{Name: "nested.struct.array.1.gauge"}),
122+
},
123+
},
116124
privateStructGauge: NewGauge(Metadata{Name: "private.struct.gauge"}),
117125
privateStructGauge64: NewGaugeFloat64(Metadata{Name: "private.struct.gauge64"}),
118126
privateStructCounter: NewCounter(Metadata{Name: "private.struct.counter"}),
@@ -132,19 +140,20 @@ func TestRegistry(t *testing.T) {
132140
r.AddMetricStruct(ms)
133141

134142
expNames := map[string]struct{}{
135-
"top.histogram": {},
136-
"top.gauge": {},
137-
"top.floatgauge": {},
138-
"top.counter": {},
139-
"bottom.gauge": {},
140-
"struct.gauge": {},
141-
"struct.gauge64": {},
142-
"struct.counter": {},
143-
"struct.histogram": {},
144-
"nested.struct.gauge": {},
145-
"array.struct.counter.0": {},
146-
"array.struct.counter.1": {},
147-
"array.struct.counter.3": {},
143+
"top.histogram": {},
144+
"top.gauge": {},
145+
"top.floatgauge": {},
146+
"top.counter": {},
147+
"bottom.gauge": {},
148+
"struct.gauge": {},
149+
"struct.gauge64": {},
150+
"struct.counter": {},
151+
"struct.histogram": {},
152+
"nested.struct.gauge": {},
153+
"array.struct.counter.0": {},
154+
"array.struct.counter.1": {},
155+
"array.struct.counter.3": {},
156+
"nested.struct.array.1.gauge": {},
148157
}
149158

150159
r.Each(func(name string, _ interface{}) {

0 commit comments

Comments
 (0)