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(observability-lib): add timerange to alert rule #979

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(observability-lib): add timerange to alert rule #979

wants to merge 1 commit into from

Conversation

Atrax1
Copy link
Collaborator

@Atrax1 Atrax1 commented Dec 20, 2024

  • Can delete rule with cmd
  • Can specify timerange within alert rule query
Query: []grafana.RuleQuery{
	{
		Expr:       `metric_name`,
		RefID:      "A",
		Datasource: datasource,
		TimeRange:  900, // 15 mins
	},
},

Requires

Supports

@Atrax1 Atrax1 self-assigned this Dec 20, 2024
@Atrax1 Atrax1 requested review from patrickhuie19 and a team December 20, 2024 13:18
@Atrax1 Atrax1 requested review from jmank88 and patrickhuie19 and removed request for patrickhuie19 January 6, 2025 11:48
patrickhuie19
patrickhuie19 reviewed Jan 7, 2025
View reviewed changes
observability-lib/cmd/api/rule/delete.go
return errDelete
}

return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

/nit

some type of success log/confirmation including the resource deleted may be helpful for the user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickhuie19 patrickhuie19 patrickhuie19 approved these changes

@leeyikjiun leeyikjiun leeyikjiun approved these changes

@jmank88 jmank88 Awaiting requested review from jmank88

At least 2 approving reviews are required to merge this pull request.

Assignees

@Atrax1 Atrax1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Atrax1 @leeyikjiun @patrickhuie19