Incident muting & notification suppression

[yhabteab]

resolves #190

[oxzi]

This was a first inspection of the change. Please feel free to comment if you find something unfitting.

[oxzi]

Could you please elaborate on this filter? You want all host or service event objects and every other event type unless they are ObjectCreated or ObjectDeleted. But why? Especially as those are listed within the given esTypes argument.

[yhabteab]

Object created for all Icinga 2 objects.

This is a quote from the Icinga 2 event stream docs for the ObjectCreated event and means that this event is triggered for every single object of Icinga 2 of any type, but I don't really care about any other types than Host and Service. Therefore, I want to check if event.type is not ObjectCreated and not ObjectDeleted (this means that the event type could be any other type listed in esTypes), otherwise (this means that event.type is either ObjectCreated or ObjectDeleted), I want to additionally filter on the object_type and match only on the Host or Service types.

[julianbrost]

I think this would be more readable if it was a little more verbose if expression. Something like the following should also work as a filter:

if (event.type == "ObjectCreated" || event.type == "ObjectDeleted") {
	event.object_type == "Host" || event.object_type == "Service"
} else {
	true
}

[yhabteab]

Something like the following should also work as a filter:

Did you test it? Otherwise you should have already noticed that this doesn't work.

I think this would be more readable if it was a little more verbose if expression.

This is a simple API filter expression, I'm not sure what exactly you find it to be unreadable, and apart from that I just didn't want to use the literal ObjectCreated while there is a constant typeObjectCreated for it, and no, you can't just use event.object_type == "Host" || event.object_type == "Service" or (event.type == "ObjectCreated" || event.type == "ObjectDeleted") as a filter value, you have to use quoted values "\"FOO\"".

[julianbrost]

Something like the following should also work as a filter:

Did you test it? Otherwise you should have already noticed that this doesn't work.

I didn't test it within the code here, I just verified in icinga2 console that if expressions indeed evaluate to a value so I'd expect it to work here as well:

Icinga 2 (version: v2.14.2)
Type $help to view available commands.
<1> => if (true) { 23 } else { 42 }
23.000000
<2> => if (false) { 23 } else { 42 }
42.000000

I think this would be more readable if it was a little more verbose if expression.

This is a simple API filter expression, I'm not sure what exactly you find it to be unreadable, and apart from that I just didn't want to use the literal ObjectCreated while there is a constant typeObjectCreated for it

That comment was primarily about the structure of the condition. Isn't the purpose of the filter to perform a check for some event types and leave the other ones unfiltered? I think such an if would convey this in a more straight-forward way.

and no, you can't just use event.object_type == "Host" || event.object_type == "Service" or (event.type == "ObjectCreated" || event.type == "ObjectDeleted") as a filter value, you have to use quoted values "\"FOO\"".

Yes, you can, you can alternatively enclose string literals in `` and then don't need to escape double quotes.

[yhabteab]

Have just rebased the PR to resolve the conflicts resulting from #114.

[oxzi]

I have successfully tested the PR!
@julianbrost, please take another look.

[yhabteab]

I have just rebased it and committed new changes separately 246f0f0.

[julianbrost]

@oxzi FYI: I don't expect further large changes to this PR, so feel free to have another look.

[oxzi]

Otherwise, I have successfully tested the PR and have not seen anything suspicious in the diff.

[julianbrost]

I'm happy with the current state now, I'm just not yet clicking the approve button because I still want you to address @oxzi's recent review.