Unclear Event Handling

[nilmerg]

With the introduction of internal events, questions arose how to visualize them in Web. (See Icinga/icinga-notifications-web#157 for details)

Questions

a) Are there other events with type='internal' that don't relate to an incident's age?
If so, there is currently no way to reliably differentiate them from another, which would then be needed.

b) Internal events are linked to entries in incident_history, so the goal was seemingly to have an event for them as otherwise there wouldn't be any. Is this correct?

c) @julianbrost and me already had a discussion which led to the topic of incident_history.caused_by_incident_history_id. If b) is correct, isn't this the same?
Shouldn't we drop it then and make incident_history.event_id not nullable?

[julianbrost]

a) Are there other events with type='internal' that don't relate to an incident's age? If so, there is currently no way to reliably differentiate them from another, which would then be needed.

At the moment, there's also the following event that's closely related to the incident age, you would see it if the triggering of an escalation is delayed when the daemon wasn't running at the time it should have triggered.

icinga-notifications/internal/incident/incidents.go

Lines 48 to 52 in 4e5716e

	incident.RetriggerEscalations(&event.Event{
	Time: time.Now(),
	Type: event.TypeInternal,
	Message: "Incident reevaluation at daemon startup",
	})

b) Internal events are linked to entries in incident_history, so the goal was seemingly to have an event for them as otherwise there wouldn't be any. Is this correct?

Yes. The initial idea was that it would be very helpful to see during testing the time-based escalation what exactly caused the escalation. Then I figured this could be useful in general to always have an explicit event given as the cause for every action that happened.

c) @julianbrost and me already had a discussion which led to the topic of incident_history.caused_by_incident_history_id. If b) is correct, isn't this the same?

caused_by_incident_history_id was our first attempt to show some kind of causality in the database. In contrast to b), this would also allow to represent more complex causation trees, but I'm not sure that having this complexity is useful and I doubt that this would be displayed in a tree form anyways. And then just grouping together all history entries that share the the same event_id should be just as good.

Shouldn't we drop it then and make incident_history.event_id not nullable?

That would be my preferred option unless someone comes up with a need for caused_by_incident_history_id and it's tree structure. Otherwise, just relying on event_id sounds much simpler. It's easier to set in the database and should also be easier to handle when reading from the database.

[nilmerg]

there's also the following event

Then the type should reflect that. Not just internal but e.g. incident_age_threshold_reached and incident_reevaluation.

unless someone comes up with a need for caused_by_incident_history_id

Let's discuss/brainstorm this next week. We should settle this for once since internal events (written to db) weren't in my picture at the time. We must also not forget, that additions to escalation conditions may influence our decision. (We should think of some new ones anyway)

[nilmerg]

I have opened issues for a) (#162) and c) (#163)