Skip to content

Commit

Permalink
Dont append to slices in mergeStruct
Browse files Browse the repository at this point in the history
  • Loading branch information
sparrc committed Nov 18, 2015
1 parent 03a6f28 commit 3e3f015
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 66 deletions.
6 changes: 1 addition & 5 deletions agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,7 @@ func (a *Agent) LoadPlugins(filters []string, config *Config) ([]string, error)

for name, plugin := range config.PluginsDeclared() {
if sliceContains(name, filters) || len(filters) == 0 {
config, err := config.ApplyPlugin(name, plugin)
if err != nil {
return nil, err
}

config := config.GetPluginConfig(name)
a.plugins = append(a.plugins, &runningPlugin{name, plugin, config})
names = append(names, name)
}
Expand Down
21 changes: 3 additions & 18 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,8 @@ func (c *Config) ApplyAgent(a *Agent) error {
return nil
}

// ApplyPlugin loads the Plugin struct built from the config into the given Plugin struct.
// Overrides only values in the given struct that were set in the config.
// Additionally return a ConfiguredPlugin, which is always generated from the config.
func (c *Config) ApplyPlugin(name string, v interface{}) (*ConfiguredPlugin, error) {
if c.plugins[name] != nil {
err := mergeStruct(v, c.plugins[name], c.pluginFieldsSet[name])
if err != nil {
return nil, err
}
return c.pluginConfigurations[name], nil
}

return nil, nil
func (c *Config) GetPluginConfig(name string) *ConfiguredPlugin {
return c.pluginConfigurations[name]
}

// Couldn't figure out how to get this to work with the declared function.
Expand Down Expand Up @@ -405,11 +394,7 @@ func mergeStruct(base, overlay interface{}, fields []string) error {
overlayValue.Type(), field)
}
baseFieldValue := findField(field, baseValue)
if overlayFieldValue.Kind() == reflect.Slice {
baseFieldValue.Set(reflect.AppendSlice(baseFieldValue, overlayFieldValue))
} else {
baseFieldValue.Set(overlayFieldValue)
}
baseFieldValue.Set(overlayFieldValue)
}
return nil
}
Expand Down
71 changes: 34 additions & 37 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ type MergeStructSuite struct {
}

func (s *MergeStructSuite) SetupSuite() {
s.AllFields = []string{"string_field", "integer_field", "float_field", "boolean_field", "date_time_field", "array_field", "table_array_field"}
s.AllFields = []string{"string_field", "integer_field", "float_field",
"boolean_field", "date_time_field", "array_field", "table_array_field"}
}

func (s *MergeStructSuite) SetupTest() {
Expand Down Expand Up @@ -90,7 +91,8 @@ func (s *MergeStructSuite) TestEmptyMerge() {
if err != nil {
s.T().Error(err)
}
s.Equal(s.FullStruct, s.EmptyStruct, fmt.Sprintf("Full merge of %v onto an empty struct failed.", s.FullStruct))
s.Equal(s.FullStruct, s.EmptyStruct,
fmt.Sprintf("Full merge of %v onto an empty struct failed.", s.FullStruct))
}

func (s *MergeStructSuite) TestFullMerge() {
Expand All @@ -100,16 +102,8 @@ func (s *MergeStructSuite) TestFullMerge() {
FloatField: 2.2,
BooleansField: true,
DatetimeField: time.Date(1965, time.March, 25, 17, 0, 0, 0, time.UTC),
ArrayField: []string{"one", "two", "three", "four", "five", "six"},
ArrayField: []string{"four", "five", "six"},
TableArrayField: []subTest{
subTest{
AField: "one",
AnotherField: 1,
},
subTest{
AField: "two",
AnotherField: 2,
},
subTest{
AField: "three",
AnotherField: 3,
Expand All @@ -125,7 +119,8 @@ func (s *MergeStructSuite) TestFullMerge() {
if err != nil {
s.T().Error(err)
}
s.Equal(result, s.FullStruct, fmt.Sprintf("Full merge of %v onto FullStruct failed.", s.AnotherFullStruct))
s.Equal(result, s.FullStruct,
fmt.Sprintf("Full merge of %v onto FullStruct failed.", s.AnotherFullStruct))
}

func (s *MergeStructSuite) TestPartialMergeWithoutSlices() {
Expand All @@ -148,11 +143,14 @@ func (s *MergeStructSuite) TestPartialMergeWithoutSlices() {
},
}

err := mergeStruct(s.FullStruct, s.AnotherFullStruct, []string{"string_field", "float_field", "date_time_field"})
err := mergeStruct(s.FullStruct, s.AnotherFullStruct,
[]string{"string_field", "float_field", "date_time_field"})
if err != nil {
s.T().Error(err)
}
s.Equal(result, s.FullStruct, fmt.Sprintf("Partial merge without slices of %v onto FullStruct failed.", s.AnotherFullStruct))
s.Equal(result, s.FullStruct,
fmt.Sprintf("Partial merge without slices of %v onto FullStruct failed.",
s.AnotherFullStruct))
}

func (s *MergeStructSuite) TestPartialMergeWithSlices() {
Expand All @@ -164,14 +162,6 @@ func (s *MergeStructSuite) TestPartialMergeWithSlices() {
DatetimeField: time.Date(1965, time.March, 25, 17, 0, 0, 0, time.UTC),
ArrayField: []string{"one", "two", "three"},
TableArrayField: []subTest{
subTest{
AField: "one",
AnotherField: 1,
},
subTest{
AField: "two",
AnotherField: 2,
},
subTest{
AField: "three",
AnotherField: 3,
Expand All @@ -183,11 +173,14 @@ func (s *MergeStructSuite) TestPartialMergeWithSlices() {
},
}

err := mergeStruct(s.FullStruct, s.AnotherFullStruct, []string{"string_field", "float_field", "date_time_field", "table_array_field"})
err := mergeStruct(s.FullStruct, s.AnotherFullStruct,
[]string{"string_field", "float_field", "date_time_field", "table_array_field"})
if err != nil {
s.T().Error(err)
}
s.Equal(result, s.FullStruct, fmt.Sprintf("Partial merge with slices of %v onto FullStruct failed.", s.AnotherFullStruct))
s.Equal(result, s.FullStruct,
fmt.Sprintf("Partial merge with slices of %v onto FullStruct failed.",
s.AnotherFullStruct))
}

func TestConfig_mergeStruct(t *testing.T) {
Expand Down Expand Up @@ -240,8 +233,10 @@ func TestConfig_parsePlugin(t *testing.T) {
Interval: 5 * time.Second,
}

assert.Equal(t, kafka, c.plugins["kafka"], "Testdata did not produce a correct kafka struct.")
assert.Equal(t, kConfig, c.pluginConfigurations["kafka"], "Testdata did not produce correct kafka metadata.")
assert.Equal(t, kafka, c.plugins["kafka"],
"Testdata did not produce a correct kafka struct.")
assert.Equal(t, kConfig, c.pluginConfigurations["kafka"],
"Testdata did not produce correct kafka metadata.")
}

func TestConfig_LoadDirectory(t *testing.T) {
Expand All @@ -257,7 +252,7 @@ func TestConfig_LoadDirectory(t *testing.T) {
kafka := plugins.Plugins["kafka"]().(*kafka_consumer.Kafka)
kafka.ConsumerGroupName = "telegraf_metrics_consumers"
kafka.Topic = "topic_with_metrics"
kafka.ZookeeperPeers = []string{"localhost:2181", "test.example.com:2181"}
kafka.ZookeeperPeers = []string{"test.example.com:2181"}
kafka.BatchSize = 10000

kConfig := &ConfiguredPlugin{
Expand All @@ -281,10 +276,6 @@ func TestConfig_LoadDirectory(t *testing.T) {

ex := plugins.Plugins["exec"]().(*exec.Exec)
ex.Commands = []*exec.Command{
&exec.Command{
Command: "/usr/bin/mycollector --foo=bar",
Name: "mycollector",
},
&exec.Command{
Command: "/usr/bin/myothercollector --foo=bar",
Name: "myothercollector",
Expand All @@ -305,12 +296,18 @@ func TestConfig_LoadDirectory(t *testing.T) {

pConfig := &ConfiguredPlugin{Name: "procstat"}

assert.Equal(t, kafka, c.plugins["kafka"], "Merged Testdata did not produce a correct kafka struct.")
assert.Equal(t, kConfig, c.pluginConfigurations["kafka"], "Merged Testdata did not produce correct kafka metadata.")
assert.Equal(t, kafka, c.plugins["kafka"],
"Merged Testdata did not produce a correct kafka struct.")
assert.Equal(t, kConfig, c.pluginConfigurations["kafka"],
"Merged Testdata did not produce correct kafka metadata.")

assert.Equal(t, ex, c.plugins["exec"], "Merged Testdata did not produce a correct exec struct.")
assert.Equal(t, eConfig, c.pluginConfigurations["exec"], "Merged Testdata did not produce correct exec metadata.")
assert.Equal(t, ex, c.plugins["exec"],
"Merged Testdata did not produce a correct exec struct.")
assert.Equal(t, eConfig, c.pluginConfigurations["exec"],
"Merged Testdata did not produce correct exec metadata.")

assert.Equal(t, pstat, c.plugins["procstat"], "Merged Testdata did not produce a correct procstat struct.")
assert.Equal(t, pConfig, c.pluginConfigurations["procstat"], "Merged Testdata did not produce correct procstat metadata.")
assert.Equal(t, pstat, c.plugins["procstat"],
"Merged Testdata did not produce a correct procstat struct.")
assert.Equal(t, pConfig, c.pluginConfigurations["procstat"],
"Merged Testdata did not produce correct procstat metadata.")
}
3 changes: 0 additions & 3 deletions testdata/subconfig/monitor_grafana.conf

This file was deleted.

3 changes: 0 additions & 3 deletions testdata/subconfig/monitor_influxdb.conf

This file was deleted.

5 changes: 5 additions & 0 deletions testdata/subconfig/procstat.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[procstat]
[[procstat.specifications]]
pid_file = "/var/run/grafana-server.pid"
[[procstat.specifications]]
pid_file = "/var/run/influxdb/influxd.pid"

0 comments on commit 3e3f015

Please sign in to comment.