Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

[dev.icinga.com #770] Acknowledge with expire time #369

Closed
icinga-migration opened this issue Sep 2, 2010 · 37 comments
Closed

[dev.icinga.com #770] Acknowledge with expire time #369

icinga-migration opened this issue Sep 2, 2010 · 37 comments

Comments

@icinga-migration
Copy link

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

Created by mfriedrich on 2010-09-02 10:28:32 +00:00

Assignee: ricardo
Status: Resolved (closed on 2011-11-11 16:56:49 +00:00)
Target Version: 1.6
Last Update: 2011-12-03 11:30:09 +00:00 (in Redmine)


http://feedback.icinga.org/forums/50329-general/suggestions/890535-acknowledge-with-expire-time?ref=title

Setting a default (mandatory) expiration for acknowledgements would require NOC to actively work issues or be paged again. Currently system allows for acknowledged but unfixed problems to fester.

Attachments

Changesets

2010-09-04 10:26:46 +00:00 by ricardo 09e15152b5c60b1e289359b487427321a43ad37f

added the ability to set a expiring date/time for a acknowledgement.

there is a new option when you acknowledge hosts or services in the Classic-UI.
this has also to be implemented in modules which are using acknowledgements
and in icinga-web because the command string changed.

Fixes #770

2010-10-22 17:25:06 +00:00 by ricardo 36e0a33

added the ability to set a expiring date/time for a acknowledgement. #770

there is a new option when you acknowledge hosts or services in the Classic-UI.
this has also to be implemented in modules which are using acknowledgements
and in icinga-web because the command string changed.

Fixes #770

2011-09-12 20:25:52 +00:00 by ricardo 55c200f

Added the ability for acknowledgements to expire #770

Refs: #770

You can set a date (option) in the Classic-UI when the acknowledgment expires.
After the expire date Icinga removes the acknowledgement itself. The date the
acknowledgement expires is added to the comment.

* Added 2 new COMMANDS:
  * CMD_ACKNOWLEDGE_HOST_PROBLEM_EXPIRE    174
  * CMD_ACKNOWLEDGE_SVC_PROBLEM_EXPIRE     175
* Added function "broker_acknowledgement_data_expire" with added expire timestamp
  Function "broker_acknowledgement_data" still exists and is working
* Added new attribute "end_time" at the end of "nebstruct_acknowledgement_data"
* Added new event to event loop "EVENT_EXPIRE_ACKNOWLEDGEMENT"
* Added acknowledgement_end_time to host and service data in status and retention file
* Added a cgi config option "default_expiering_acknowledgement_duration"

2011-09-13 17:25:46 +00:00 by ricardo afa2835

fixed last commit for expiring downtime. #770

refs: #770

* typo
* change statusdata and objectdata sctruct, added value at the end now.

2011-09-16 16:42:36 +00:00 by mfriedrich a125426

idoutils: added end_time column for acknowledgelemts expiry #770

please check the issue itsself for further information.
https://dev.icinga.org/issues/770

this step is required in order to give the icinga-web team some time
to implement that too.

refs #770

Relations:

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-02 10:29:17 +00:00

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

@ricardo

Please have a look :-)

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-04 10:39:02 +00:00

  • Target Version set to 1.2 (Stable)
  • Done % changed from 0 to 50

Fixed with git commit: 09e15152b5c60b1e289359b487427321a43ad37f

The command string has changed. Added the expire timestamp.
So it needs further implementation in modules which uses acknowledgement and
in icinga-web to send the correct command.

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-04 19:40:34 +00:00

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

Applied in changeset commit:"09e15152b5c60b1e289359b487427321a43ad37f".

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-14 07:23:26 +00:00

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

by changing the command string, this will break backwards compatibility.
wouldn't it be better to create a new external command in this case?

furthermore, the broker data struct ds is being enhanced too. this might lead into problems for current neb modules.

next to that, idoutils needs to be adapted reading acknowledgement expiry time from the buffer, pushing into the database (query adaption, schema changes). this affects then the api querying the information, and the web displaying this information.

since this is a rather huge change, please discuss.

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-14 20:12:58 +00:00

I'll try to implement the changes that have to be done to the other components as well.

This well be:

  • idoutils
  • databasesheme
  • api
  • web

hopefully finishing it bevor the next release

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-14 21:42:24 +00:00

i would propose the other way around - check if other popular addons like merlin or livestatus still work with that (typecasting hell!) and provide feedback.

i still can implement the changes for idoutils, it's not that much to do/change. for api/web:

  • api: acknowledgements query with one more column, resultset bigger
  • web: acknowledgements cronk with one more column fetching resultset from api

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-16 10:23:26 +00:00

The problem is, I have no idea what kind of modules are out there.

At first I tested mk-livestatus. It crashed icinga. I have written a patch for mk-livestatus and with this one it works fine.

now i gonna try out merlin.
is it the same with dnx?

What modules do you all know? Is there a other way to implement this feature?

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-16 10:34:27 +00:00

ricardo wrote:

The problem is, I have no idea what kind of modules are out there.

At first I tested mk-livestatus. It crashed icinga. I have written a patch for mk-livestatus and with this one it works fine.

well everything changing core structures will crash livestatus. can you attach the patch for livestatus to see what needs to be changed?

now i gonna try out merlin.
is it the same with dnx?

probably dnx too, but i am not sure if the acknowledgements struct is used over there in regard of typecasting.

What modules do you all know? Is there a other way to implement this feature?

well in regard of changing core structures, a proposed way is to add new struct members at the end in order to allow memory mapping and typecasts. but i am not sure if anything else will break that then too.

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-16 12:45:28 +00:00

  • File added add_expired_acknowledgements_to_mk-livestatus.diff
  • File added add_expired_acknowledgements_to_module_icinga_headers.diff

here is the patch for livestatus.

All other modules should be working with changed headers. Then they can not request the data but at least they don't crash.

Merlin seems to work fine.

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-16 13:50:07 +00:00

as far as I could investigate it dnx isn't using any of the nebstruct_acknowledgement_struct. So this shouldn't be a problem.

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-16 14:40:36 +00:00

ricardo wrote:

as far as I could investigate it dnx isn't using any of the nebstruct_acknowledgement_struct. So this shouldn't be a problem.

the same for mod_gaerman

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-16 14:41:53 +00:00

maybe we can adapt the struct changes - add the new params at the end of the struct. but for the rest i figure that it might need a livestatus patch then.

@icinga-migration
Copy link
Author

Updated by magellanic on 2010-09-16 15:15:41 +00:00

ricardo wrote:

as far as I could investigate it dnx isn't using any of the nebstruct_acknowledgement_struct. So this shouldn't be a problem.

I've been having a peak at dnx, I agree that this shouldn't effect it, it doesn't seem to do weird struct casting noted in the livestatus stuff.
I can pledge to test dnx in about 2 weeks when I'm back home.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-16 15:29:27 +00:00

i think this a very big change/enhancement to the current core which needs to be tested on several platforms and with different addons like you just did.

we can try to do it during the release freeze, but i would like to give addons devs the chance to discuss about that changes and even to implement comaptibility.

for that reason i'd like to add that change into 1.3.x (unstable) and show that to the community - and not 1.2 which should run stable.

your thoughts?

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-09-16 22:05:10 +00:00

  • Target Version changed from 1.2 (Stable) to 62

I agree, sounds reasonable.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-09-17 11:46:13 +00:00

regarding your commit, my fix for #790 will remove the check_program_update define (16) completely. just to mention that.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-10-10 12:13:52 +00:00

  • Target Version changed from 62 to 1.3

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-10-22 17:41:26 +00:00

reimplemented it into my new rbartels/1.3 branch

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-10-22 19:13:32 +00:00

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

Applied in changeset commit:"36e0a3369e039f0af2913287c444461c150769d5".

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-11-03 17:05:43 +00:00

  • Status changed from Resolved to Feedback
  • Done % changed from 100 to 90

still, this breaks the NEB API. i would like to bump for Icinga 2.x instead of pushing that onto 1.3 branch. 1.3 will be the base for 1.4 stable.

We can take care of IDOUtils, but it will also break Merlin or Centreon Broker which are considered to be important addons meanwhile.

we need to figure out a workaround for modules not supporting that, but exporting the old fashioned style of the neb callbacks. possibly by adding a new callback then keeping the old for compatibility reasons!!!

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-11-05 11:24:09 +00:00

just some more thoughts on this, as you have proposed another feature which might break the neb api.

how about creating an "icinga event broker api" and keeping the old "nagios event broker" api intact?

then we might register idoutils/idomod functions like we want, but all other addons still work with the old grown neb api.

this will need a deep rundown into the core, re-implementing the broker.c and nebmods.c to fit our needs.

using this way, we are able to keep compatibility in various ways, but also offer our own abstracted layer.

i'd also see that as pre-requisite of a new core api in that case, doing a proof of concept and clearly adding abstracted layer functions for such callbacks.

then i'd like to see this feature with the new IEB API instead of NEB API, and IDOUtils will remove compatibility with NEB and step on IEB - which brings the benefit of integrating it even more deep into the core and hackup things like queued events in the broker e.g.

thoughts?

@icinga-migration
Copy link
Author

Updated by ricardo on 2010-11-05 11:33:42 +00:00

this sounds terrific.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-11-05 11:36:15 +00:00

  • Target Version changed from 1.3 to 1.5

ok, then i'll set this one up for 1.5 as the required changes might take a bunch of time for 1.3

@icinga-migration
Copy link
Author

Updated by elagon on 2010-11-05 14:23:13 +00:00

+1 for IEB !

@icinga-migration
Copy link
Author

Updated by Anonymous on 2010-11-05 16:09:27 +00:00

how will this look for mod_gearman / dnx / pnp4nagios?
Can they also recieve benefits from this? (Ofc, yes, maybe bring them here and cooperate to have them ready with functionality on ship =) )

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2010-11-05 16:13:29 +00:00

pls move the discussion over to #964 as this is just the feature request issue for acknowledgement with expiry time.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-03-14 22:00:38 +00:00

hmmm considering the recent changes on

  • address6
  • making things threadsafe

i have another idea.

  • object changes:
    • add all new attributes at the end (mklivestatus can typecast easily then, and the performance doesn't lack too much)
  • core function changes
    • create new ones, but keeping the old ones.
acknowledge_host_problem(temp_host,ack_author,ack_data,type,notify,persistent);

and

acknowledge_host_problem_end_time(temp_host,ack_author,ack_data,type,notify,persistent,end_time);

broker_acknowledgement_data(NEBTYPE_ACKNOWLEDGEMENT_ADD,NEBFLAG_NONE,NEBATTR_NONE,HOST_ACKNOWLEDGEMENT,(void *)hst,ack_author,ack_data,type,notify,persistent,end_time,NULL);

and

broker_acknowledgement_data_end_time(NEBTYPE_ACKNOWLEDGEMENT_ADD,NEBFLAG_NONE,NEBATTR_NONE,HOST_ACKNOWLEDGEMENT,(void *)hst,ack_author,ack_data,type,notify,persistent,end_time,NULL);

if someone changes the compatibility to support that too then, we can still change the old kept functions too.

cmd.cgi and retention.dat shouldn't be a problem then. i also need to add changes to idoutils then.

let's consider this something for the next unstable release!

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-07-12 18:47:42 +00:00

  • Target Version changed from 1.5 to 1.6

@icinga-migration
Copy link
Author

Updated by ricardo on 2011-09-12 20:37:56 +00:00

Well, finally submitted (1 year later) a new patch to add this function.

It will break the ABI, like "address6" does, but all modules should work fine.

THe commit Message:

Added the ability for acknowledgements to expire #770

Refs: #770

You can set a date (option) in the Classic-UI when the acknowledgment expires.
After the expire date Icinga removes the acknowledgement itself. The date the
acknowledgement expires is added to the comment.

* Added 2 new COMMANDS:
  * CMD_ACKNOWLEDGE_HOST_PROBLEM_EXPIRE    174
  * CMD_ACKNOWLEDGE_SVC_PROBLEM_EXPIRE     175
* Added function "broker_acknowledgement_data_expire" with added expire timestamp
  Function "broker_acknowledgement_data" still exists and is working
* Added new attribute "end_time" at the end of "nebstruct_acknowledgement_data" 
* Added new event to event loop "EVENT_EXPIRE_ACKNOWLEDGEMENT"
* Added acknowledgement_end_time to host and service data in status and retention file
* Added a cgi config option "default_expiering_acknowledgement_duration"

Could everyone please test this to make sure it works properly that it can hit 1.6?

After other people tested it and works for them I will add a Issue for icinga-web and icinga-docs.

Cheers
Ricardo

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-09-12 20:46:53 +00:00

and how would a neb module get the appropriate callback then?

idomod needs to be aware of this changes, ido2db needs to know that too, and the db schema will require an upgrade, before you can alert web / docs.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-09-12 20:53:42 +00:00

ok, ic, replacing the function and keeping the old one.

2 more things. the one for the neb struct new attribute at the end is fine, but you can't leave that as is in objects.h and statusdata.h - this requires this small adaption too.

https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=55c200f24ea790d78f891de85eb35b9857f087f1#patch14
https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=55c200f24ea790d78f891de85eb35b9857f087f1#patch15

and one typo - it's called "expiring", not "expiering".

https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=55c200f24ea790d78f891de85eb35b9857f087f1#patch16

the rest looks good, thanks. i'll sneak into within the next days.

@icinga-migration
Copy link
Author

Updated by ricardo on 2011-09-13 17:28:55 +00:00

Thanks for pointing out.

fixed in current 'test/core'

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-09-16 16:36:36 +00:00

  • File added icinga_classicui_acknowledgements_expiry_01.png

sweet and fancy javascript gui look

icinga_classicui_acknowledgements_expiry_01.png

external command example

icinga: EXTERNAL COMMAND: ACKNOWLEDGE_SVC_PROBLEM_EXPIRE;localhost;dep1;2;1;0;1316185500;icingaadmin;test - The acknowledgement expires at: 2011-09-16 17:05:00.

i've taken the liberty of adding it to idoutils too without creating an extra issue.

idomod debug message sample (note the 267 is a new type at the end)

[1316186007.853761] [001.2] [pid=19931] idomod_write_to_sink(
222:
1=1700
2=0
3=0
4=1316186007.853745
7=1
53=localhost
114=dep1
10=icingaadmin
17=TEST EXPIRE - The acknowledgement expires at: 2011-09-16 17:20:00.
118=2
122=1
100=0
90=1
267=1316186400
999

)

within the mysql database, a newer example after a debug session.

mysql> select * from icinga_acknowledgements;
+--------------------+-------------+---------------------+-----------------+----------------------+-----------+-------+-------------+---------------------------------------------------------------+-----------+--------------------+-----------------+---------------------+
| acknowledgement_id | instance_id | entry_time          | entry_time_usec | acknowledgement_type | object_id | state | author_name | comment_data                                                  | is_sticky | persistent_comment | notify_contacts | end_time            |
+--------------------+-------------+---------------------+-----------------+----------------------+-----------+-------+-------------+---------------------------------------------------------------+-----------+--------------------+-----------------+---------------------+
|                  1 |           1 | 2011-09-16 18:05:32 |          397377 |                    1 |       282 |     2 | icingaadmin    | EXPIRE - The acknowledgement expires at: 2011-09-16 18:10:00. |         1 |                  0 |               1 | 2011-09-16 18:10:00 | 
|                  2 |           1 | 2011-09-16 18:05:32 |          397811 |                    1 |       283 |     2 | icingaadmin    | EXPIRE - The acknowledgement expires at: 2011-09-16 18:10:00. |         1 |                  0 |               1 | 2011-09-16 18:10:00 | 
+--------------------+-------------+---------------------+-----------------+----------------------+-----------+-------+-------------+---------------------------------------------------------------+-----------+--------------------+-----------------+---------------------+
2 rows in set (0.00 sec)


icinga=> select * from icinga_acknowledgements;
 acknowledgement_id | instance_id |     entry_time      | entry_time_usec | acknowledgement_type | object_id | state | author_name |                       
  comment_data                          | is_sticky | persistent_comment | notify_contacts |      end_time       
--------------------+-------------+---------------------+-----------------+----------------------+-----------+-------+-------------+-----------------------
----------------------------------------+-----------+--------------------+-----------------+---------------------
                  1 |           1 | 2011-09-16 18:20:55 |          752665 |                    1 |       241 |     2 | icingaadmin    | EXPIRY - The acknowled
gement expires at: 2011-09-16 18:25:00. |         1 |                  0 |               1 | 2011-09-16 18:25:00
                  2 |           1 | 2011-09-16 18:20:55 |          753689 |                    1 |       242 |     2 | icingaadmin    | EXPIRY - The acknowled
gement expires at: 2011-09-16 18:25:00. |         1 |                  0 |               1 | 2011-09-16 18:25:00
(2 rows)


SQL ido@nfoo> select * from acknowledgements;

               2                2 16-Sep-2011 16:32:44           334029
                   1             4266                2
icingaadmin
EXPIRY - The acknowledgement expires at: 2011-09-16 18:35:00.
               1                  0                1 16-Sep-2011 16:35:00

               3                2 16-Sep-2011 16:32:44           334903
                   1             4267                2
icingaadmin
EXPIRY - The acknowledgement expires at: 2011-09-16 18:35:00.
               1                  0                1 16-Sep-2011 16:35:00


Elapsed: 00:00:00.01

icinga.debug on the event for expiring

[1316189401.144492] [008.0] [pid=13822] ** Timed Event ** Type: 16, Run Time: Fri Sep 16 18:10:01 2011
[1316189401.144499] [008.0] [pid=13822] ** Expire Acknowledgement Event

and voila it is gone :-)

the question remains - how should this get cleaned from the database itsself?

the dummy test for the mklivestatus users (using 1,1,9irgendwas)

# echo 'GET hosts' | /usr/bin/unixcat /var/nagios/rw/live 

1;0;0;;;1;127.0.4.1;up_4;check-host-alive!up;0;0;1.2000000000e+02;0;24x7;0;1;test_host_196,test_host_190,test_host_184,test_host_178,test_host_172,test_host_166,test_host_160,test_host_154,test_host_148,test_host_142,test_host_136,test_host_130,test_host_124,test_host_118,test_host_112,test_host_106,test_host_100,test_host_094,test_host_088,test_host_082,test_host_076,test_host_070,test_host_064,test_host_058,test_host_052,test_host_046,test_host_040,test_host_034,test_host_028,test_host_022,test_host_016,test_host_010,test_host_004;;;icingaadmin;1;0;;;test_router_4;;;1;4.1000000000e-02;0.0000000000e+00;1;router;0;1;0.0000000000e+00;../../docs/images/switch.png;;../../docs/images/switch.png;1;1;0;0;0;1316187742;0;0;0;0;0;0;0;1316187743;1.2400000000e-01;;0.0000000000e+00;5;0;;test_router_4;1316194943;0;0;;;;;0.0000000000e+00;24x7;1;0;0;0;0;0;0;0;0;0;0;1;;0;0.0000000000e+00;/=2643MB;5948;5958;0;5968;test_router_4 (checked by icinga-dev) OK: ok hostcheck;-1;1;6.0000000000e+01;0;;;0;1;;0;0;0;0.0000000000e+00;0.0000000000e+00;0.0000000000e+00
1;0;0;;;1;a.b.c.d;дверь_дверь;;0;0;1.2000000000e+02;0;24x7;0;1;;;;icingaadmin;1;0;;;дверь;;;1;0.0000000000e+00;0.0000000000e+00;1;;0;0;0.0000000000e+00;;;;1;1;0;0;0;0;0;0;0;0;0;0;0;0;5.5100000000e-01;;0.0000000000e+00;5;0;;дверь;1316194814;0;0;;;;;0.0000000000e+00;24x7;1;0;0;0;0;0;0;0;0;0;0;1;;0;0.0000000000e+00;;;-1;1;1.0000000000e+00;0;;;0;1;;0;0;0;0.0000000000e+00;0.0000000000e+00;0.0000000000e+00

[...]

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-09-16 16:41:51 +00:00

so, this needs proper testing. and as far as i have seen, you've been adding 2 new external commands, which need to get documented then.

if this hits the release branch for 1.6 we need a docs issue for that then.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-09-16 16:46:26 +00:00

and once more, when this is running properly, assign the icinga-web dev team an issue to

  • add the commands to the gui
  • add a new option + end_time to the gui
  • read that from the database for an acknowledgement list

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-10-06 17:55:00 +00:00

the gui stuff is done, the docs issue is still pending.

@icinga-migration
Copy link
Author

Updated by mfriedrich on 2011-11-11 16:56:49 +00:00

  • Status changed from Feedback to Resolved
  • Done % changed from 90 to 100

ready for release. thanks again ricardo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant