-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix repeat flag doesn't terminate monika on fail probe #1165
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1165 +/- ##
==========================================
- Coverage 61.88% 61.06% -0.82%
==========================================
Files 105 106 +1
Lines 3130 3136 +6
Branches 536 535 -1
==========================================
- Hits 1937 1915 -22
- Misses 1015 1035 +20
- Partials 178 186 +8
... and 2 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked for me. Ready to merge.
Monika Pull Request (PR)
What feature/issue does this PR add
How did you implement / how did you fix it
There are several bugs that causes the issue #1164
The
setProbeRunning
is called almost every second in this line https://github.com/hyperjumptech/monika/pull/1165/files#diff-c3cb2be3e6fbad9e8455e2fc282a2d52cc7875688162e517023bff5be26371b6L54. The fix: moved it back down as originally written.The
retry
function is called withoutmaxAttempt
here https://github.com/hyperjumptech/monika/pull/1165/files#diff-c3cb2be3e6fbad9e8455e2fc282a2d52cc7875688162e517023bff5be26371b6R81. This causes Monika to keep retrying to monitor the probe in a cycle which causes the bug in Repeat 1 doesn't exit when the probe is not accessible #1164. The fix: addedmaxAttempt
with value whichever bigger betweenincidentThreshold
andrecoveryThreshold
because we want to retry for both incident and recovery cases. See below for more information.Removal of
recoveryThreshold
causes the recovery notification to be sent immediately upon recovery. But we don't want that. Just like we don't want to immediately send incident notification, Monika needs to retry several times first before sending the recovery notification. The fix:recoveryThreshold
and remove the deprecation warning https://github.com/hyperjumptech/monika/pull/1165/files#diff-028bd48d9206d28afc908d305ef5ac57026ad51ba014a0741fb8abcaef2af741R113. I think there was misunderstanding, we just don't want user to be able to set the threshold from symon, but we still need it in Monika.recoveryThreshold
. https://github.com/hyperjumptech/monika/pull/1165/files#diff-537950d0505c20b7b3ec67590102fdafdbbbbddb0130d5dbf7a4ac232060598dR152Some further changes to improve performance:
retryInitialDelayMs
value to 2 seconds. 128 milliseconds is not a sensible default. There's no need for Monika to retry that fast. https://github.com/hyperjumptech/monika/pull/1165/files#diff-06df4a808fa037b48c082497d352cb7825c883648d52a17ab72b0e7dd70a5c5dR84Probing Mechanism
With this PR, I also fixed the probing mechanism. This is how it works.
Say we have the following probe:
Then say the server in the localhost:8000 is offline (unreachable). The process will go like the following:
The same process is also executed when the probe can reached but the assertion fails which in this case when response's time is greater than 800 ms.
When the
-r
flag is provided, Monika will perform as many cycles as defined by the repeat flag value. However, it will also still do retries in case of incident and recovery as defined by the probes'incidentThreshold
andrecoveryThreshold
if provided, or use their respective default values.How to test
npm run start -- -c ./monika.yml -r 2
Screenshots
Running with
-r 1
and the probe cannot be reached (offline):Running with
-r 2
and the probe cannot be reached (offline):Running with
-r 1
and the probe can be reached (online):Running without
-r
flag (continuously):