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.

Sign up for GitHub

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

Jump to bottom

feat: add support for alerting on absent metric #3245

Merged
merged 20 commits into from
Mar 13, 2024
Merged

feat: add support for alerting on absent metric #3245

merged 20 commits into from
Mar 13, 2024

Conversation

srikanthccv
Copy link
Member

Part of #1463

@github-actions github-actions bot added the enhancement New feature or request label Aug 1, 2023
@srikanthccv srikanthccv marked this pull request as ready for review September 7, 2023 03:14
prashant-shahi
prashant-shahi previously requested changes Sep 8, 2023
View reviewed changes
Copy link
Member

@prashant-shahi prashant-shahi left a comment

Choose a reason for hiding this comment

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

Requesting changes below:

  • The metric absent section should be below the threshold section.
  • Duration of metrics absent should not accept a negative number.

Screenshot:

Screenshot

@srikanthccv
Copy link
Member Author

I will fix them. Let me know if there are any other functional issues.

@srikanthccv srikanthccv mentioned this pull request Dec 19, 2023
Closed
ankitnayan
ankitnayan reviewed Dec 29, 2023
View reviewed changes
pkg/query-service/rules/alerting.go Outdated Show resolved Hide resolved
pkg/query-service/rules/thresholdRule.go Outdated Show resolved Hide resolved
pkg/query-service/rules/resultTypes.go Show resolved Hide resolved
pkg/query-service/rules/thresholdRule.go Show resolved Hide resolved
@srikanthccv
Copy link
Member Author

While this is helpful, it has certain limitations. It won't be useful when there is partial data for metrics. For instance, take host metrics, a user wants to get alerted when any host stops sending data. They would have to create one alert for each host which is not practical. The common way to achieve this is to use two queries one with last for 10 minutes back and one with a current window and alert. That would be possible when we have support for timeshift function.

@ankitnayan
Copy link
Collaborator

please add tests with some edge/different cases

@srikanthccv srikanthccv dismissed prashant-shahi’s stale review March 13, 2024 12:07

requested changes are addressed

@srikanthccv srikanthccv merged commit 3c41967 into develop Mar 13, 2024
11 checks passed
@srikanthccv srikanthccv deleted the absent-metric branch March 13, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YounixM YounixM YounixM approved these changes

@ankitnayan ankitnayan ankitnayan approved these changes

@palashgdev palashgdev Awaiting requested review from palashgdev

@prashant-shahi prashant-shahi Awaiting requested review from prashant-shahi

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@srikanthccv @ankitnayan @YounixM @prashant-shahi