Add Icinga2 input plugin #2668
Add Icinga2 input plugin #2668
Conversation
mlabouardy
changed the title
mlabouardy
changed the title
danielnelson
added
the
feat
|
@danielnelson any news ? |
plugins/inputs/icinga2/icinga2.go
Outdated
| type ObjectType string | ||
|
|
||
| var sampleConfig = ` | ||
| ## Required Icinga2 server address (default: "https://localhost:5665") |
plugins/inputs/icinga2/icinga2.go
Outdated
|
|
||
| tr := &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, | ||
| } |
There was a problem hiding this comment.
Make a single transport that is scoped to the Icinga2 struct, instead of created each Gather. This will prevent connections being closed
plugins/inputs/icinga2/icinga2.go
Outdated
|
|
||
| tr := &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, | ||
| } |
There was a problem hiding this comment.
Why are we disabling ssl verification? Should we make this configurable:
## Optional SSL Config
# ssl_ca = "/etc/telegraf/ca.pem"
# ssl_cert = "/etc/telegraf/cert.pem"
# ssl_key = "/etc/telegraf/key.pem"
## Use SSL but skip chain & host verification
# insecure_skip_verify = falseCheck the apache input for an example.
plugins/inputs/icinga2/icinga2.go
Outdated
|
|
||
| client := &http.Client{Transport: tr} | ||
|
|
||
| url := fmt.Sprintf("%s/v1/objects/%s?attrs=name&attrs=display_name&attrs=state&attrs=check_command", s.Server, s.Filter) |
There was a problem hiding this comment.
Would be good to use the url package to construct the url, It looks like you could do a lot of this only once.
plugins/inputs/icinga2/icinga2.go
Outdated
| url := fmt.Sprintf("%s/v1/objects/%s?attrs=name&attrs=display_name&attrs=state&attrs=check_command", s.Server, s.Filter) | ||
|
|
||
| req, err := http.NewRequest("GET", url, nil) | ||
|
|
There was a problem hiding this comment.
Don't add whitespace between the call and checking the error throughout the pr.
plugins/inputs/icinga2/icinga2.go
Outdated
| tags := make(map[string]string) | ||
|
|
||
| record["name"] = check.Attrs.Name | ||
| record["status"] = check.Attrs.State |
There was a problem hiding this comment.
Why not call this field state since that is the name in Icinga?
plugins/inputs/icinga2/icinga2.go
Outdated
| record := make(map[string]interface{}) | ||
| tags := make(map[string]string) | ||
|
|
||
| record["name"] = check.Attrs.Name |
There was a problem hiding this comment.
Maybe the name should be a tag? I see in the tests one of the values has a uuid: eq-par.dc2.fr!ef017af8-c684-4f3f-bb20-0dfe9fcd3dbe, how unique must this be and how often would this value change? Is it always a uuid or this is this just an artifact of your setup?
plugins/inputs/icinga2/icinga2.go
Outdated
|
|
||
| func (s *Icinga2) GatherStatus(acc telegraf.Accumulator, checks []Object) { | ||
| for _, check := range checks { | ||
| record := make(map[string]interface{}) |
There was a problem hiding this comment.
Call this variable fields
plugins/inputs/icinga2/icinga2.go
Outdated
| CheckCommand string `json:"check_command"` | ||
| DisplayName string `json:"display_name"` | ||
| Name string `json:"name"` | ||
| State float32 `json:"state"` |
There was a problem hiding this comment.
Should this be an int?
plugins/inputs/icinga2/README.md
Outdated
| ## Icing2 Endpoint | ||
| server = "https://127.0.0.1:5665" | ||
| ## Required Icinga2 object type ("services" or "hosts, default "services") | ||
| filter = "services" |
There was a problem hiding this comment.
Does this work for any object type, or just services and hosts?
|
@mlabouardy Do you have time to address the feedback provided by daniel? |
|
@glinton yes, I will work on it today |
|
I just closed by mistake, can we open it again |
|
@danielnelson @glinton I just opened a new one with the required fixes, sorry guys: #4559 |
Required for all PRs: