Conversation
996925c to
c1dce7c
Compare
|
@danielnelson Any ETA to merge this patch ? |
reimda
left a comment
There was a problem hiding this comment.
Thanks for the PR! Daniel has moved on and isn't directly involved with telegraf now.
plugins/inputs/nsd/README.md
Outdated
| ```toml | ||
| # A plugin to collect stats from the NSD DNS resolver | ||
| [[inputs.nsd]] | ||
| ## Address of server to connect to, read from nsd conf default, optionally ':port' |
There was a problem hiding this comment.
| ## Address of server to connect to, read from nsd conf default, optionally ':port' | |
| ## Address of server to connect to, optionally with ':port'. Defaults to the address in the nsd config file. |
plugins/inputs/nsd/nsd.go
Outdated
| if len(serverIps) == 0 { | ||
| return nil, fmt.Errorf("error no ip for server: %s: %s", Server, err) | ||
| } | ||
| server := serverIps[0].IP.String() |
There was a problem hiding this comment.
I understand that nsd-control's -s option requires an IP address, not a hostname, so we need to resolve the name here. I'm a little concerned that this ignores secondary IPs. Ordinary, connecting through Dial() will try each IP in order until it succeeds (https://golang.org/pkg/net/#Dial). It may be unlikely that we can't connect on the first IP and we can on a secondary, but if this is the case and we never try the secondary IPs, telegraf will fail silently and it will be difficult for a user to debug.
I wonder if it would be worth it to remove the ability to resolve the server hostname and require the user to provide an IP, since this is what nsd-control's -s option requires.
There was a problem hiding this comment.
Agree. In fact I add this because the unbound input plugin have this behavior.
eb5a14f to
f043543
Compare
Required for all PRs: