Sorting alerts by group name in /alerts #5448

pbhudiaBAE · 2019-04-10T13:02:59Z

As mentioned in #1371 and #3579, this is a draft pull request for sorting alerts by group name on the alerts page. As it stands I need some help sorting the groups alphabetically, when the page is refreshed the groups shuffle position.

juliusv · 2019-04-11T09:47:11Z

rules/manager.go

+	Groups []*AlertingGroup
+}
+
+type AlertingGroup struct {


Hmm, I think it might be better to just return the existing rule.Group structs here and add Name() and AlertingRules() methods on that type directly.

juliusv · 2019-04-11T09:49:34Z

rules/manager.go

+	return alerts
+}
+
+type AlertingGroups struct {


I'm not sure we need this intermediary type. Just return a slice of []*rule.Group directly from the method that returns the rule groups?

juliusv · 2019-04-11T09:50:54Z

rules/manager.go

+type AlertingGroup struct {
+	Name                 string
+	Rules                []*AlertingRule
+	AlertStateToRowClass map[AlertState]string


This map is only relevant for display purposes and shouldn't live in the rule code or be attached to every individual group. Best to keep it part of the top-level data value that is passed into the HTML template.

juliusv · 2019-04-11T09:52:39Z

web/web.go

-
-	alertStatus := AlertStatus{
-		AlertingRules: alertsSorter.alerts,
-		AlertStateToRowClass: map[rules.AlertState]string{


You'll probably want to keep the AlertStatus struct as the top-level value that is passed into the HTML template, but change the type to take alerting groups instead of individual alerting rules. In any case, AlertStateToRowClass should only be instantiated once, in this top-level field of that struct.

squat · 2019-04-15T14:44:05Z

rules/manager.go

@@ -362,6 +362,38 @@ func (g *Group) hash() uint64 {
 	return l.Hash()
 }

+// AlertingRules returns the list of the group's alerting rules


tiny nit: for consistency, godoc comments should be full sentences ending with a .

pbhudiaBAE · 2019-04-15T16:54:34Z

I think I've addressed the comments from @juliusv let me know if I've missed something or you want something changing. The "Fixed group sorting and comments" commit didn't pass the checks because I didn't run gofmt on manager.go and web.go. The new "Fixed group sorting and comments with gofmt" commit passes the checks.

I've also changed how the RuleGroups() method sorts groups, before if two groups had the same name then refreshing the page would occasionally shuffle the group, so I added code to check if the group name is the same then sort by file name so that the Group position is more likely to be fixed.

I welcome any feedback/ suggestions 👍

juliusv · 2019-04-18T08:50:19Z

rules/manager.go

@@ -871,7 +903,7 @@ func (m *Manager) RuleGroups() []*Group {

 	// Sort rule groups by file, then by name.
 	sort.Slice(rgs, func(i, j int) bool {
-		if rgs[i].file != rgs[j].file {
+		if rgs[i].name == rgs[j].name {


It kinda seems odd to sort by file as if it's lower in the file->group->rule hierarchy than the group... how about always sorting by file first, then by group name, but surfacing the file name in the group header somehow (maybe {{.Name}} ({{.File}}) instead of just {{.Name}})? I don't think we should introduce a new dropdown level in the UI for just the file, but making it visible in an unobtrusive way could be good and make the file->group sorting behavior less confusing for users. Maybe even make the group name grey/italic/small or something, so it pops out less, but is still there?

Hmm, your screenshots and updates say that the ordering now happens by file first, but the code here still looks like it's sorting by name first?

Ah, I think a code push is missing?

juliusv · 2019-04-18T08:50:33Z

Looks great besides the one comment, thanks :)

pbhudiaBAE · 2019-04-18T12:58:16Z

// Sort rule groups by file, then by name.
sort.Slice(rgs, func(i, j int) bool {
	if rgs[i].file != rgs[j].file {
		return rgs[i].file < rgs[j].file
	}
	return rgs[i].name < rgs[j].name
})
return rgs
}

You mean keep the sorting of groups like how it was before (above)?

I changed the File method on the Group struct to return filepath.Base(g.file) instead of g.file (so you only get the file name and not the whole path) and made the group name italic in the HTML.

Here's a screenshot of it so far.

Some suggestions for styling:

Move the File Name to the very end of the Group Header row.
Make the Group Header row thinner than the normal alert rows.
Make the Group header text size smaller than the alert text size.

pbhudiaBAE · 2019-04-18T16:07:29Z

Here's a view with setting the Group Header row padding to 2px, which style looks better to you @juliusv. Should there be a label like Group:(Group Name) and File:(File Name) in the row so users know what they mean?

pbhudiaBAE · 2019-04-29T10:41:51Z

@juliusv I swapped the File and Group name since it is ordered by File name first, and made the group name smaller so it is still visible. If there aren't any more changes you would like I can make the commit and make the pull request an Open one.

juliusv · 2019-05-08T20:01:15Z

Gah, so sorry for the delay again! Taking another look now!

juliusv · 2019-05-08T20:08:43Z

Ah ok, was confused by the code not being there yet, but you said "I can make the commit", sorry :)

Regarding the formatting I might find something like "foo-rules.yml > alerting" more visually pleasing than the more explicit "File: foo-rules.yml Group: alerting" (maybe in that case, even keep it the same font). Otherwise, great.

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

pbhudiaBAE · 2019-05-14T09:17:21Z

@juliusv The latest commit changes the formatting to File Name > Group Name.

juliusv · 2019-05-14T12:26:45Z

rules/manager.go

@@ -267,7 +268,7 @@ func NewGroup(name, file string, interval time.Duration, rules []Rule, shouldRes
 func (g *Group) Name() string { return g.name }

 // File returns the group's file.
-func (g *Group) File() string { return g.file }
+func (g *Group) File() string { return filepath.Base(g.file) }


I think it's ok to preserve the full path here (this function is already used by /rules as well, where it already shows the whole path, see e.g. http://demo.robustperception.io:9090/rules).

juliusv · 2019-05-14T12:27:09Z

Looks good besides the last comment, thanks! Could you mark the PR as ready for review as well?

juliusv · 2019-05-14T12:27:49Z

(and as for what exactly we display how, I might still go through some iterations / tweaks of it after having experienced it for a while :))

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

juliusv · 2019-05-14T21:14:21Z

I allowed myself to mark the PR as ready for review and will merge - thank you :)

sirikhum · 2019-07-31T06:27:12Z

Is there any configurable parameter to have /alerts sorted by status as in the 2.9.x?

pbhudiaBAE mentioned this pull request Apr 10, 2019

Improve display of alerts in /alerts #1371

Closed

juliusv reviewed Apr 11, 2019

View reviewed changes

squat reviewed Apr 15, 2019

View reviewed changes

juliusv reviewed Apr 18, 2019

View reviewed changes

pbhudiaBAE added 6 commits May 9, 2019 09:52

Working group name

5d57868

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

Working categorised by group name

b8e77bc

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

Changed group sorting in web

341a682

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

Fixed group sorting and comments

8a5fcf7

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

Fixed group sorting and comments with gofmt

6a51154

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

Added file and group name

e7f9fb8

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

juliusv reviewed May 14, 2019

View reviewed changes

reverted back to full path to yml file

341baa8

Signed-off-by: Pritam Bhudia <pritam.bhudia@baesystems.com>

juliusv marked this pull request as ready for review May 14, 2019 21:13

juliusv merged commit 43953b1 into prometheus:master May 14, 2019

pbhudiaBAE mentioned this pull request May 28, 2019

[Feature request] Group alerts by group name on /alerts #3579

Closed

kustodian mentioned this pull request Jun 12, 2019

Display active alerts on top #5655

Closed

simonpasquier mentioned this pull request Jul 30, 2019

Prometheus dashboard UI Bug: Shows the path to the alerting rules on top of each alerting rule #5809

Closed

IntiGabriel mentioned this pull request Sep 19, 2019

Add toggle to hide non active alerts coliquio/prometheus#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sorting alerts by group name in /alerts #5448

Sorting alerts by group name in /alerts #5448

pbhudiaBAE commented Apr 10, 2019

juliusv Apr 11, 2019

juliusv Apr 11, 2019

juliusv Apr 11, 2019

juliusv Apr 11, 2019

squat Apr 15, 2019

pbhudiaBAE Apr 15, 2019

pbhudiaBAE commented Apr 15, 2019

juliusv Apr 18, 2019

juliusv May 8, 2019

juliusv May 8, 2019

juliusv commented Apr 18, 2019

pbhudiaBAE commented Apr 18, 2019 •

edited

pbhudiaBAE commented Apr 18, 2019

pbhudiaBAE commented Apr 29, 2019 •

edited

juliusv commented May 8, 2019

juliusv commented May 8, 2019

pbhudiaBAE commented May 14, 2019

juliusv May 14, 2019

juliusv commented May 14, 2019

juliusv commented May 14, 2019

juliusv commented May 14, 2019

sirikhum commented Jul 31, 2019

Sorting alerts by group name in /alerts #5448

Sorting alerts by group name in /alerts #5448

Conversation

pbhudiaBAE commented Apr 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbhudiaBAE commented Apr 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusv commented Apr 18, 2019

pbhudiaBAE commented Apr 18, 2019 • edited

pbhudiaBAE commented Apr 18, 2019

pbhudiaBAE commented Apr 29, 2019 • edited

juliusv commented May 8, 2019

juliusv commented May 8, 2019

pbhudiaBAE commented May 14, 2019

Choose a reason for hiding this comment

juliusv commented May 14, 2019

juliusv commented May 14, 2019

juliusv commented May 14, 2019

sirikhum commented Jul 31, 2019

pbhudiaBAE commented Apr 18, 2019 •

edited

pbhudiaBAE commented Apr 29, 2019 •

edited