[dev.icinga.com #9858] Gelf module: expose 'perfdata' fields for 'CHECK_RESULT' events

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/9858

Created by daniilyar on 2015-08-06 14:30:18 +00:00

Assignee: mariussturm
Status: Resolved (closed on 2015-12-18 10:15:03 +00:00)
Target Version: 2.5.0
Last Update: 2015-12-18 10:15:03 +00:00 (in Redmine)

Backport?: No
Include in Changelog: 1

---

Current GELF Icinga plugin output for check results doesn't include any 'perfdata' fields. Instead GELF output includes human-readable, but not highly parseable strings (like 'shortmessage') which contains only a part of meaningful check result. This makes GELF plugin almost impossible to use for graphing and analysing Icinga check results somewhere else outside the Icinga. E.g., at popular Elasticsearch + Logstash + Kibana stack. This is very inconvenient from the point of Logstash (or any other monitoring data parsing tools coming with a Gelf input) as all data parsing tools needs input data to be highly parseable.

Also there is a bunch of Icinga internal variables which, I think, are good to be exposed in GELF plugin CHECK RESULT events, too. E.g., 'checkable->IsReachable()' field could be useful at GELF receiver side for alerting purposes.

I've made a pull request at GitHub which exposes all 'perfdata' fields and 'checkable->IsReachable()' field at GELF plugin 'CHECK RESULT' events: #43
You could get more info there.

Changesets

2015-12-18 10:05:38 +00:00 by (unknown) d739675

GelfWriter: Add additional fields for 'CHECK RESULT' events

fixes #9858

2015-12-18 10:10:54 +00:00 by mfriedrich 5aa4700

Update AUTHORS

refs #9858

2016-02-23 15:28:04 +00:00 by mfriedrich c256ea1

Update AUTHORS

refs #9858

[icinga-migration]

Updated by gbeutner on 2015-08-11 06:28:46 +00:00

Stalled until dnsmichi is back.

[icinga-migration]

Updated by mfriedrich on 2015-08-17 19:01:32 +00:00

• Status changed from New to Assigned
• Assigned to set to bernd

Imho that's a question for the Graylog guys. I'm assigning this to Bernd now :)

[icinga-migration]

Updated by bernd on 2015-08-19 10:04:32 +00:00

Thanks for the heads-up and the pull request. I will talk to my colleague Marius who wrote the initial GELF support and we will review and test the PR.

[icinga-migration]

Updated by gbeutner on 2015-10-13 08:04:01 +00:00

Are there any news for this?

[icinga-migration]

Updated by mariussturm on 2015-10-20 10:37:15 +00:00

• Assigned to changed from bernd to mariussturm

Sorry, took me awhile to get my dev account running.
@daniilyar as I see the PR there is no switch to disable additional metrics collection?
Storing raw metrics data in Elasticsearch is pretty expensive and should be configurable. Is this something you could add to the PR?

[icinga-migration]

Updated by daniilyar on 2015-11-25 12:21:43 +00:00

Sorry, I didn't receive an email notification about your answer for some weird reason.
Please expect me to provide an update to PR this week. What else would you like me to fix?

mariussturm wrote:

Sorry, took me awhile to get my dev account running.
@daniilyar as I see the PR there is no switch to disable additional metrics collection?
Storing raw metrics data in Elasticsearch is pretty expensive and should be configurable. Is this something you could add to the PR?

[icinga-migration]

Updated by mariussturm on 2015-11-25 13:06:01 +00:00

rest is fine, just a minor typo :) daniilyar@208e0ad#diff-db284b572b8d58fd607830d73594f479R143

[icinga-migration]

Updated by daniilyar on 2015-11-26 22:12:02 +00:00

rest is fine, just a minor typo :) daniilyar@208e0ad#diff-db284b572b8d58fd607830d73594f479R143

aww, here is why our patched Icinga logs 'invalid perfdata' errors on behalf of OpenTsdbWriter which we don't use ) Fixed.

I've switched adding perfdata fields OFF by default, so folks who already use GelfWriter will only notice 3 new fields: 'latency', 'execution_time' and 'reachable'.
To switch adding perfdata fields ON 'include_perfdata_fields' option should be changed to 'true' at /etc/icinga2/features-available/gelf.conf

Is there any docs for GelfWriter module I can extend with new option?

[icinga-migration]

Updated by mariussturm on 2015-12-07 10:57:17 +00:00

Tested this one and looks good to me now!

@dnsmichi could you merge please?

[icinga-migration]

Updated by mfriedrich on 2015-12-07 12:29:43 +00:00

Can you provide a feature branch on git.icinga.org with the changes squashed into one single commit for review? Merci.

[icinga-migration]

Updated by mariussturm on 2015-12-07 21:46:43 +00:00

@dnsmichi should be now in feature/gelf-perfdata-9858

[icinga-migration]

Updated by mfriedrich on 2015-12-18 10:09:45 +00:00

• Target Version set to 2.5.0
• Backport? changed from Not yet backported to No

Review

1. Code Style

• Line-breaks for if-conditions
• Log() with shift operators require a new line with 4 spaces indent
• It's not necessary to include the commented config option inside the gelf.conf file, as it is not a mandatory one. Users shall read the documentation, especially getting to know its meaning from the "description" table entry.

2. Naming of the configuration attribute

I've renamed it to "enable_send_perfdata" to reflect its purpose, in a similar naming pattern as we use in the GraphiteWriter feature.

3. Documentation

The attribute was missing in the 6-object-types.md table. When sending patches adding or changing things, please always add/update the documentation.

[icinga-migration]

Updated by Anonymous on 2015-12-18 10:15:03 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset d739675.