Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont append to slices in mergeStruct #377

Merged
merged 1 commit into from
Nov 18, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
## v0.2.2 [unreleased]
## v0.2.3 [unreleased]

### Release Notes
## v0.2.2 [2015-11-18]

### Features
### Release Notes
- 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]

Expand Down
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"