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

phpfpm plugin: enhance socket stat gathering #538

Closed
wants to merge 1 commit into from
Closed

phpfpm plugin: enhance socket stat gathering #538

wants to merge 1 commit into from

Conversation

v9n
Copy link
Contributor

@v9n v9n commented Jan 16, 2016

Ref: #502 #499

  • If we detect errors when gathering stat via socket, return those error
    so it canbe appear in Telegraf log

Example:

./telegraf -config telegraf.conf -test -input-filter phpfpm
* Plugin: phpfpm, Collection 1
2016/01/16 13:42:53 Unable parse php-fpm status. Error: Primary script unknown
 <nil>
  • Improve fcgi client, also upgrade it to current version of Go at
    https://golang.org/src/net/http/fcgi/fcgi.go
  • Add test for fcgi: both of unixsocket and tcp
  • Document about using of host in case unixsocket that it isn't used
    to remotely connect but only as an extra url field.

@sparrc
Copy link
Contributor

sparrc commented Jan 17, 2016

thanks much @kureikain 👍

#
# Note that the unixsocket only works when telegraf run on the same
# host which is being gather metric because unix socket require local
# file access. The IP address field is used as a tag, it doesn't mean
Copy link
Contributor

Choose a reason for hiding this comment

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

@kureikain as of Telegraf 0.10.0, we now have the ability to set per-plugin tags (see here: https://github.com/influxdata/telegraf/blob/master/CONFIGURATION.md#input-config-tags)

For this reason, could you just remove the IP address from unix sockets entirely? I still find this confusing, and I think it will confuse more people in the future. I think it's better if people set custom tags using the standard way, rather than inserting it into the unix socket path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sparrc Yes, agree. Let's me remove that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sparrc Removed that tag feature. I also rebase it with master so it's ready to merge if thing looks good.

- If we detect errors when gathering stat via socket, return those error
  so it canbe appear in Telegraf log
- Improve fcgi client, also upgrade it to current version of Go at
  https://golang.org/src/net/http/fcgi/fcgi.go
- Add test for unix socket and fcgi
  to remotely connect but only as an extra url field.
- Allow customization of fpm status path
- Document about using of `host` in case `unixsocket` that it isn't used
- Documet upgrade for new data layout
@sparrc sparrc closed this in 5af6974 Jan 18, 2016
@sparrc
Copy link
Contributor

sparrc commented Jan 18, 2016

great, thanks @kureikain

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