From 21c4e70f33336738265a104f525f5969422e9a22 Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Tue, 17 Nov 2015 17:57:29 -0700 Subject: [PATCH] Dont append to slices in mergeStruct --- CHANGELOG.md | 9 ++- agent.go | 6 +- config.go | 21 +------ config_test.go | 71 ++++++++++++------------ testdata/subconfig/monitor_grafana.conf | 3 - testdata/subconfig/monitor_influxdb.conf | 3 - testdata/subconfig/procstat.conf | 5 ++ 7 files changed, 49 insertions(+), 69 deletions(-) delete mode 100644 testdata/subconfig/monitor_grafana.conf delete mode 100644 testdata/subconfig/monitor_influxdb.conf create mode 100644 testdata/subconfig/procstat.conf diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e0c43ff4..525e866f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,13 @@ -## v0.2.2 [unreleased] +## v0.2.3 [unreleased] + +## v0.2.2 [2015-11-18] ### Release Notes - -### Features +- 0.2.1 has a bug where all lists within plugins get duplicated, this includes +lists of servers/URLs. 0.2.2 is being released solely to fix that bug ### Bugfixes +- [#377](https://github.com/influxdb/telegraf/pull/377): Fix for duplicate slices in plugins. ## v0.2.1 [2015-11-16] diff --git a/agent.go b/agent.go index 47c2293cc..7e537f0c4 100644 --- a/agent.go +++ b/agent.go @@ -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) } diff --git a/config.go b/config.go index 1181e109b..2e9677c87 100644 --- a/config.go +++ b/config.go @@ -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. @@ -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 } diff --git a/config_test.go b/config_test.go index e5414ac0c..925bde3c9 100644 --- a/config_test.go +++ b/config_test.go @@ -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() { @@ -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() { @@ -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, @@ -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() { @@ -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() { @@ -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, @@ -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) { @@ -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) { @@ -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{ @@ -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", @@ -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.") } diff --git a/testdata/subconfig/monitor_grafana.conf b/testdata/subconfig/monitor_grafana.conf deleted file mode 100644 index f8d5dc6fa..000000000 --- a/testdata/subconfig/monitor_grafana.conf +++ /dev/null @@ -1,3 +0,0 @@ -[procstat] -[[procstat.specifications]] -pid_file = "/var/run/grafana-server.pid" diff --git a/testdata/subconfig/monitor_influxdb.conf b/testdata/subconfig/monitor_influxdb.conf deleted file mode 100644 index 43cfe94f0..000000000 --- a/testdata/subconfig/monitor_influxdb.conf +++ /dev/null @@ -1,3 +0,0 @@ -[procstat] -[[procstat.specifications]] -pid_file = "/var/run/influxdb/influxd.pid" diff --git a/testdata/subconfig/procstat.conf b/testdata/subconfig/procstat.conf new file mode 100644 index 000000000..94cfcb694 --- /dev/null +++ b/testdata/subconfig/procstat.conf @@ -0,0 +1,5 @@ +[procstat] + [[procstat.specifications]] + pid_file = "/var/run/grafana-server.pid" + [[procstat.specifications]] + pid_file = "/var/run/influxdb/influxd.pid"