Merge ~xavpaice/charm-nagios:pagerduty-defaults into ~nagios-charmers/charm-nagios:master

Proposed by Xav Paice
Status: Merged
Approved by: Jeremy Lounder
Approved revision: f392fd53ced90e46d7e449532942f7ebfa287c75
Merged at revision: 9aa1d8465582cb44e65e4b227ae3c9c83d063132
Proposed branch: ~xavpaice/charm-nagios:pagerduty-defaults
Merge into: ~nagios-charmers/charm-nagios:master
Diff against target: 19 lines (+2/-3)
1 file modified
config.yaml (+2/-3)
Reviewer Review Type Date Requested Status
Jeremy Lounder (community) +1 Approve
James Hebden (community) Approve
Review via email: mp+369496@code.launchpad.net

Commit message

Change pagerduty_notification_levels default

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
James Hebden (ec0) wrote :

Approving from a technical perspective, and from a usage perspective.

For two reasons I believe not sending warnings to PagerDuty makes sense as a default:
- Paging via PagerDuty for warnings is orthogonal to the meaning and intention of warnings in Nagios, which are intended for reflecting opportunities for proactive work to prevent future incidents.
- With the unidirectional integration we currently use for PagerDuty in the charm, warnings are not created with low-priority associated with them in PagerDuty, instead being given the exact same characteristics as a critical-initiated page. This leads to pager spam of most heinous proportions.

One additional idea would be to also implement support in the bundled integration to map warning pages to low-importance pages in PagerDuty. We currently lack this support, so for now, this MP gets a +1 from me.

I have also raised https://bugs.launchpad.net/nagios-charm/+bug/1834818 to cover the mapping of Nagios warnings to low-urgency PagerDuty events as a wishlist item.

review: Approve
Revision history for this message
Jeremy Lounder (jldev) wrote :

This is a recent enough configuration option, I believe it is acceptable to change the default.

review: Approve (+1)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 9aa1d8465582cb44e65e4b227ae3c9c83d063132

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/config.yaml b/config.yaml
index 5aff8c9..cc7102b 100644
--- a/config.yaml
+++ b/config.yaml
@@ -171,12 +171,11 @@ options:
171 to report warning at 5.0, 4.0 and 3.0 averages, critical at 10.0, 171 to report warning at 5.0, 4.0 and 3.0 averages, critical at 10.0,
172 6.0 and 4.0.172 6.0 and 4.0.
173 pagerduty_notification_levels:173 pagerduty_notification_levels:
174 default: w,u,c,r174 default: u,c,r
175 type: string175 type: string
176 description: |176 description: |
177 A string to use for the service_notification_options in the177 A string to use for the service_notification_options in the
178 pagerduty contact configuration. Remove w to avoid paging for178 pagerduty contact configuration. Add w to page for warning events.
179 warning events.
180 check_timeout:179 check_timeout:
181 default: 10180 default: 10
182 type: int181 type: int

Subscribers

People subscribed via source and target branches