Sensu-go: Event history is sorted by timestamp, should be FIFO

Created on 26 Feb 2020  路  4Comments  路  Source: sensu/sensu-go

Expected Behavior

Event history is FIFO order.

Current Behavior

Event history is ordered by timestamp. However, since events are processed in essentially FIFO order, their histories should be ordered in the same way.

Possible Solution

Stop sorting event history.

bug

Most helpful comment

@echlebek @ccressent thank you both for discussing this design decision, it's very important that we understand why we do what we do. Just doing what we have done previously without challenging or coming to an understanding is dangerous. Thank you both for your thoughtful input 馃憤

All 4 comments

As discussed earlier with @echlebek, I am not 100% convinced. Here is my reasoning.

We define:

  • FIFO order: event.check.history is the last 21 check results _processed by the backend_, regardless of when the check actually ran
  • Timestamp order: event.check.history is the last 21 check results _based on their timestamp_

Those are fairly similar, if not identical, orders most of the time, but they are not if/when we receive and process older or out of order events. As summarized by @echlebek:

  • With FIFO order: stale alerts, but analysis should at least be more or less correct
  • With Timestamp order: no stale alerts, but possibly incorrect alerts/no alerts

My hunch is that timestamp order is more useful than FIFO order and that missing stale alerts is probably not an issue:

If that history has been sorted by timestamp up until now, it might be a breaking change to change that. It changes the definition of what the history is, so it's reasonable to think that could lead to breaking something else's assumptions. Our docs say the history is "Check status history for the last 21 check executions"; I think not having the last 21 highest timestamps in there would contradict that definition because the check executions happen on the agent's side, not the backend's.

To me, FIFO order is centered on the backend's perspective while Timestamp order is centered on the monitored entity's perspective. I imagine that an operator is more interested in the entity's perspective and would consider the backend's internal way of processing events to be an implementation details.

I'm not sure I agree about stale alerts being better than missed alert in practice: if I get an alert that has been stale for 2 days, in practice it's as if I had missed it altogether anyway. The latest check executions, as they happened on the entity, is what matters to me for alerting. An extreme example to illustrate my point: data points from 1979 suddenly appearing shouldn't lead to alerts.

We talked about what the definition of "old" is for an event, and how/if these old events should be processed at all. I propose that "old" is defined as "timestamp is older than the oldest timestamp in the current history" and that these events should still be processed to trigger the associated handlers, mutators, ... but are never added to event.check.history. This leads to situations where a handler can be processing events with with an event.check.history that's in the future, but I don't see that as an issue given how the documentation defines this history (the last 21 check executions).

As events are processed in FIFO order, affecting state transition, I am in the opinion that history should reflect this fact and be FIFO ordered. If we continue to order by Sensu Go agent set timestamp, we may hide out of order (in terms of check execution) event processing, making it very difficult to understand why a state transition doesn't make sense (i.e. why did or didn't this page me last night?). Event check timestamps are only used for context, we do not inspect them in our system, however, that context is still very important within history (even if out of order, fifo). Sensu 1.x stored history in FIFO order.

@echlebek @ccressent thank you both for discussing this design decision, it's very important that we understand why we do what we do. Just doing what we have done previously without challenging or coming to an understanding is dangerous. Thank you both for your thoughtful input 馃憤

@ccressent if check execution timestamps were considered within the backend, I am curious to understand the impact of "lossy" event processing, dropping older events that are processed out-of-order. I consider this to be a separate issue/exercise altogether.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

blufor picture blufor  路  4Comments

grepory picture grepory  路  5Comments

palourde picture palourde  路  3Comments

amdprophet picture amdprophet  路  4Comments

echlebek picture echlebek  路  4Comments