Feature/Severity status #487

bestwebua · 2023-05-22T13:09:19Z

Summary

This PR has been inspired by: #227, #412

Screenshorts

Checklist

Tested and/or added tests to validate that the changes work as intended, if applicable.
Updated documentation in README.md, if applicable.

README.md

bestwebua · 2023-06-27T15:31:43Z

This PR covers "Support severity for PagerDuty" feature: #412

Exagone313 · 2023-07-15T20:38:28Z

I am interested in this feature so I will make a review and propose some changes.

Please note that I am not a maintainer and my views may different than TwiN's.

Exagone313

I haven't finished my review, I still have some things to try out.

Here are additional things I couldn't add to my review (if you know how to prepare comments in a review, let me know how, I'm more used to GitLab on that):

issue: Can you rebase you PR? Both commits require a frontend rebuild (those are the only conflicts).

issue: The GitLab alerting provider was added recently, is it possible to add support for sending the severity on this provider? Currently, there is a alerting.gitlab.severity parameter, but it should be removed and replaced by the severity from this feature. (Need more input from @TwiN about this)

can be one of critical, high, medium, low, info, unknown

suggestion: It could be possible, for each alerting provider, to disable alerts if severity doesn't reach a minimal threshold (by default, the least critical severity). This can be done in a future issue.

I think I will work on this.

question/suggestion: I don't understand why both severity and severity_status exist in results.

I think the endpoint_result_conditions table should have a new column severity with the level used when success is false. Then the endpoint_results table can have a single severity column.

The Severity struct can then be removed.

I will try to make this improvement.

thought: I'm wondering if the type for a condition could be something else than a string (while keeping it possibly a string), e.g.:

conditions:
  - condition: "[RESPONSE_TIME] < 1s"
    severity: high
  - "[CERTIFICATE_EXPIRATION] > 72h" # critical severity (the default)

I don't know if Go permits this, so I will have to try.

thought(non-blocking) : It doesn't seem to be possible to add multiple conditions that consume the same variables, with different severity. The second time a variable is used, it returns 0. This can be addressed in another issue.

(It was an issue on my end)

Exagone313 · 2023-07-15T20:55:02Z

core/result.go

+type SeverityStatus int
+
+// Severity status enums
+const (


The names for severity levels were suggested previously and seem to be what GitLab uses for alert severity.

suggestion: What about using levels that are more often used by other software, such as PagerDuty (as used here in PagerDuty alerting) or Python (with logging levels):

critical

error (instead of high)

warning (instead of medium)

info (instead of low)

(Note that Python also has a debug level but it doesn't have any meaning here.)

Exagone313 · 2023-07-15T21:09:45Z

storage/store/sql/specific_postgres.go

+ _, _ = s.db.Exec(`
+ ALTER TABLE endpoint_results
+ ADD IF NOT EXISTS domain_expiration BIGINT NOT NULL DEFAULT 0,
+ ADD IF NOT EXISTS severity_status INTEGER NOT NULL DEFAULT 0;


thought: I know this is also the case for domain_expiration, but with this command you end up with a schema that is different than from creating the table with the CREATE TABLE IF NOT EXISTS statement present above, since the rows don't have a DEFAULT attribute in that statement. It is also the case in sqlite. (cc @TwiN)

Exagone313 · 2023-07-15T21:32:05Z

core/result.go

+
+// Severity status enums
+const (
+ None SeverityStatus = iota


suggestion(non-blocking): The default severity (used for database migrations) could be named unknown instead of none.

Exagone313 · 2023-07-15T21:55:12Z

storage/store/sql/specific_sqlite.go

- _, _ = s.db.Exec(`ALTER TABLE endpoint_results ADD domain_expiration INTEGER NOT NULL DEFAULT 0`)
+ _, _ = s.db.Exec(`
+ ALTER TABLE endpoint_results ADD domain_expiration INTEGER NOT NULL DEFAULT 0;
+ ALTER TABLE endpoint_results ADD severity_status INTEGER NOT NULL DEFAULT 0;


issue: If the first ADD statement fails (and it is likely to), the second will not be run (Parse error: duplicate column domain_expiration).

Exagone313 · 2023-07-16T08:19:49Z

core/condition.go

+// Returns separated SeverityStatus and condition
+func (c Condition) sanitizeSeverityCondition() (SeverityStatus, string) {
+ severityStatus, complexCondition := Critical, string(c)
+ regex, _ := regexp.Compile(`(Low|Medium|High|Critical) :: (.+)`)


Exagone313 · 2023-07-16T11:12:09Z

@bestwebua Can we work together? I didn't know you were active and I made most of changes from my review here: https://github.com/Exagone313/gatus/tree/severity-status

A notable change is that I simplified how the severity is computed from the results, there is a field severity on both condition results and results now.

I also made various improvements in the code (for example, the switch were needlessly making boolean comparisons).

I think commits could be squashed, as long as Co-Authored-By lines are added adequately.

bestwebua · 2023-07-16T11:56:53Z

@Exagone313 Of course I'm active. I have received your comments just today. Thanks, let it be. So what the next actions?

Exagone313 · 2023-07-16T12:09:08Z

So what the next actions?

Are you up for a pair programming session on Jitsi (or something like that)? I think it would be best to be able to share some code in real time.

bestwebua · 2023-07-16T13:14:53Z

@Exagone313 Sorry, today I'm not able to pair session. But we can combine our commits in the different branch and make a review.

Exagone313 · 2023-07-16T13:43:16Z

In my branch there are still some issues or improvements to be made:

For now I only tested the volatile storage and database needs to be more thoroughly tested (on both SQLite and PostgreSQL).
I don't save severity for each condition results in the database (I think it is needed).
I haven't added yet support for alert severity level threshold.
I haven't added severity support for GitLab provider and other alerting providers that could support it.

* Added ConditionResult.Severity * Added Result.Severity * Added Result.SeverityStatus * Added Result#determineSeverityStatus, tests * Added SeverityStatus type * Added Condition#sanitizeSeverityCondition, tests * Updated Condition#evaluate, tests * Updated Endpoint#EvaluateHealth, tests * Updated storage adapters * Updated Endpoint component (vue) * Recompiled assets * Updated readme * Added critical severity, add result severity to pagerduty

ser · 2024-04-27T11:08:38Z

is there any chance of progress of this pull request or it's abandoned? This feature is essential for real world monitoring system.

Exagone313 · 2024-04-27T20:05:02Z

I was thinking about taking over the PR but I have been working on other things, I may tackle it again in a few weeks though.

TwiN requested changes May 28, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

bestwebua force-pushed the feature/serverity-status branch 3 times, most recently from 8f3471c to 97e431f Compare June 13, 2023 20:40

bestwebua requested a review from TwiN June 13, 2023 20:40

bestwebua force-pushed the feature/serverity-status branch 4 times, most recently from 5fe383a to 161bf83 Compare June 22, 2023 06:21

skhokhlov reviewed Jun 27, 2023

View reviewed changes

README.md Show resolved Hide resolved

bestwebua requested a review from skhokhlov June 27, 2023 15:32

bestwebua mentioned this pull request Jun 27, 2023

Support severity for PagerDuty #412

Open

bestwebua force-pushed the feature/serverity-status branch from ad00df2 to 9cf7ac1 Compare June 29, 2023 09:13

Exagone313 suggested changes Jul 15, 2023

View reviewed changes

Exagone313 reviewed Jul 16, 2023

View reviewed changes

bestwebua force-pushed the feature/serverity-status branch from 9cf7ac1 to 9d04469 Compare July 16, 2023 10:46

bestwebua force-pushed the feature/serverity-status branch from 9d04469 to a6668b2 Compare October 16, 2023 21:02

bestwebua force-pushed the feature/serverity-status branch from a6668b2 to 945bbe7 Compare October 16, 2023 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/Severity status #487

Feature/Severity status #487

bestwebua commented May 22, 2023 •

edited

bestwebua commented Jun 27, 2023

Exagone313 commented Jul 15, 2023 •

edited

Exagone313 left a comment •

edited

Exagone313 Jul 15, 2023

Exagone313 Jul 15, 2023

Exagone313 Jul 15, 2023

Exagone313 Jul 15, 2023

Exagone313 Jul 16, 2023

Exagone313 commented Jul 16, 2023 •

edited

bestwebua commented Jul 16, 2023 •

edited

Exagone313 commented Jul 16, 2023

bestwebua commented Jul 16, 2023

Exagone313 commented Jul 16, 2023

ser commented Apr 27, 2024

Exagone313 commented Apr 27, 2024

	regex, _ := regexp.Compile(`(Low\|Medium\|High\|Critical) :: (.+)`)
	regex, _ := regexp.Compile(`(?i)(Low\|Medium\|High\|Critical) :: (.+)`)

Feature/Severity status #487

Are you sure you want to change the base?

Feature/Severity status #487

Conversation

bestwebua commented May 22, 2023 • edited

Summary

Screenshorts

Checklist

bestwebua commented Jun 27, 2023

Exagone313 commented Jul 15, 2023 • edited

Exagone313 left a comment • edited

Choose a reason for hiding this comment

Exagone313 Jul 15, 2023

Choose a reason for hiding this comment

Exagone313 Jul 15, 2023

Choose a reason for hiding this comment

Exagone313 Jul 15, 2023

Choose a reason for hiding this comment

Exagone313 Jul 15, 2023

Choose a reason for hiding this comment

Exagone313 Jul 16, 2023

Choose a reason for hiding this comment

Exagone313 commented Jul 16, 2023 • edited

bestwebua commented Jul 16, 2023 • edited

Exagone313 commented Jul 16, 2023

bestwebua commented Jul 16, 2023

Exagone313 commented Jul 16, 2023

ser commented Apr 27, 2024

Exagone313 commented Apr 27, 2024

bestwebua commented May 22, 2023 •

edited

Exagone313 commented Jul 15, 2023 •

edited

Exagone313 left a comment •

edited

Exagone313 commented Jul 16, 2023 •

edited

bestwebua commented Jul 16, 2023 •

edited