Icinga / icingaweb2-module-idoreports

Icinga IDO Reports provides host and service availability reports for Icinga based on the monitoring database (IDO).
GNU General Public License v2.0
34 stars 13 forks source link

Don't fail if there were downtimes during the reporting period #52

Closed lippserd closed 2 years ago

bluthg commented 2 years ago

I'm a bit puzzled....

cla-bot[bot] commented 2 years ago

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @bluthg

- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please [contact us](https://icinga.com/company/contact/) if you think this is the case. - If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.
lippserd commented 2 years ago

I'm a bit puzzled....

  • What's the issue?
  • Where's the test showing the issue?
  • Do we really need this monstrosity of a function? I'm pretty sure there's a much nices (and less performance hungry!) way to get around this...

Hi Nick,

Hope you are well! Problem was/is something like this:

select tsrange('2021-10-28 11:43:08', '2021-10-28 13:02:52', '(]') - tsrange('2021-10-28 11:45:15', '2021-10-28 11:45:51', '[)');
ERROR:  result of range difference would not be contiguous

The first range is a period of time in which the object was down, and the second range is a downtime during this period.

I'm quite sure there's a better solution.

cla-bot[bot] commented 2 years ago

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @bluthg

- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please [contact us](https://icinga.com/company/contact/) if you think this is the case. - If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.
bluthg commented 2 years ago

Yeah, I figured that out :)

That's a tricky one indeed. I mean, the way the function is built right now does pass the tests I added. However they actually shouldn't pass...

postgres=# SELECT * from icinga_hoststatus where host_object_id = 7;
 host_object_id | status_update_time  | current_state | state_type
----------------+---------------------+---------------+------------
              7 | 2019-04-14 11:00:00 |             0 |          1
              7 | 2019-04-15 11:00:00 |             1 |          1
              7 | 2019-04-15 12:52:00 |             0 |          1
              7 | 2019-04-15 12:55:00 |             1 |          1
              7 | 2019-04-15 12:57:00 |             0 |          1
(5 rows)

postgres=# SELECT
postgres-#                 tsrange(actual_start_time, actual_end_time) AS downtime
postgres-#             FROM
postgres-#                 icinga_downtimehistory
postgres-#             WHERE
postgres-#                 object_id = 7
postgres-# ;
                   downtime
-----------------------------------------------
 ["2019-04-15 11:45:00","2019-04-15 11:46:00")
 ["2019-04-15 12:00:00","2019-04-15 12:05:00")
(2 rows)

postgres=# SELECT idoreports_get_sla_ok_percent(7,'2019-04-14 12:00:00','2019-04-15 15:00:00');
 idoreports_get_sla_ok_percent
-------------------------------
                           100
(1 row)

On top, just cutting a timerange in two will not cut it (pun intended) if you have TWO of these planned downtimes during one "server down" event. You'd end up with four overlapping time frames, I reckon.

cla-bot[bot] commented 2 years ago

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @bluthg

- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please [contact us](https://icinga.com/company/contact/) if you think this is the case. - If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.
bluthg commented 2 years ago

So, that last three commits (once the hellosign stops to fail, they should show up) should fix the issue without all the ado 😎

cla-bot[bot] commented 2 years ago

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @bluthg

- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please [contact us](https://icinga.com/company/contact/) if you think this is the case. - If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.
bobapple commented 2 years ago

@cla-bot check

nilmerg commented 2 years ago

Hey @bluthg,

many thanks for your changes! I don't know what this does or why it's better, but I see the increased efficiency which I really like :D

It works fine, too. :joy: