From ceee04f4c6ade82ffc432b6e61627142814e8862 Mon Sep 17 00:00:00 2001 From: FFMMM Date: Thu, 24 Mar 2022 10:37:04 -0700 Subject: [PATCH] remove Telemetry.MergeDefaults (#12606) Signed-off-by: FFMMM --- lib/telemetry.go | 52 ----------------------- lib/telemetry_test.go | 99 ------------------------------------------- 2 files changed, 151 deletions(-) delete mode 100644 lib/telemetry_test.go diff --git a/lib/telemetry.go b/lib/telemetry.go index d85e51d45d..d74edb37cb 100644 --- a/lib/telemetry.go +++ b/lib/telemetry.go @@ -1,7 +1,6 @@ package lib import ( - "reflect" "time" "github.com/armon/go-metrics" @@ -200,57 +199,6 @@ type TelemetryConfig struct { PrometheusOpts prometheus.PrometheusOpts } -// MergeDefaults copies any non-zero field from defaults into the current -// config. -// TODO(kit): We no longer use this function and can probably delete it -func (c *TelemetryConfig) MergeDefaults(defaults *TelemetryConfig) { - if defaults == nil { - return - } - cfgPtrVal := reflect.ValueOf(c) - cfgVal := cfgPtrVal.Elem() - otherVal := reflect.ValueOf(*defaults) - for i := 0; i < cfgVal.NumField(); i++ { - f := cfgVal.Field(i) - if !f.IsValid() || !f.CanSet() { - continue - } - // See if the current value is a zero-value, if _not_ skip it - // - // No built in way to check for zero-values for all types so only - // implementing this for the types we actually have for now. Test failure - // should catch the case where we add new types later. - switch f.Kind() { - case reflect.Struct: - if f.Type() == reflect.TypeOf(prometheus.PrometheusOpts{}) { - continue - } - case reflect.Slice: - if !f.IsNil() { - continue - } - case reflect.Int, reflect.Int64: // time.Duration == int64 - if f.Int() != 0 { - continue - } - case reflect.String: - if f.String() != "" { - continue - } - case reflect.Bool: - if f.Bool() { - continue - } - default: - // Needs implementing, should be caught by tests. - continue - } - - // It's zero, copy it from defaults - f.Set(otherVal.Field(i)) - } -} - func statsiteSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { addr := cfg.StatsiteAddr if addr == "" { diff --git a/lib/telemetry_test.go b/lib/telemetry_test.go deleted file mode 100644 index 4ee012f1ec..0000000000 --- a/lib/telemetry_test.go +++ /dev/null @@ -1,99 +0,0 @@ -package lib - -import ( - "reflect" - "testing" - "time" - - "github.com/armon/go-metrics/prometheus" - - "github.com/stretchr/testify/require" -) - -func makeFullTelemetryConfig(t *testing.T) TelemetryConfig { - var ( - promOpts = prometheus.PrometheusOpts{} - strSliceVal = []string{"foo"} - strVal = "foo" - intVal = int64(1 * time.Second) - ) - - cfg := TelemetryConfig{} - cfgP := reflect.ValueOf(&cfg) - cfgV := cfgP.Elem() - for i := 0; i < cfgV.NumField(); i++ { - f := cfgV.Field(i) - if !f.IsValid() || !f.CanSet() { - continue - } - // Set non-zero values for all fields. We only implement kinds that exist - // now for brevity but will fail the test if a new field type is added since - // this is likely not implemented in MergeDefaults either. - switch f.Kind() { - case reflect.Struct: - if f.Type() != reflect.TypeOf(promOpts) { - t.Fatalf("unknown struct type in TelemetryConfig: actual %v, expected: %v", f.Type(), reflect.TypeOf(promOpts)) - } - // TODO(kit): This should delve into the fields and set them individually rather than using an empty struct - f.Set(reflect.ValueOf(promOpts)) - case reflect.Slice: - if f.Type() != reflect.TypeOf(strSliceVal) { - t.Fatalf("unknown slice type in TelemetryConfig." + - " You need to update MergeDefaults and this test code.") - } - f.Set(reflect.ValueOf(strSliceVal)) - case reflect.Int, reflect.Int64: // time.Duration == int64 - f.SetInt(intVal) - case reflect.String: - f.SetString(strVal) - case reflect.Bool: - f.SetBool(true) - default: - t.Fatalf("unknown field type in TelemetryConfig" + - " You need to update MergeDefaults and this test code.") - } - } - return cfg -} - -func TestTelemetryConfig_MergeDefaults(t *testing.T) { - tests := []struct { - name string - cfg TelemetryConfig - defaults TelemetryConfig - want TelemetryConfig - }{ - { - name: "basic merge", - cfg: TelemetryConfig{ - StatsiteAddr: "stats.it:4321", - }, - defaults: TelemetryConfig{ - StatsdAddr: "localhost:5678", - StatsiteAddr: "localhost:1234", - }, - want: TelemetryConfig{ - StatsdAddr: "localhost:5678", - StatsiteAddr: "stats.it:4321", - }, - }, - { - // This test uses reflect to build a TelemetryConfig with every value set - // to ensure that we exercise every possible field type. This means that - // if new fields are added that are not supported types in the code, this - // test should either ensure they work or fail to build the test case and - // fail the test. - name: "exhaustive", - cfg: TelemetryConfig{}, - defaults: makeFullTelemetryConfig(t), - want: makeFullTelemetryConfig(t), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := tt.cfg - c.MergeDefaults(&tt.defaults) - require.Equal(t, tt.want, c) - }) - } -}