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

Resend Notification if Down X times consequently #1212

Merged
merged 26 commits into from
Aug 13, 2022
Merged

Resend Notification if Down X times consequently #1212

merged 26 commits into from
Aug 13, 2022

Conversation

OidaTiftla
Copy link
Contributor

@OidaTiftla OidaTiftla commented Jan 23, 2022

Description

This pull request will introduce a resend interval. The use case is the following: If a service is down a notification is sent. If the service is not fixed by time I want to get another notification after some time, for example one day.

I'm very new to pull requests, so I hope I've done everything correct. If there is anything I have forgotten please let me know, I will fix it.

Type of change

  • User interface (UI)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image

image

@OidaTiftla OidaTiftla changed the title Introduce resend interval if down Draft: Introduce resend interval if down Jan 23, 2022
@OidaTiftla OidaTiftla changed the title Draft: Introduce resend interval if down Introduce resend interval if down Jan 23, 2022
@OidaTiftla OidaTiftla marked this pull request as draft January 23, 2022 15:54
package.json Outdated Show resolved Hide resolved
server/server.js Outdated Show resolved Hide resolved
@OidaTiftla OidaTiftla marked this pull request as ready for review January 24, 2022 21:25
@OidaTiftla
Copy link
Contributor Author

Hello,

I'm new to this project and would like to know if there is anything I can do, so that this can be merged?

Best regards,
Christoph

@Saibamen
Copy link
Contributor

@louislam

@macedo
Copy link

macedo commented Apr 20, 2022

I'm very interested in this PR too, can I help with anything to get this merged?

server/model/monitor.js Outdated Show resolved Hide resolved
src/pages/EditMonitor.vue Outdated Show resolved Hide resolved
OidaTiftla and others added 3 commits April 21, 2022 17:45
Co-authored-by: Matthew Nickson <mnickson@sidingsmedia.com>
Co-authored-by: Matthew Nickson <mnickson@sidingsmedia.com>
@louislam louislam added this to the 1.17.0 milestone May 7, 2022
@louislam
Copy link
Owner

louislam commented Jun 11, 2022

Thanks for the pull request.

Just tested, I think the feature is great, but I think the implementation is not what the name described.

For example:
Monitor Interval = 3600s
Notification Resend Interval = 60s
In this case, it won't send a notification for every 60s

My suggestion:

  • Rename it to something like Resend Notification if Down X times consequently.
  • Change last_notified_time to simple Down count, if Down X times, send it again.

@louislam louislam added the question Further information is requested label Jun 13, 2022
@louislam louislam modified the milestones: 1.17.0, Pending Jun 14, 2022
@OidaTiftla OidaTiftla changed the title Introduce resend interval if down Resend Notification if Down X times consequently Jun 15, 2022
Co-authored-by: Louis Lam <louislam@users.noreply.github.com>
@OidaTiftla
Copy link
Contributor Author

@louislam thank you for your feedback and sorry for the late reply. Now I had time to integrate your suggestions.

You're right I didn't thought of this constellation, your description is the better one, thanks 👍.

I tested it again. You may now have a look again.

@louislam louislam modified the milestones: Pending, 1.18.0 Jun 15, 2022
@Gottarocket Gottarocket mentioned this pull request Jul 31, 2022
1 task
@louislam louislam removed the question Further information is requested label Aug 5, 2022
…terval

# Conflicts:
#	src/pages/EditMonitor.vue
@louislam
Copy link
Owner

louislam commented Aug 5, 2022

Just test this pr.

It seems that the notification was sent every time, but I set 2 times

image

@louislam louislam added the question Further information is requested label Aug 5, 2022
@OidaTiftla
Copy link
Contributor Author

Hi @louislam,

I tested it again and couldn't verify the failure you experienced. I had the following logging:

2022-08-05T10:12:45.659Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Check isImportant
2022-08-05T10:12:45.659Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] sendNotification
2022-08-05T10:12:45.660Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] apicache clear
2022-08-05T10:12:45.660Z [MONITOR] WARN: Monitor #1 '[Resend Noti] Resend every 2 times': Failing: getaddrinfo ENOTFOUND louislam.net2 | Interval: 20 seconds | Type: http | Down Count: 0 | Resend Interval: 2
2022-08-05T10:12:45.660Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Send to socket
2022-08-05T10:12:45.662Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Store
2022-08-05T10:12:45.732Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] prometheus.update
2022-08-05T10:12:45.732Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] SetTimeout for next check.
2022-08-05T10:13:05.734Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Prepare Options for axios
2022-08-05T10:13:05.735Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Axios Options: {"url":"https://louislam.net2","method":"get","timeout":16000,"headers":{"Accept":"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9","User-Agent":"Uptime-Kuma/1.17.1"},"maxRedirects":10,"httpsAgent":{"_events":{},"_eventsCount":2,"defaultPort":443,"protocol":"https:","options":{"maxCachedSessions":0,"rejectUnauthorized":true,"path":null},"requests":{},"sockets":{},"freeSockets":{},"keepAliveMsecs":1000,"keepAlive":false,"maxSockets":null,"maxFreeSockets":256,"scheduling":"lifo","maxTotalSockets":null,"totalSocketCount":0,"maxCachedSessions":0,"_sessionCache":{"map":{},"list":[]}}}
2022-08-05T10:13:05.735Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Axios Request
2022-08-05T10:13:05.774Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Check isImportant
2022-08-05T10:13:05.775Z [MONITOR] WARN: Monitor #1 '[Resend Noti] Resend every 2 times': Failing: getaddrinfo ENOTFOUND louislam.net2 | Interval: 20 seconds | Type: http | Down Count: 1 | Resend Interval: 2
2022-08-05T10:13:05.775Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Send to socket
2022-08-05T10:13:05.776Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Store
2022-08-05T10:13:05.867Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] prometheus.update
2022-08-05T10:13:05.867Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] SetTimeout for next check.
2022-08-05T10:13:25.873Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Prepare Options for axios
2022-08-05T10:13:25.874Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Axios Options: {"url":"https://louislam.net2","method":"get","timeout":16000,"headers":{"Accept":"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9","User-Agent":"Uptime-Kuma/1.17.1"},"maxRedirects":10,"httpsAgent":{"_events":{},"_eventsCount":2,"defaultPort":443,"protocol":"https:","options":{"maxCachedSessions":0,"rejectUnauthorized":true,"path":null},"requests":{},"sockets":{},"freeSockets":{},"keepAliveMsecs":1000,"keepAlive":false,"maxSockets":null,"maxFreeSockets":256,"scheduling":"lifo","maxTotalSockets":null,"totalSocketCount":0,"maxCachedSessions":0,"_sessionCache":{"map":{},"list":[]}}}
2022-08-05T10:13:25.874Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Axios Request
2022-08-05T10:13:25.924Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Check isImportant
2022-08-05T10:13:25.924Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] sendNotification again: Down Count: 2 | Resend Interval: 2
2022-08-05T10:13:25.925Z [MONITOR] WARN: Monitor #1 '[Resend Noti] Resend every 2 times': Failing: getaddrinfo ENOTFOUND louislam.net2 | Interval: 20 seconds | Type: http | Down Count: 0 | Resend Interval: 2
2022-08-05T10:13:25.926Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Send to socket
2022-08-05T10:13:25.926Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] Store
2022-08-05T10:13:26.004Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] prometheus.update
2022-08-05T10:13:26.004Z [MONITOR] DEBUG: [[Resend Noti] Resend every 2 times] SetTimeout for next check.

The timestamps between the sendNotification messages have exactly 40 seconds in between. This is what I expect.

Is there any setting I missed, that produces this failure? May you have an idea? Please let me know, so I can verify and fix the problem.

Best regards, Christoph

@mabed-fr mabed-fr mentioned this pull request Aug 12, 2022
1 task
@louislam
Copy link
Owner

Oh, I forget to re-run the db patch again. It's ok now.

@louislam louislam merged commit 30b72d8 into louislam:master Aug 13, 2022
@OidaTiftla OidaTiftla deleted the introduce-resend-interval branch August 16, 2022 06:22
@OidaTiftla
Copy link
Contributor Author

Great, thanks a lot for merging this.

@skylarv
Copy link

skylarv commented Feb 8, 2023

@louislam Shouldn't "consequently" be "consecutively". Seems like a typo?

@louislam
Copy link
Owner

louislam commented Feb 9, 2023

@louislam Shouldn't "consequently" be "consecutively". Seems like a typo?

You are right, I messed up both words.

@OidaTiftla
Copy link
Contributor Author

Hi @louislam,

I updated the code in #2747 you may have a look.

Best regards.

@chakflying chakflying mentioned this pull request Jul 30, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants