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

Fix ipmi_sensor config is shared between all plugin instances #2684

Merged
merged 2 commits into from
Apr 20, 2017

Conversation

danielnelson
Copy link
Contributor

@danielnelson danielnelson commented Apr 18, 2017

I noticed ipmi_sensor always returns the same struct instance. This prevents having separate ipmi_sensor plugins active with different configs.

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@danielnelson danielnelson added this to the 1.3.0 milestone Apr 18, 2017
@danielnelson danielnelson added the bug unexpected problem or unintended behavior label Apr 18, 2017
path, _ := exec.LookPath("ipmitool")
if len(path) > 0 {
m.Path = path
}
Copy link
Contributor

@phemmer phemmer Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that exec.LookPath is expensive, and worth the effort to keep from repeating it, but you could do something like:

inputs.Add("ipmi_sensor", func() telegraf.Input {
  m := m
  return &m
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

@danielnelson danielnelson merged commit bf30ef8 into master Apr 20, 2017
@danielnelson danielnelson deleted the fix-multiple-ipmi-sensors branch April 20, 2017 00:02
vlamug pushed a commit to vlamug/telegraf that referenced this pull request May 30, 2017
@danielnelson danielnelson mentioned this pull request Jun 7, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants