From 821614fb431af195d6bd23055272292f42cf68d9 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Tue, 13 Dec 2022 19:50:24 -0600 Subject: [PATCH] Alerting: Align notifier truncation and logging with prometheus/alertmanager (#59339) * Move truncation code to util to mirror upstream * Resolve merge conflicts * Align logging of alert key * Update tests and fix field passing bug * Remove superfluous newline in test now that we trim whitespace * Uptake minor log changes from upstream --- .../ngalert/notifier/channels/opsgenie.go | 6 ++- .../ngalert/notifier/channels/pagerduty.go | 12 +++-- .../ngalert/notifier/channels/pushover.go | 41 +++++++++++++-- .../notifier/channels/pushover_test.go | 4 +- .../ngalert/notifier/channels/slack.go | 17 ++++++- .../ngalert/notifier/channels/telegram.go | 11 +++- .../ngalert/notifier/channels/util.go | 50 +++++++++++++++++++ .../ngalert/notifier/channels/victorops.go | 10 +++- .../ngalert/notifier/channels/webex.go | 35 ------------- .../alerting/api_notification_channel_test.go | 2 +- 10 files changed, 136 insertions(+), 52 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels/opsgenie.go b/pkg/services/ngalert/notifier/channels/opsgenie.go index bb45abce242..f88a290217b 100644 --- a/pkg/services/ngalert/notifier/channels/opsgenie.go +++ b/pkg/services/ngalert/notifier/channels/opsgenie.go @@ -25,6 +25,8 @@ const ( OpsgenieSendTags = "tags" OpsgenieSendDetails = "details" OpsgenieSendBoth = "both" + // https://docs.opsgenie.com/docs/alert-api - 130 characters meaning runes. + opsGenieMaxMessageLenRunes = 130 ) var ( @@ -203,9 +205,9 @@ func (on *OpsgenieNotifier) buildOpsgenieMessage(ctx context.Context, alerts mod var tmplErr error tmpl, data := TmplText(ctx, on.tmpl, as, on.log, &tmplErr) - message, truncated := notify.Truncate(tmpl(on.settings.Message), 130) + message, truncated := TruncateInRunes(tmpl(on.settings.Message), opsGenieMaxMessageLenRunes) if truncated { - on.log.Debug("Truncated message", "originalMessage", message) + on.log.Warn("Truncated message", "alert", key, "max_runes", opsGenieMaxMessageLenRunes) } description := tmpl(on.settings.Description) diff --git a/pkg/services/ngalert/notifier/channels/pagerduty.go b/pkg/services/ngalert/notifier/channels/pagerduty.go index 7d69b35634e..ee90a20726b 100644 --- a/pkg/services/ngalert/notifier/channels/pagerduty.go +++ b/pkg/services/ngalert/notifier/channels/pagerduty.go @@ -19,6 +19,11 @@ import ( "github.com/grafana/grafana/pkg/services/notifications" ) +const ( + // https://developer.pagerduty.com/docs/ZG9jOjExMDI5NTgx-send-an-alert-event - 1024 characters or runes. + pagerDutyMaxV2SummaryLenRunes = 1024 +) + const ( pagerDutyEventTrigger = "trigger" pagerDutyEventResolve = "resolve" @@ -236,10 +241,11 @@ func (pn *PagerdutyNotifier) buildPagerdutyMessage(ctx context.Context, alerts m }, as...) - if summary, truncated := notify.Truncate(msg.Payload.Summary, 1024); truncated { - pn.log.Debug("Truncated summary", "original", msg.Payload.Summary) - msg.Payload.Summary = summary + summary, truncated := TruncateInRunes(msg.Payload.Summary, pagerDutyMaxV2SummaryLenRunes) + if truncated { + pn.log.Warn("Truncated summary", "key", key, "runes", pagerDutyMaxV2SummaryLenRunes) } + msg.Payload.Summary = summary if tmplErr != nil { pn.log.Warn("failed to template PagerDuty message", "error", tmplErr.Error()) diff --git a/pkg/services/ngalert/notifier/channels/pushover.go b/pkg/services/ngalert/notifier/channels/pushover.go index 293d537e783..06b4b924cf8 100644 --- a/pkg/services/ngalert/notifier/channels/pushover.go +++ b/pkg/services/ngalert/notifier/channels/pushover.go @@ -10,7 +10,9 @@ import ( "mime/multipart" "os" "strconv" + "strings" + "github.com/prometheus/alertmanager/notify" "github.com/prometheus/alertmanager/template" "github.com/prometheus/alertmanager/types" "github.com/prometheus/common/model" @@ -23,6 +25,12 @@ import ( const ( pushoverMaxFileSize = 1 << 21 // 2MB + // https://pushover.net/api#limits - 250 characters or runes. + pushoverMaxTitleLenRunes = 250 + // https://pushover.net/api#limits - 1024 characters or runes. + pushoverMaxMessageLenRunes = 1024 + // https://pushover.net/api#limits - 512 characters or runes. + pushoverMaxURLLenRunes = 512 ) var ( @@ -184,6 +192,11 @@ func (pn *PushoverNotifier) SendResolved() bool { } func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Alert) (map[string]string, bytes.Buffer, error) { + key, err := notify.ExtractGroupKey(ctx) + if err != nil { + return nil, bytes.Buffer{}, err + } + b := bytes.Buffer{} w := multipart.NewWriter(&b) @@ -206,6 +219,27 @@ func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Al return nil, b, fmt.Errorf("failed to write the token: %w", err) } + title, truncated := TruncateInRunes(tmpl(pn.settings.title), pushoverMaxTitleLenRunes) + if truncated { + pn.log.Warn("Truncated title", "incident", key, "max_runes", pushoverMaxTitleLenRunes) + } + message := tmpl(pn.settings.message) + message, truncated = TruncateInRunes(message, pushoverMaxMessageLenRunes) + if truncated { + pn.log.Warn("Truncated message", "incident", key, "max_runes", pushoverMaxMessageLenRunes) + } + message = strings.TrimSpace(message) + if message == "" { + // Pushover rejects empty messages. + message = "(no details)" + } + + supplementaryURL := joinUrlPath(pn.tmpl.ExternalURL.String(), "/alerting/list", pn.log) + supplementaryURL, truncated = TruncateInRunes(supplementaryURL, pushoverMaxURLLenRunes) + if truncated { + pn.log.Warn("Truncated URL", "incident", key, "max_runes", pushoverMaxURLLenRunes) + } + status := types.Alerts(as...).Status() priority := pn.settings.alertingPriority if status == model.AlertResolved { @@ -231,12 +265,11 @@ func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Al } } - if err := w.WriteField("title", tmpl(pn.settings.title)); err != nil { + if err := w.WriteField("title", title); err != nil { return nil, b, fmt.Errorf("failed to write the title: %w", err) } - ruleURL := joinUrlPath(pn.tmpl.ExternalURL.String(), "/alerting/list", pn.log) - if err := w.WriteField("url", ruleURL); err != nil { + if err := w.WriteField("url", supplementaryURL); err != nil { return nil, b, fmt.Errorf("failed to write the URL: %w", err) } @@ -244,7 +277,7 @@ func (pn *PushoverNotifier) genPushoverBody(ctx context.Context, as ...*types.Al return nil, b, fmt.Errorf("failed to write the URL title: %w", err) } - if err := w.WriteField("message", tmpl(pn.settings.message)); err != nil { + if err := w.WriteField("message", message); err != nil { return nil, b, fmt.Errorf("failed write the message: %w", err) } diff --git a/pkg/services/ngalert/notifier/channels/pushover_test.go b/pkg/services/ngalert/notifier/channels/pushover_test.go index d4a80092ae2..97931bd1063 100644 --- a/pkg/services/ngalert/notifier/channels/pushover_test.go +++ b/pkg/services/ngalert/notifier/channels/pushover_test.go @@ -61,7 +61,7 @@ func TestPushoverNotifier(t *testing.T) { "title": "[FIRING:1] (val1)", "url": "http://localhost/alerting/list", "url_title": "Show alert rule", - "message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", + "message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh", "attachment": "\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\b\x04\x00\x00\x00\xb5\x1c\f\x02\x00\x00\x00\vIDATx\xdacd`\x00\x00\x00\x06\x00\x020\x81\xd0/\x00\x00\x00\x00IEND\xaeB`\x82", "html": "1", }, @@ -90,7 +90,7 @@ func TestPushoverNotifier(t *testing.T) { "title": "Alerts firing: 1", "url": "http://localhost/alerting/list", "url_title": "Show alert rule", - "message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", + "message": "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh", "attachment": "\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\b\x04\x00\x00\x00\xb5\x1c\f\x02\x00\x00\x00\vIDATx\xdacd`\x00\x00\x00\x06\x00\x020\x81\xd0/\x00\x00\x00\x00IEND\xaeB`\x82", "html": "1", }, diff --git a/pkg/services/ngalert/notifier/channels/slack.go b/pkg/services/ngalert/notifier/channels/slack.go index 71cc5440860..df753ec6836 100644 --- a/pkg/services/ngalert/notifier/channels/slack.go +++ b/pkg/services/ngalert/notifier/channels/slack.go @@ -18,6 +18,7 @@ import ( "time" "github.com/prometheus/alertmanager/config" + "github.com/prometheus/alertmanager/notify" "github.com/prometheus/alertmanager/template" "github.com/prometheus/alertmanager/types" @@ -56,6 +57,9 @@ var SlackAPIEndpoint = "https://slack.com/api/chat.postMessage" type sendFunc func(ctx context.Context, req *http.Request, logger log.Logger) (string, error) +// https://api.slack.com/reference/messaging/attachments#legacy_fields - 1024, no units given, assuming runes or characters. +const slackMaxTitleLenRunes = 1024 + // SlackNotifier is responsible for sending // alert notification to Slack. type SlackNotifier struct { @@ -357,6 +361,15 @@ func (sn *SlackNotifier) createSlackMessage(ctx context.Context, alerts []*types ruleURL := joinUrlPath(sn.tmpl.ExternalURL.String(), "/alerting/list", sn.log) + title, truncated := TruncateInRunes(tmpl(sn.settings.Title), slackMaxTitleLenRunes) + if truncated { + key, err := notify.ExtractGroupKey(ctx) + if err != nil { + return nil, err + } + sn.log.Warn("Truncated title", "key", key, "max_runes", slackMaxTitleLenRunes) + } + req := &slackMessage{ Channel: tmpl(sn.settings.Recipient), Username: tmpl(sn.settings.Username), @@ -367,8 +380,8 @@ func (sn *SlackNotifier) createSlackMessage(ctx context.Context, alerts []*types Attachments: []attachment{ { Color: getAlertStatusColor(types.Alerts(alerts...).Status()), - Title: tmpl(sn.settings.Title), - Fallback: tmpl(sn.settings.Title), + Title: title, + Fallback: title, Footer: "Grafana v" + setting.BuildVersion, FooterIcon: FooterIconURL, Ts: time.Now().Unix(), diff --git a/pkg/services/ngalert/notifier/channels/telegram.go b/pkg/services/ngalert/notifier/channels/telegram.go index f495de3c44d..948bf5f5ce4 100644 --- a/pkg/services/ngalert/notifier/channels/telegram.go +++ b/pkg/services/ngalert/notifier/channels/telegram.go @@ -23,6 +23,9 @@ var ( TelegramAPIURL = "https://api.telegram.org/bot%s/%s" ) +// Telegram supports 4096 chars max - from https://limits.tginfo.me/en. +const telegramMaxMessageLenRunes = 4096 + // TelegramNotifier is responsible for sending // alert notifications to Telegram. type TelegramNotifier struct { @@ -161,9 +164,13 @@ func (tn *TelegramNotifier) buildTelegramMessage(ctx context.Context, as []*type tmpl, _ := TmplText(ctx, tn.tmpl, as, tn.log, &tmplErr) // Telegram supports 4096 chars max - messageText, truncated := notify.Truncate(tmpl(tn.settings.Message), 4096) + messageText, truncated := TruncateInRunes(tmpl(tn.settings.Message), telegramMaxMessageLenRunes) if truncated { - tn.log.Warn("Telegram message too long, truncate message", "original_message", tn.settings.Message) + key, err := notify.ExtractGroupKey(ctx) + if err != nil { + return nil, err + } + tn.log.Warn("Truncated message", "alert", key, "max_runes", telegramMaxMessageLenRunes) } m := make(map[string]string) diff --git a/pkg/services/ngalert/notifier/channels/util.go b/pkg/services/ngalert/notifier/channels/util.go index 1149f3b40b7..525fa11acb6 100644 --- a/pkg/services/ngalert/notifier/channels/util.go +++ b/pkg/services/ngalert/notifier/channels/util.go @@ -319,3 +319,53 @@ func splitCommaDelimitedString(str string) []string { } return res } + +// Copied from https://github.com/prometheus/alertmanager/blob/main/notify/util.go, please remove once we're on-par with upstream. +// truncationMarker is the character used to represent a truncation. +const truncationMarker = "…" + +// Copied from https://github.com/prometheus/alertmanager/blob/main/notify/util.go, please remove once we're on-par with upstream. +// TruncateInrunes truncates a string to fit the given size in Runes. +func TruncateInRunes(s string, n int) (string, bool) { + r := []rune(s) + if len(r) <= n { + return s, false + } + + if n <= 3 { + return string(r[:n]), true + } + + return string(r[:n-1]) + truncationMarker, true +} + +// TruncateInBytes truncates a string to fit the given size in Bytes. +// TODO: This is more advanced than the upstream's TruncateInBytes. We should consider upstreaming this, and removing it from here. +func TruncateInBytes(s string, n int) (string, bool) { + // First, measure the string the w/o a to-rune conversion. + if len(s) <= n { + return s, false + } + + // The truncationMarker itself is 3 bytes, we can't return any part of the string when it's less than 3. + if n <= 3 { + switch n { + case 3: + return truncationMarker, true + default: + return strings.Repeat(".", n), true + } + } + + // Now, to ensure we don't butcher the string we need to remove using runes. + r := []rune(s) + truncationTarget := n - 3 + + // Next, let's truncate the runes to the lower possible number. + truncatedRunes := r[:truncationTarget] + for len(string(truncatedRunes)) > truncationTarget { + truncatedRunes = r[:len(truncatedRunes)-1] + } + + return string(truncatedRunes) + truncationMarker, true +} diff --git a/pkg/services/ngalert/notifier/channels/victorops.go b/pkg/services/ngalert/notifier/channels/victorops.go index f8b5066a4cf..d8c11899ade 100644 --- a/pkg/services/ngalert/notifier/channels/victorops.go +++ b/pkg/services/ngalert/notifier/channels/victorops.go @@ -20,6 +20,9 @@ import ( "github.com/grafana/grafana/pkg/setting" ) +// https://help.victorops.com/knowledge-base/incident-fields-glossary/ - 20480 characters. +const victorOpsMaxMessageLenRunes = 20480 + const ( // victoropsAlertStateCritical - Victorops uses "CRITICAL" string to indicate "Alerting" state victoropsAlertStateCritical = "CRITICAL" @@ -116,12 +119,17 @@ func (vn *VictoropsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo return false, err } + stateMessage, truncated := TruncateInRunes(tmpl(vn.settings.Description), victorOpsMaxMessageLenRunes) + if truncated { + vn.log.Warn("Truncated stateMessage", "incident", groupKey, "max_runes", victorOpsMaxMessageLenRunes) + } + bodyJSON := map[string]interface{}{ "message_type": messageType, "entity_id": groupKey.Hash(), "entity_display_name": tmpl(vn.settings.Title), "timestamp": time.Now().Unix(), - "state_message": tmpl(vn.settings.Description), + "state_message": stateMessage, "monitoring_tool": "Grafana v" + setting.BuildVersion, } diff --git a/pkg/services/ngalert/notifier/channels/webex.go b/pkg/services/ngalert/notifier/channels/webex.go index 63121e2f7cb..bc33301c71f 100644 --- a/pkg/services/ngalert/notifier/channels/webex.go +++ b/pkg/services/ngalert/notifier/channels/webex.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "net/url" - "strings" "github.com/prometheus/alertmanager/template" "github.com/prometheus/alertmanager/types" @@ -175,37 +174,3 @@ func (wn *WebexNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, func (wn *WebexNotifier) SendResolved() bool { return !wn.GetDisableResolveMessage() } - -// Copied from https://github.com/prometheus/alertmanager/blob/main/notify/util.go, please remove once we're on-par with upstream. -// truncationMarker is the character used to represent a truncation. -const truncationMarker = "…" - -// TruncateInBytes truncates a string to fit the given size in Bytes. -func TruncateInBytes(s string, n int) (string, bool) { - // First, measure the string the w/o a to-rune conversion. - if len(s) <= n { - return s, false - } - - // The truncationMarker itself is 3 bytes, we can't return any part of the string when it's less than 3. - if n <= 3 { - switch n { - case 3: - return truncationMarker, true - default: - return strings.Repeat(".", n), true - } - } - - // Now, to ensure we don't butcher the string we need to remove using runes. - r := []rune(s) - truncationTarget := n - 3 - - // Next, let's truncate the runes to the lower possible number. - truncatedRunes := r[:truncationTarget] - for len(string(truncatedRunes)) > truncationTarget { - truncatedRunes = r[:len(truncatedRunes)-1] - } - - return string(truncatedRunes) + truncationMarker, true -} diff --git a/pkg/tests/api/alerting/api_notification_channel_test.go b/pkg/tests/api/alerting/api_notification_channel_test.go index 3fc09ffba95..8321680e67c 100644 --- a/pkg/tests/api/alerting/api_notification_channel_test.go +++ b/pkg/tests/api/alerting/api_notification_channel_test.go @@ -2529,7 +2529,7 @@ var expNonEmailNotifications = map[string][]string{ }`, }, "pushover_recv/pushover_test": { - "--abcd\r\nContent-Disposition: form-data; name=\"user\"\r\n\r\nmysecretkey\r\n--abcd\r\nContent-Disposition: form-data; name=\"token\"\r\n\r\nmysecrettoken\r\n--abcd\r\nContent-Disposition: form-data; name=\"priority\"\r\n\r\n0\r\n--abcd\r\nContent-Disposition: form-data; name=\"sound\"\r\n\r\n\r\n--abcd\r\nContent-Disposition: form-data; name=\"title\"\r\n\r\n[FIRING:1] PushoverAlert (default)\r\n--abcd\r\nContent-Disposition: form-data; name=\"url\"\r\n\r\nhttp://localhost:3000/alerting/list\r\n--abcd\r\nContent-Disposition: form-data; name=\"url_title\"\r\n\r\nShow alert rule\r\n--abcd\r\nContent-Disposition: form-data; name=\"message\"\r\n\r\n**Firing**\n\nValue: A=1\nLabels:\n - alertname = PushoverAlert\n - grafana_folder = default\nAnnotations:\nSource: http://localhost:3000/alerting/grafana/UID_PushoverAlert/view\nSilence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DPushoverAlert&matcher=grafana_folder%3Ddefault\n\r\n--abcd\r\nContent-Disposition: form-data; name=\"html\"\r\n\r\n1\r\n--abcd--\r\n", + "--abcd\r\nContent-Disposition: form-data; name=\"user\"\r\n\r\nmysecretkey\r\n--abcd\r\nContent-Disposition: form-data; name=\"token\"\r\n\r\nmysecrettoken\r\n--abcd\r\nContent-Disposition: form-data; name=\"priority\"\r\n\r\n0\r\n--abcd\r\nContent-Disposition: form-data; name=\"sound\"\r\n\r\n\r\n--abcd\r\nContent-Disposition: form-data; name=\"title\"\r\n\r\n[FIRING:1] PushoverAlert (default)\r\n--abcd\r\nContent-Disposition: form-data; name=\"url\"\r\n\r\nhttp://localhost:3000/alerting/list\r\n--abcd\r\nContent-Disposition: form-data; name=\"url_title\"\r\n\r\nShow alert rule\r\n--abcd\r\nContent-Disposition: form-data; name=\"message\"\r\n\r\n**Firing**\n\nValue: A=1\nLabels:\n - alertname = PushoverAlert\n - grafana_folder = default\nAnnotations:\nSource: http://localhost:3000/alerting/grafana/UID_PushoverAlert/view\nSilence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DPushoverAlert&matcher=grafana_folder%3Ddefault\r\n--abcd\r\nContent-Disposition: form-data; name=\"html\"\r\n\r\n1\r\n--abcd--\r\n", }, "telegram_recv/bot6sh027hs034h": { "--abcd\r\nContent-Disposition: form-data; name=\"chat_id\"\r\n\r\ntelegram_chat_id\r\n--abcd\r\nContent-Disposition: form-data; name=\"parse_mode\"\r\n\r\nhtml\r\n--abcd\r\nContent-Disposition: form-data; name=\"text\"\r\n\r\n**Firing**\n\nValue: A=1\nLabels:\n - alertname = TelegramAlert\n - grafana_folder = default\nAnnotations:\nSource: http://localhost:3000/alerting/grafana/UID_TelegramAlert/view\nSilence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DTelegramAlert&matcher=grafana_folder%3Ddefault\n\r\n--abcd--\r\n",