Skip to content

Commit

Permalink
metric: Support slice type
Browse files Browse the repository at this point in the history
Extend metric registry to allow metric slices to be used.
Prior to this change metric registry only alowed arrays.

Slices are treated identical to the arrays -- the values
are expanded, and each metric added to the registry.  The array
itself was not added.

From some historical archeological perspective, cockroachdb#46747 added
support for embedded metric Arrays.  My best read on this PR is
that it narrowly addressed the existing "TODOs", and avoided
doing anything extra (which is good!).  But it does not appear
that there is a fundamental reason why slices should not be supported
the same way.

Expic: None

Release note: None
  • Loading branch information
Yevgeniy Miretskiy committed Sep 7, 2023
1 parent f316ead commit 6b4b6df
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 12 deletions.
9 changes: 2 additions & 7 deletions pkg/util/metric/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (r *Registry) AddMetricStruct(metricStruct interface{}) {
}

switch vfield.Kind() {
case reflect.Array:
case reflect.Array, reflect.Slice:
for i := 0; i < vfield.Len(); i++ {
velem := vfield.Index(i)
telemName := fmt.Sprintf("%s[%d]", tname, i)
Expand Down Expand Up @@ -253,12 +253,7 @@ func checkFieldCanBeSkipped(
}

switch fieldType.Kind() {
case reflect.Slice:
if isMetricType(fieldType.Elem()) {
panicHandler(context.Background(),
"expected array, found slice instead for field %s (%s)", qualifiedFieldName, fieldType)
}
case reflect.Array:
case reflect.Array, reflect.Slice:
checkFieldCanBeSkipped(skipReason, fieldName, fieldType.Elem(), parentType)
case reflect.Struct:
containsMetrics := false
Expand Down
10 changes: 5 additions & 5 deletions pkg/util/metric/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,14 @@ func TestRegistryPanicsWhenAddingUnexportedMetrics(t *testing.T) {
},
)

// Metric slice is a bug since registry only supports arrays.
// Panics when we have a mix of exported and unexported metrics.
require.PanicsWithValue(t,
"expected array, found slice instead for field <unnamed>.unexportedGaugeSlice ([]*metric.Gauge)",
unexportedErr(unnamedStructName, "unexportedSlice"),
func() {
r.AddMetricStruct(struct {
ExportedArray [1]*Counter
unexportedGaugeSlice []*Gauge
}{[1]*Counter{c}, []*Gauge{g}})
ExportedGauge *Gauge
unexportedSlice []*Counter
}{g, []*Counter{c}})
},
)

Expand Down

0 comments on commit 6b4b6df

Please sign in to comment.