Skip to content

Conversation

@trueleo
Copy link
Contributor

@trueleo trueleo commented Oct 29, 2022

Description

Implement alert state such that webhooks can be called on both alert trigger and alert resolution. Timeouts are also added to avoid flooding targets ( this is opt in rather than opt out because it adds overhead of state check and timer spawn mechanism ). Numeric rule and String rule are used through Consecutive* rule which takes repeat value as 1 if no repeat value is given.

ref to #184


This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

Implement alert state such that webhooks can be called on both
alert trigger and alert resolution. Timeouts are also added to avoid
flooding targets ( this is opt in rather than opt out because it adds overhead
of state check and timer spawn mechanism ). Numeric rule and String rule are
used through Consecutive* rule which takes repeat value as 1 if no repeat value
is given.
Copy link
Member

@nitisht nitisht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, looking great!

Few small changes we need

  1. Split the rule field into a type and config field. It is easy to understand for the user. We can later add more types like query-based, status-based etc.
	"rule": {
		"type": "column-based", // we'll add more types later
		"config": {
                      "column": "status",
                      "operator": ">=",
                      "value": 500,
                      "repeats": 3
		}
	},
  1. Under targets, let's rename timeout --> repeat. server_url --> url.
  2. Add validation for repeat field inside the target. If it is not provided, no issues, but if a junk value is sent inside repeat field, server should fail the set alert API.

@nitisht nitisht merged commit 5aeb1cd into parseablehq:main Oct 30, 2022
@nitisht nitisht mentioned this pull request Oct 30, 2022
3 tasks
@trueleo trueleo deleted the alert_cons branch October 30, 2022 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants