Skip to content

Add ipmi plugin #888

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

Closed
wants to merge 14 commits into from
Closed

Add ipmi plugin #888

wants to merge 14 commits into from

Conversation

ebookbug
Copy link
Contributor

This plugin wants to collect the sensor metrics of bare metal.

Unlike the sensor plugin,the ipmi plugin will collect remote sensor metrics using ipmitool in linux

One plugin can collect multiple bare metal hosts.

@sparrc
Copy link
Contributor

sparrc commented Mar 18, 2016

Overall looks good, thank you @ebookbug for the contribution, but two things I will need to merge:

  1. Write unit tests, you can probably do this by modifying your code to make the IpmiCommand.Run function "pluggable", see the exec plugin for a good example of this in practice: https://github.com/influxdata/telegraf/tree/master/plugins/inputs/exec
  2. Write a README in this format: https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md

@ebookbug
Copy link
Contributor Author

OK,finished.
Pls check it.

for i := 0; i < len(lines); i++ {
vals := strings.Split(lines[i], "|")
if len(vals) == 3 {
tags := map[string]string{"host": conn.Hostname, "inst": trim(vals[0])}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change "host" to "server" (so that we don't override the builtin "host" tag key)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default host tag will use the local hostname which the telegraf agent running on,but ipmi plugin collect remote bare metal host's metric.
I think the local hostname is not usable,but a more tag will increase the series cardinality.

Copy link
Contributor

Choose a reason for hiding this comment

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

there is now an option to disable the default hostname, please change host to server

@ebookbug
Copy link
Contributor Author

The tags have changed to 'server','name'.
Please check it.

sparrc added a commit that referenced this pull request Mar 23, 2016
sparrc added a commit that referenced this pull request Mar 23, 2016
sparrc added a commit that referenced this pull request Mar 23, 2016
sparrc added a commit that referenced this pull request Mar 23, 2016
sparrc added a commit that referenced this pull request Mar 23, 2016
sparrc added a commit that referenced this pull request Mar 29, 2016
sparrc added a commit that referenced this pull request Mar 29, 2016
sparrc added a commit that referenced this pull request Mar 29, 2016
sparrc added a commit that referenced this pull request Mar 29, 2016
@sparrc sparrc closed this in 21ece2d Mar 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants