Soft failure handling for healthchecks #292

New Issue

Closed

opened 2020-02-03 21:30:12 +00:00 by lambtho12 · 7 comments

lambtho12 commented 2020-02-03 21:30:12 +00:00

Copy Link

I am currently using the "soft failure" option to backup on an external drive that is not always plugged. My backups also integrate healtchecks.io hooks. Currently it seems that healthcheck is started before the before_backup hook. Therefore in case of a soft failure, it never receives the regular ping or fail ping that it would get in case of success or hard failure. The check remains "started" until the grace period comes to an end (in this case it is flagged as failed).

I think it should be preferable to either start healthcheck after the before_bakcup hook, or to send a regular ping in case of a soft failure (as this kind of failure is "expected").

(currently running v1.5.0)

I am currently using the "soft failure" option to backup on an external drive that is not always plugged. My backups also integrate healtchecks.io hooks. Currently it seems that healthcheck is started before the before_backup hook. Therefore in case of a soft failure, it never receives the regular ping or fail ping that it would get in case of success or hard failure. The check remains "started" until the grace period comes to an end (in this case it is flagged as failed). I think it should be preferable to either start healthcheck after the before_bakcup hook, or to send a regular ping in case of a soft failure (as this kind of failure is "expected"). (currently running v1.5.0)

witten commented 2020-02-03 21:50:44 +00:00

Owner

Copy Link

Good find! This is one of those unfortunate interactions between features.. I think you are correct in that the two options are either to start the ping later, or consider a soft failure as a success as far as Healthchecks is concerned.

Given that some folks may want the ping start/end to encompass their before/after hooks as well, my first instinct is to go with option two: Leave the Healthchecks ping start where it is (before the before hooks), and then send a success ping in the case of a soft failure. Looking at the code, it looks like a pretty straight-forward (if slightly duplicative) change.

Good find! This is one of those unfortunate interactions between features.. I think you are correct in that the two options are either to start the ping later, or consider a soft failure as a success as far as Healthchecks is concerned. Given that some folks may want the ping start/end to encompass their `before`/`after` hooks as well, my first instinct is to go with option two: Leave the Healthchecks ping start where it is (before the `before` hooks), and then send a success ping in the case of a soft failure. Looking at the code, it looks like a pretty straight-forward (if slightly duplicative) change.

lambtho12 commented 2020-02-04 06:21:04 +00:00

Author

Copy Link

I have been thinking a bit more about it and I think there is unfortunately no absolute best solution.
Basically there are three cases:

1. Start -> soft fail -> ping "pass": In this case, healthcheck timer will be reset and user may never be warned by healthcheck.io that they need to backup on their removable storage. IMO this defeats the main purpose of using healthcheck for some backups. (Ex: backup everyday to cloud but want to do once a week on removable drive. Timer will be reset everyday and after a week you will skip the removable drive backup)

2. Start -> soft fail -> ping "fail": In this case the user will get a useless notification (because they now the removable drive is not plugged). Healthcheck status will be down, so user must either reactivate it by pinging manually (then fallback to the situation described in 1.) or let it stay down. In the latter, the user will not receive a notification to remind they to do the backup after the grace period because healthcheck remained down.

3. Soft fail -> healthcheck do not start: In this situation, healthcheck timer will keep counting down and the user will get a reminder after a while to ensure they do their backup on the removable drive.

Personally I am more leaning towards option 3 because as it is the only one that will accurately remind the user that they need to backup on their removable drive, but I understand your point too. I honestly do not know which one would be the absolute best for everyone. Maybe you could introduce a second parameter for the healthcheck hook (besides the ping url) to let the user chose. But that would require a bit more changes.
for instance soft_fail_reply = {pass, fail, none}

I have been thinking a bit more about it and I think there is unfortunately no absolute best solution. Basically there are three cases: 1. Start -> soft fail -> ping "pass": In this case, healthcheck timer will be reset and user may never be warned by healthcheck.io that they need to backup on their removable storage. IMO this defeats the main purpose of using healthcheck for some backups. (Ex: backup everyday to cloud but want to do once a week on removable drive. Timer will be reset everyday and after a week you will skip the removable drive backup) 2. Start -> soft fail -> ping "fail": In this case the user will get a useless notification (because they now the removable drive is not plugged). Healthcheck status will be down, so user must either reactivate it by pinging manually (then fallback to the situation described in 1.) or let it stay down. In the latter, the user will not receive a notification to remind they to do the backup after the grace period because healthcheck remained down. 3. Soft fail -> healthcheck do not start: In this situation, healthcheck timer will keep counting down and the user will get a reminder after a while to ensure they do their backup on the removable drive. Personally I am more leaning towards option 3 because as it is the only one that will accurately remind the user that they need to backup on their removable drive, but I understand your point too. I honestly do not know which one would be the absolute best for everyone. Maybe you could introduce a second parameter for the healthcheck hook (besides the ping url) to let the user chose. But that would require a bit more changes. for instance `soft_fail_reply = {pass, fail, none}`

witten commented 2020-02-05 19:56:36 +00:00

Owner

Copy Link

Switching to option 3 outright may indeed be an okay compromise, especially if borgmatic still does a Healthchecks failure ping whenever a before hook hard fails. That way, you'll continue to find out via Healthchecks if your before hook is broken, even if it's no longer wrapped in start/end pings.

The main challenge though is internal.. Currently, the Healthchecks hook starts recording the logs to be sent to Healthchecks at the time the start ping fires. If that start ping is moved to after the before hook, then the hook's logs (e.g. in the case of hook failure) won't get sent to Healthchecks.

This limitation could potentially be remedied by introducing the concept of hook initialization..

Switching to option 3 outright may indeed be an okay compromise, especially if borgmatic still does a Healthchecks failure ping whenever a `before` hook hard fails. That way, you'll continue to find out via Healthchecks if your `before` hook is broken, even if it's no longer wrapped in start/end pings. The main challenge though is internal.. Currently, the Healthchecks hook starts recording the logs to be sent to Healthchecks at the time the start ping fires. If that start ping is moved to after the `before` hook, then the hook's logs (e.g. in the case of hook failure) won't get sent to Healthchecks. This limitation could potentially be remedied by introducing the concept of hook initialization..

👍 1

N-Coder commented 2020-05-31 17:04:46 +00:00

Copy Link

I'd also vote for option 3, having a similar optional harddisk setup as lambtho.

Regarding the "internal challenge", wouldn't it suffice to add a further "after-hooks-but-before-backup" state that is passed on to the healthcheck callback function? Then you could continue to start recoding the logs on monitor.State.START, but would only report to healthchecks on monitor.State.BEFORE_BACKUP, FINISH and FAIL. The configurable hooks are already pretty diverse, why not mirror this in monitor.State?

I'd also vote for option 3, having a similar optional harddisk setup as lambtho. Regarding the "internal challenge", wouldn't it suffice to add a further "after-hooks-but-before-backup" state that is passed on to the [healthcheck callback function](https://projects.torsion.org/witten/borgmatic/src/branch/master/borgmatic/hooks/healthchecks.py#L68)? Then you could continue to start recoding the logs on `monitor.State.START`, but would only report to healthchecks on `monitor.State.BEFORE_BACKUP, FINISH` and `FAIL`. The configurable hooks are already [pretty diverse](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/), why not mirror this in `monitor.State`?

witten commented 2020-06-02 20:51:40 +00:00

Owner

Copy Link

Something like that could certainly work as well. The main reason I could see to mirror every hook into monitor.State would be if there's a need for a monitor to act differently on each of those states, e.g. before pruning, before checking, etc. In terms of satisfying this ticket, even your suggestion of just four different states would do it. Especially since the first state can serve to initialize the logging!

Something like that could certainly work as well. The main reason I could see to mirror every hook into `monitor.State` would be if there's a need for a monitor to act differently on each of those states, e.g. before pruning, before checking, etc. In terms of satisfying this ticket, even your suggestion of just four different states would do it. Especially since the first state can serve to initialize the logging!

witten referenced this issue from a commit 2020-06-02 21:36:16 +00:00

Allow before_backup and similiar hooks to exit with a soft failure without altering the monitoring status (#292).

witten commented 2020-06-02 21:39:40 +00:00

Owner

Copy Link

Well, I ended up going with the initialization function. I tried the multiple states thing first, but I found myself having to add if state in states_I_don't_want: return to a bunch of the non-Healthchecks monitors. So it seemed cleaner to break that out to an initialization function.

Given that this is all internal, we can always revisit if it turns out there's more of a need for monitors to find out about per-hook states.

This is in master and will be part of the next release! I'll note here when that happens.

Well, I ended up going with the initialization function. I tried the multiple states thing first, but I found myself having to add `if state in states_I_don't_want: return` to a bunch of the non-Healthchecks monitors. So it seemed cleaner to break that out to an initialization function. Given that this is all internal, we can always revisit if it turns out there's more of a need for monitors to find out about per-hook states. This is in master and will be part of the next release! I'll note here when that happens.

🎉 1

witten closed this issue 2020-06-02 21:39:40 +00:00

witten commented 2020-06-06 22:19:51 +00:00

Owner

Copy Link

This is released now as part of borgmatic 1.5.6!

This is released now as part of borgmatic 1.5.6!

witten referenced this issue 2020-06-17 17:55:31 +00:00

With multiple config files in borgmatic.d, cumulative logs are sent to the healthchecks endpoints #328

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

spot-check

1.8.10

1.8.9

1.8.8

1.8.7

1.8.6

1.8.5

1.8.4

1.8.3

1.8.2

1.8.1

1.8.0

1.7.15

1.7.14

1.7.13

1.7.12

1.7.11

1.7.10

1.7.9

1.7.8

1.7.7

1.7.6

1.7.5

1.7.4

1.7.3

1.7.2

1.7.1

1.7.0

1.6.6

1.6.5

1.6.4

1.6.3

1.6.2

1.6.1

1.6.0

1.5.24

1.5.23

1.5.22

1.5.21

1.5.20

1.5.19

1.5.18

1.5.17

1.5.16

1.5.15

1.5.14

1.5.13

1.5.12

1.5.11

1.5.10

1.5.9

1.5.8

1.5.7

1.5.7.dev0

1.5.6

1.5.5

1.5.4

1.5.3

1.5.2

1.5.1

1.5.0

1.4.22

1.4.21

1.4.20

1.4.19

1.4.18

1.4.17

1.4.16

1.4.15

1.4.14

1.4.13

1.4.12

1.4.11

1.4.10

1.4.9

1.4.8

1.4.7

1.4.6

1.4.5

1.4.4

1.4.3

1.4.2

1.4.1

1.4.0

1.3.26

1.3.25

1.3.24

1.3.23

1.3.22

1.3.21

1.3.20

1.3.19

1.3.18

1.3.17

1.3.16

1.3.15

1.3.14

1.3.13

1.3.12

1.3.11

1.3.10

1.3.9

1.3.8

1.3.7

1.3.6

1.3.5

1.3.4

1.3.3

1.3.2

1.3.1

1.3.0

1.2.18

1.2.17

1.2.16

1.2.15

1.2.14

1.2.13

1.2.12

1.2.11

1.2.10

1.2.9

1.2.8

1.2.7

1.2.6

1.2.5

1.2.4

1.2.3

1.2.2

1.2.1

1.2.0

1.1.15

1.1.14

1.1.13

1.1.12

1.1.11

1.1.10

1.1.9

1.1.7

1.1.6

1.1.5

1.1.4

1.1.3

1.1.2

1.1.1

1.1.0

1.0.3

1.0.2

1.0.1

1.0.0

0.1.8

0.1.7

0.1.6

0.1.5

0.1.4

0.1.3

0.1.2

0.1.1

0.1.0

0.0.7

0.0.6

0.0.5

0.0.4

0.0.3

0.0.2

Labels

Clear labels   

bug

  

data loss

  

design finalized

  

good first issue

  

new feature area

  

question / support

  

security

  

waiting for response

No Label

bug

data loss

design finalized

good first issue

new feature area

question / support

security

waiting for response

Milestone

Clear milestone

No items

No Milestone

Assignees

Clear assignees

No Assignees

3 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#292

No description provided.