-
Notifications
You must be signed in to change notification settings - Fork 491
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.
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
cmd/bosun: (wip) link previous incidents and next incident #2323
Conversation
TODO:
|
example for the template func:
|
Fancier table with col/rowspan (note to self: if making into example check for nil on incident End time just in case):
|
slog.Infoln("Running population of previous incidents. This can take several minutes.") | ||
|
||
// Hacky Work better? | ||
ids, err := d.getAllIncidentIdsByKeys() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly collect Alert Keys up front, and run inner loop once per alert key, not once per incident.
cmd/bosun/database/migrate.go
Outdated
return err | ||
} | ||
previousIds := []int64{} | ||
previousIds, err = d.State().GetAllIncidentIdsByAlertKey(incident.AlertKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least cache these results by alert key.
} | ||
for _, id := range previousIds { | ||
if incident.Id > id { | ||
incident.PreviousIds = append(incident.PreviousIds, id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, we're storing all previous alert keys on every alert? Gross. Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@captncraig To make it easier to either:
- Display more than just the previous incident in a view
- Make it easier to iterate over all previous incidents in templates without having to follow a linked list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also to clarify not the previous alert keys, just an array of incident id numbers (int64)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another Also :P alert keys are the alert name plus the tagset (not all alerts under the name, unless the tagset is {}
or of len 1. So the list generally should not be crazy long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'm just uncomfortable anytime I see potentially large, and also redundant, and also repetitive collections or lists.
I guess though I'd rather a template could show them all without having to recursively traverse the chain, which was gonna be the alternative I suggested. So ok.
c337957
to
ab73269
Compare
You should back up your redis db before this Migration operation. This will update all incidents in redis/ledis, bosun will not start until this one-time operation is complete. ~100k incidents took two minutes with redis on a development workstation. Add GetIncidentState template function, as well as PreviousIds field to templates.
ab73269
to
4ae4972
Compare
requires a migration as all of the IncidentState objects are updated.
This makes it so when viewing incidents you can quickly navigate to previous incidents
or the next incident if there is a new one for the same alert key.