Skip to content

Commit

Permalink
Fix bug where struct slice elements were not being reported
Browse files Browse the repository at this point in the history
Fix a bug where the iterator did not iterate correctly over slice
elements of type struct. Add tests that assert corresponding metrics are
reported with expected attributes.

Additionally, assert that collected values do not contain duplicates.
  • Loading branch information
masih committed Jun 20, 2023
1 parent a9be8d8 commit c47aa31
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 22 deletions.
26 changes: 12 additions & 14 deletions metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,18 @@ func populateObservables(ctx context.Context, tctx traversalContext, observables
return
}
}
elems, ok := tctx.field.Interface().([]any)
if ok {
for _, elem := range elems {
select {
case <-ctx.Done():
return
default:
populateObservables(ctx, traversalContext{
field: reflect.ValueOf(elem),
metricName: tctx.metricName,
attrs: tctx.attrs,
tag: tctx.tag,
}, observables, commonAttrs)
}
elems := reflect.ValueOf(tctx.field.Interface())
for i := 0; i < elems.Len(); i++ {
select {
case <-ctx.Done():
return
default:
populateObservables(ctx, traversalContext{
field: elems.Index(i),
metricName: tctx.metricName,
attrs: tctx.attrs,
tag: tctx.tag,
}, observables, commonAttrs)
}
}
case reflect.Map:
Expand Down
66 changes: 58 additions & 8 deletions metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package fdbmeter
import (
"context"
"encoding/json"
"fmt"
"os"
"reflect"
"testing"
Expand All @@ -24,6 +25,24 @@ func TestMetricsPopulatesValues(t *testing.T) {
attrs: attribute.NewSet(),
},
},
"cluster_processes_roles_bytes_queried_counter": {
{
int64Value: 75292021001,
attrs: attribute.NewSet(
attribute.String("active_primary_dc", "abc"),
attribute.String("address", "20.10.7.15:4509"),
attribute.String("class_source", "command_line"),
attribute.String("class_type", "storage"),
attribute.String("fault_domain", "ip-1-2-7-106"),
attribute.String("id", "1d43ff854cd58260"),
attribute.String("machine_id", "ip-1-2-7-106"),
attribute.String("process_id", "1e5c1cb8cafedd3b4aa335c91c008e5d"),
attribute.String("protocol_version", "fdb00b071010000"),
attribute.String("role", "storage"),
attribute.String("version", "7.1.33"),
),
},
},
"cluster_data_state_healthy": {
{int64Value: 0, attrs: attribute.NewSet(
attribute.String("active_primary_dc", "abc"),
Expand Down Expand Up @@ -63,6 +82,18 @@ func TestMetricsPopulatesValues(t *testing.T) {
),
},
},
"cluster_machines_memory_free_bytes": {
{
int64Value: 63909851136,
attrs: attribute.NewSet(
attribute.String("active_primary_dc", ""),
attribute.String("address", "20.10.5.192"),
attribute.String("machine_id", "ip-1-2-5-156"),
attribute.String("name", "ip-1-2-5-156"),
attribute.String("protocol_version", "fdb00b071010000"),
),
},
},
},
},
}
Expand All @@ -88,19 +119,38 @@ func TestMetricsPopulatesValues(t *testing.T) {
}
subject.notifyStatus(context.Background(), status)

// Assert that there are no duplicate attributes for the same metric name.
for metric, observables := range subject.observables {
seen := make(map[string]struct{})
for _, o := range observables {
key := fmt.Sprintf("%v", o)
if _, exists := seen[key]; exists {
t.Fatal("duplicate observable for metric ", metric)
} else {
seen[key] = struct{}{}
}
}
}

WantLoop:
for k, want := range test.want {

got := subject.observables[k]
if len(got) != len(want) {
t.Fatal()
}

for i, target := range got {
if !reflect.DeepEqual(want[i], target) {
t.Fatal(want[i], " got ", target)
switch len(want) {
case 0:
if len(got) != 0 {
t.Fatal("expected no observables")
}
default:
for _, target := range got {
for _, wantObservable := range want {
if reflect.DeepEqual(wantObservable, target) {
continue WantLoop
}
}
}
t.Fatal("did not find expected observable for ", k)
}

}
})
}
Expand Down

0 comments on commit c47aa31

Please sign in to comment.