Distinct "internal" Event Types #175
Conversation
There was a problem hiding this comment.
General question: do we want to distinguish internal/external events in an obvious way, like keeping an internal- prefix? My intuitive choice would have been to keep some common prefix, but no strong opinion there. @nilmerg Any thoughts on this?
internal/event/event.go
Outdated
| TypeState = "state" | ||
| TypeAcknowledgement = "acknowledgement" | ||
| TypeReevaluationScheduled = "reevaluation-scheduled" | ||
| TypeReevaluationStartup = "reevaluation-startup" |
There was a problem hiding this comment.
Now that I think of this, how much of a different event is this? I mean currently, any follow-up action is still due to the incident reaching a certain age, this simply additionally says that the processing might have been delayed due to the daemon not running at the time it happened.
@nilmerg You opened the corresponding issue, so that probably means that you'd want to display this differently. Is that correct and how would you display it?
(Something I've thought about in the past and that could serve as an alternative here: store two timestamps with each event: when it happened and when it was processed, that could then be used to show a generic "processing was delayed" indicator if there's a significant difference.)
There was a problem hiding this comment.
I'd have no objections about both using the same type. We'd display an icon and short textual phrase, in addition to the event's message. (e.g. ⌛ Incident reached age)
There was a problem hiding this comment.
I have pushed a new version, effectively renaming internal to reevaluation. If future internal events emerge, we could then introduce further event types.
There was a problem hiding this comment.
Is this now final? Can Web rely on these identifiers now?
There was a problem hiding this comment.
No, at least I'm not yet convinced enough that reevaluation isn't just too generic as well. Intuitively, I'd have named it more like incident_age, but I'm als not sure if this isn't too specific and how well we can isolate the actual reason (once there are more) that happened while the daemon was stopped and thus caused some action to happen right after daemon startup.
I'd give it a try to stay as specific as we can, worst case, we could still add an generic "we reevaluated the rule, now it matched, we don't really know the event that caused this" event type in the future, but at the same time, this sounds like something that should be avoided.
There was a problem hiding this comment.
No other comments/opinions/feedback on this? 😅
We'd display an icon and short textual phrase, in addition to the event's message. (e.g. ⌛ Incident reached age)
If that's the stuff you want to display for that event type, it sounds more like you'd also expect something more like incident_age, at least "⌛ Incident reached age" wouldn't really fit for reevaluation.
oxzi
force-pushed
the
split-internal-event-actual-event-i162
branch
from
af4b719
to
1736b30
Compare
No, no idea. I wouldn't parse it as such either, so if there's a prefix, it wouldn't have any meaning to Web. |
It was ambiguous what the "internal" event type especially stand for. As currently both of its use cases are some kind of reevaluation - either during the program's startup or due to an event's age -, it was renamed to "reevaluation". To ensure database-level stability, the column type was changed from text to a custom enum. Closes #162.
oxzi
force-pushed
the
split-internal-event-actual-event-i162
branch
from
1736b30
to
d67ec1e
Compare
The incident history and event list currently display a little source icon. I don't know the exact logic for the incident history because it's currently not displayed for the events related to time-based escalations. Wouldn't it make sense to only display this for events that were externally submitted by the source and have an easy way to check this? |
|
The source icon will be removed from the history alltogether (Icinga/icinga-notifications-web#161) and the internal events won't have one either once Icinga/icinga-notifications-web#157 (comment) is done. |
The "internal" event type was used both for scheduled reevaluations and for initial reevaluations during startup. This type is currently being used at two different points for two distinct topics: scheduled reevaluations and initial reevaluations during startup.
This event type was now split into "reevaluation-scheduled" and "reevaluation-startup" to distinct those types.
To ensure database-level stability, the column type was changed from text to a custom enum.
Closes #162.