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.

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

Added extra info to unknown templates #2269

Merged

Conversation

@pradeepbbl
Copy link
Contributor

@pradeepbbl pradeepbbl commented Jun 18, 2018

Currently, we have a very limited number of variable available to the unknown templates which prevent us to match the alert and action notification template (includes incident states) and sometimes unknowns are getting unnoticed due inconsistency (when using external notification systems e.g. pagerduty).

With this patch, I tried to include the alert states to unknown which is very useful when grouping disable using unknownMinGroupSize.

Please let me know your thoughts.

Pradeep Mishra
 - conf/unknownNotify.go: added incident states to unknown context and a new func `IncidentUnknownLink` to build unknown incident link.
 - sched/notify.go: modified `sendUnknownNotifications` func to pass incident states.
 - docs/definitions.md: unknown template document update
@pradeepbbl pradeepbbl force-pushed the bookingcom:fix_unknown_notification branch from 934df2d to 73e14ed Jun 20, 2018
@pradeepbbl
Copy link
Contributor Author

@pradeepbbl pradeepbbl commented Jul 2, 2018

here is an example of a PagerDuty alert body template

pdMonitoringBody = `{
       "service_key": "XXXXXXXXXXXXXX",
       "event_type": "trigger",
       "description": "{{.Subject | jsonStr}}",
       "client": "bosun",
       "client_url": "{{.Incident}}",
       "incident_key": "AlertName={{.Alert.Name}};IncidentId={{.Id}}"
 }` 

if you make close attention to incident_key we use alert name and incident id combination to reduce duplication. With current unknown template contexts/var, it's impossible for us to prepare an unknown body that matches the above specification and due to this many unknowns are getting unnoticed by teams.

With this patch, we are able to produce the unknown body and post to PagerDuty successfully

pdMonitoringUnknownBody = `{
       "service_key": "XXXXXXXXXXXXX",
       "event_type": "trigger",
       "description": "Alert goes to unknown states",
       "client": "bosun",
       "client_url": "{{ .IncidentUnknownLink .States.Id }}",
       "incident_key": "AlertName={{.Name}};IncidentId={{.States.Id}}"
}`

@kylebrandt / @captncraig could you please let us know your thoughts?

@pradeepbbl
Copy link
Contributor Author

@pradeepbbl pradeepbbl commented Aug 10, 2018

can this be merged soon :)

@stepashka
Copy link

@stepashka stepashka commented Sep 20, 2018

Hey @kylebrandt, @captncraig!
Any objections to this change? Is there something else we need to implement before this gets merged?
Thanks,
Anna

@kylebrandt kylebrandt merged commit 97302ba into bosun-monitor:master Sep 20, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sipidronov pushed a commit to sipidronov/bosun that referenced this pull request Sep 21, 2018
- conf/unknownNotify.go: added incident states to unknown context and a new func `IncidentUnknownLink` to build unknown incident link.
 - sched/notify.go: modified `sendUnknownNotifications` func to pass incident states.
 - docs/definitions.md: unknown template document update
@pradeepbbl pradeepbbl deleted the bookingcom:fix_unknown_notification branch Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants