SLA

[yhabteab]

Daily breakdown:

Weekly breakdown:

fixes #713

[yhabteab]

refs Icinga/icingaweb2-module-reporting#180

[yhabteab]

refs Icinga/icingaweb2-module-reporting#181

[yhabteab]

[julianbrost]

I'm focusing only on the algorithm itself as I'm not very familiar with PHP and IPL.

General remarks:

• The query for the timeline itself is now split across two files (one with the column definitions and one with the filters). That would probably be better to understand if it was possible to keep this in one place and include comments.
• If I understand correctly, in case of a breakdown report, this currently fetches the timeline for each sub-interval separately. Generally, since now instead of a single value, the entire SLA history is fetches, it would be possible to do all this in a single query and split the result into the sub-intervals in PHP (could be an optimization, someone else has to judge if this is worth putting effort into).

The test data looks sensible but I didn't compare every single detail with the current test data on the Go side. Are there any intentional changes, especially to accommodate the change in handling 99/pending?

Also to anyone else reviewing this: if there are any question regarding the algorithm, just ask me.

[julianbrost]

This condition is changed to compared to the current one: https://github.com/Icinga/icingadb/blob/4ed4db3ab642622bb917797a8207589ce5f89f07/schema/mysql/schema.sql#L139

I think this is the change that's actually supposed to address Icinga/icingadb#558. If we want to mix this in here, I'd try change the order of actions in the loop, the current condition was my suggestion for a quick test to see if this would bring an improvement. The general idea was to check if $lastHardState would be 99 at the end of the iteration. So it could make sense to just move this branch of the conditional after the update to $lastHardState (the update of $lastEventTime would have to be moved behind that then).

[yhabteab]

So it could make sense to just move this branch of the conditional after the update to $lastHardState (the update of $lastEventTime would have to be moved behind that then).

$lastHardState is only updated when the current row event type is a state_change. How would this make any difference compared the current implementation?

[julianbrost]

It should do exactly the same. My hope is that the change makes the code easier to understand.

[yhabteab]

Are there any intentional changes, especially to accommodate the change in handling 99/pending?

Yes! ffee32b