add groupKey to alerts/groups API endpoint #576
Conversation
dispatch/dispatch.go
Outdated
| alertGroup, ok := seen[ag.fingerprint()] | ||
| if !ok { | ||
| alertGroup = &AlertGroup{Labels: ag.labels} | ||
| alertGroup.GroupKey = uint64(ag.labels.Fingerprint()) |
There was a problem hiding this comment.
It's actually generated differently: https://github.com/prometheus/alertmanager/blob/master/dispatch/dispatch.go#L332.
Probably best to at a GroupKey() uint64 method to the aggregation group to avoid getting out of sync.
…tion for context initialization
|
Updated the PR as proposed in the comment |
| } | ||
|
|
||
| func (ag *aggrGroup) GroupKey() uint64 { | ||
| return uint64(ag.labels.Fingerprint() ^ ag.routeFP) |
There was a problem hiding this comment.
Maybe I'm missing something, but we can just keep the Fingerprint type here all the way no? Then we don't have to do any casting along the way.
There was a problem hiding this comment.
To be honest, I dislike the Fingerprint type with a passion – algorithm as well as naming. I'd like to get rid of it everywhere at some point and would rather not propagate it further.
There was a problem hiding this comment.
I was thinking the same about the Fingerprint, but sticked with the grouopKey as it was already in the webhook. This is up to you guys :)
There was a problem hiding this comment.
Not sticking to the "Fingerprint" type is fine by me as well as long as we stick with the same type. But in the context of this change I guess the way it is is fine then. Nevermind me here :)
|
LGTM 👍 |
Discussion started at #574
Add the
groupKeyinformation to thealerts/groupsendpoint.When having configured a webhook receiver it contains the
groupKey. Adding thegroupKeyto the overview of alertgroups makes it possible to match the incoming webhook with groups reported back from the API.