Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing the retries enhancement #56

Closed
Spiritreader opened this issue Jul 15, 2021 · 1 comment · Fixed by #86
Closed

Implementing the retries enhancement #56

Spiritreader opened this issue Jul 15, 2021 · 1 comment · Fixed by #86
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added type:enhance-existing feature wants to enhance existing monitor

Comments

@Spiritreader
Copy link
Contributor

Spiritreader commented Jul 15, 2021

I've been looking at the code responsible for sending notifications when a service goes down mentioned in #21.
It looks like this would be quite reasonable to implement, as such I am writing a draft and am curious to see if ends up being any good.

I've had the issue many times now since using Uptime Kuma that my services get marked as down and I get a notification, when it was just a pinging issue.

I would actually try and implement this myself if that is welcome, however I am currently quite busy and don't know if I can follow up in a timely enough matter before someone else picks it up.
But making a draft is potentially helpful for the developer who ends up implementing it.

The steps would pretty much include (if I'm not mistaken, I only had 30 min or so to look at the codebase)

  • [BACKEND] add a field to the monitor called "maxRetries" which holds the maximum amount of retries allowed before a service will go "down" and trigger notifications
  • [BACKEND] Create a new monitor.status value (for example 2, indicating that the service is currently "disrupted")
  • [BACKEND] keep track of the threshold by storing the amount of failed retries with the monitor ID in an array or KV data structure (easy) or in-memory database (more sophisticated). And also update the status to the new value (instead of it being 0 if the service has not reached maxRetries. I believe this should go here in the catch section of the polling try block
    } catch (error) {
  • [BACKEND] make the modifications here and in similar places below
    if (previousBeat || bean.status !== 1) {
    to incorporate the notification triggering only when the threshold has been reached
  • [FRONTEND] Add the retries field to the monitor edit page
  • [FRONTEND] Optional: Indicate that the service is potentially disrupted by displaying the retry count. This could add bloat to the UI experience, so not sure if that makes sense.

I also never worked with bean, so it's possible that a manual data migration would be necessary if a new field is added to the database. The documentation lists that bean does not have migration?
Does anyone know how @louislam is solving the migration task?

I think this would describe the minimum valuable product. Using the configured monitor ping interval doesn't require too many changes and requires little modification in the codebase.

What do you think?

Also I apologize that this issue is such a mess, I accidentally hit enter instead of backspace when editing the title so I essentially had to write the draft while the issue already existed 😢

@Spiritreader Spiritreader changed the title Implementing the etries enhancement Implementing the retries enhancement Jul 15, 2021
@louislam louislam added the feature-request Request for new features to be added label Jul 16, 2021
@Spiritreader
Copy link
Contributor Author

Spiritreader commented Jul 21, 2021

This feature is now tracked in PR #86.

@louislam louislam linked a pull request Jul 22, 2021 that will close this issue
louislam added a commit that referenced this issue Jul 22, 2021
@CommanderStorm CommanderStorm added area:monitor Everything related to monitors type:enhance-existing feature wants to enhance existing monitor labels Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors feature-request Request for new features to be added type:enhance-existing feature wants to enhance existing monitor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants