search

Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
2.01k stars 577 forks source link

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

Closed icinga-migration closed 8 years ago

icinga-migration commented 9 years ago

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: https://github.com/Icinga/icinga2/pull/43 You could get more info there.

Changesets

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

GelfWriter: Add additional fields for 'CHECK RESULT' events

fixes #9858

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

Update AUTHORS

refs #9858

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

Update AUTHORS

refs #9858
icinga-migration commented 9 years ago

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

Stalled until dnsmichi is back.

icinga-migration commented 9 years ago

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

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

icinga-migration commented 9 years ago

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 commented 9 years ago

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

Are there any news for this?

icinga-migration commented 9 years ago

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

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 commented 8 years ago

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 commented 8 years ago

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

rest is fine, just a minor typo :) https://github.com/daniilyar/icinga2/commit/208e0ad14801919a70fee4c4a54cbc8419013393#diff-db284b572b8d58fd607830d73594f479R143

icinga-migration commented 8 years ago

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

rest is fine, just a minor typo :) https://github.com/daniilyar/icinga2/commit/208e0ad14801919a70fee4c4a54cbc8419013393#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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

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

icinga-migration commented 8 years ago

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

Review

1) Code Style

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 commented 8 years ago

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

Applied in changeset d7396757994b6b4cb94a4d35a9bb22cc6ec23504.