From 484ee8a621213def70c9bfa9533afed5592b8ef8 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Wed, 9 Nov 2022 17:14:51 +0000 Subject: [PATCH] Truncate: Be explicit on truncation of runes or bytes. While most integrations set a limit by UTF-8 compatible characters (some like Webex) use runes - as pointed out in https://github.com/prometheus/alertmanager/pull/3132. This PR makes it explicit wether the truncation is happening at a byte or rune level. Signed-off-by: gotjosh Signed-off-by: Yijie Qin --- notify/opsgenie/opsgenie.go | 3 +- notify/pagerduty/pagerduty.go | 6 ++- notify/pushover/pushover.go | 9 ++-- notify/slack/slack.go | 3 +- notify/telegram/telegram.go | 4 +- notify/util.go | 19 +++++++- notify/util_test.go | 82 ++++++++++++++++++++++++++--------- notify/victorops/victorops.go | 3 +- 8 files changed, 96 insertions(+), 33 deletions(-) diff --git a/notify/opsgenie/opsgenie.go b/notify/opsgenie/opsgenie.go index 10c84493c8..522ec4e4f7 100644 --- a/notify/opsgenie/opsgenie.go +++ b/notify/opsgenie/opsgenie.go @@ -171,7 +171,8 @@ func (n *Notifier) createRequests(ctx context.Context, as ...*types.Alert) ([]*h } requests = append(requests, req.WithContext(ctx)) default: - message, truncated := notify.Truncate(tmpl(n.conf.Message), 130) + // https://docs.opsgenie.com/docs/alert-api - 130 characters meaning runes. + message, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), 130) if truncated { level.Debug(n.logger).Log("msg", "truncated message", "truncated_message", message, "alert", key) } diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index fad2c6369a..c95f7e10fb 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -149,7 +149,8 @@ func (n *Notifier) notifyV1( var tmplErr error tmpl := notify.TmplText(n.tmpl, data, &tmplErr) - description, truncated := notify.Truncate(tmpl(n.conf.Description), 1024) + // https://developer.pagerduty.com/docs/ZG9jOjExMDI5NTgx-send-an-alert-event - 1204 characters or runes. + description, truncated := notify.TruncateInRunes(tmpl(n.conf.Description), 1024) if truncated { level.Debug(n.logger).Log("msg", "Truncated description", "description", description, "key", key) } @@ -214,7 +215,8 @@ func (n *Notifier) notifyV2( n.conf.Severity = "error" } - summary, truncated := notify.Truncate(tmpl(n.conf.Description), 1024) + // https://developer.pagerduty.com/docs/ZG9jOjExMDI5NTgx-send-an-alert-event - 1204 characters or runes. + summary, truncated := notify.TruncateInRunes(tmpl(n.conf.Description), 1024) if truncated { level.Debug(n.logger).Log("msg", "Truncated summary", "summary", summary, "key", key) } diff --git a/notify/pushover/pushover.go b/notify/pushover/pushover.go index d42d558d5b..33a0d281fd 100644 --- a/notify/pushover/pushover.go +++ b/notify/pushover/pushover.go @@ -78,7 +78,8 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) parameters.Add("token", tmpl(string(n.conf.Token))) parameters.Add("user", tmpl(string(n.conf.UserKey))) - title, truncated := notify.Truncate(tmpl(n.conf.Title), 250) + // https://pushover.net/api#limits - 250 characters or runes. + title, truncated := notify.TruncateInRunes(tmpl(n.conf.Title), 250) if truncated { level.Debug(n.logger).Log("msg", "Truncated title", "truncated_title", title, "incident", key) } @@ -91,7 +92,8 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) message = tmpl(n.conf.Message) } - message, truncated = notify.Truncate(message, 1024) + // https://pushover.net/api#limits - 1024 characters or runes. + message, truncated = notify.TruncateInRunes(message, 1024) if truncated { level.Debug(n.logger).Log("msg", "Truncated message", "truncated_message", message, "incident", key) } @@ -102,7 +104,8 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } parameters.Add("message", message) - supplementaryURL, truncated := notify.Truncate(tmpl(n.conf.URL), 512) + // https://pushover.net/api#limits - 512 characters or runes. + supplementaryURL, truncated := notify.TruncateInRunes(tmpl(n.conf.URL), 512) if truncated { level.Debug(n.logger).Log("msg", "Truncated URL", "truncated_url", supplementaryURL, "incident", key) } diff --git a/notify/slack/slack.go b/notify/slack/slack.go index 8f3445e093..887ef28f7b 100644 --- a/notify/slack/slack.go +++ b/notify/slack/slack.go @@ -99,7 +99,8 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } else { markdownIn = n.conf.MrkdwnIn } - title, truncated := notify.Truncate(tmplText(n.conf.Title), 1024) + // No refernce in https://api.slack.com/reference/messaging/attachments#legacy_fields - assuming runes or characters. + title, truncated := notify.TruncateInRunes(tmplText(n.conf.Title), 1024) if truncated { key, err := notify.ExtractGroupKey(ctx) if err != nil { diff --git a/notify/telegram/telegram.go b/notify/telegram/telegram.go index 43bf2a36bc..7dfc22eec1 100644 --- a/notify/telegram/telegram.go +++ b/notify/telegram/telegram.go @@ -65,8 +65,8 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err tmpl = notify.TmplText(n.tmpl, data, &err) ) - // Telegram supports 4096 chars max - messageText, truncated := notify.Truncate(tmpl(n.conf.Message), 4096) + // Telegram supports 4096 chars max - from https://limits.tginfo.me/en. + messageText, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), 4096) if truncated { level.Debug(n.logger).Log("msg", "truncated message", "truncated_message", messageText) } diff --git a/notify/util.go b/notify/util.go index 0c5b675448..9cdbee8c8d 100644 --- a/notify/util.go +++ b/notify/util.go @@ -81,18 +81,33 @@ func Drain(r *http.Response) { r.Body.Close() } -// Truncate truncates a string to fit the given size. -func Truncate(s string, n int) (string, bool) { +// Truncate truncates a string to fit the given size in Bytes. +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]) + "…", true } +// Truncate truncates a string to fit the given size in Runes. +func TruncateInBytes(s string, n int) (string, bool) { + if len(s) <= n { + return s, false + } + + if n <= 3 { + return string(s[:n]), true + } + + return string(s[:n-3]) + "…", true // This rune +} + // TmplText is using monadic error handling in order to make string templating // less verbose. Use with care as the final error checking is easily missed. func TmplText(tmpl *template.Template, data *template.Data, err *error) func(string) string { diff --git a/notify/util_test.go b/notify/util_test.go index b0826ddb62..434e8c558a 100644 --- a/notify/util_test.go +++ b/notify/util_test.go @@ -18,69 +18,109 @@ import ( "fmt" "io" "net/http" + "path" + "reflect" + "runtime" "testing" "github.com/stretchr/testify/require" ) func TestTruncate(t *testing.T) { + type expect struct { + out string + trunc bool + } + testCases := []struct { in string n int - out string - trunc bool + runes expect + bytes expect }{ { in: "", n: 5, - out: "", - trunc: false, + runes: expect{out: "", trunc: false}, + bytes: expect{out: "", trunc: false}, }, { in: "abcde", n: 2, - out: "ab", - trunc: true, + runes: expect{out: "ab", trunc: true}, + bytes: expect{out: "ab", trunc: true}, }, { in: "abcde", n: 4, - out: "abc…", - trunc: true, + runes: expect{out: "abc…", trunc: true}, + bytes: expect{out: "a…", trunc: true}, }, { in: "abcde", n: 5, - out: "abcde", - trunc: false, + runes: expect{out: "abcde", trunc: false}, + bytes: expect{out: "abcde", trunc: false}, }, { in: "abcdefgh", n: 5, - out: "abcd…", - trunc: true, + runes: expect{out: "abcd…", trunc: true}, + bytes: expect{out: "ab…", trunc: true}, }, { in: "a⌘cde", n: 5, - out: "a⌘cde", - trunc: false, + runes: expect{out: "a⌘cde", trunc: false}, + bytes: expect{out: "a\xe2…", trunc: true}, }, { in: "a⌘cdef", n: 5, - out: "a⌘cd…", - trunc: true, + runes: expect{out: "a⌘cd…", trunc: true}, + bytes: expect{out: "a\xe2…", trunc: true}, + }, + { + in: "世界cdef", + n: 3, + runes: expect{out: "世界c", trunc: true}, + bytes: expect{out: "世", trunc: true}, + }, + { + in: "❤️✅🚀🔥❌", + n: 4, + runes: expect{out: "❤️✅…", trunc: true}, + bytes: expect{out: "\xe2…", trunc: true}, }, } + type truncateFunc func(string, int) (string, bool) + for _, tc := range testCases { - t.Run(fmt.Sprintf("truncate(%s,%d)", tc.in, tc.n), func(t *testing.T) { - s, trunc := Truncate(tc.in, tc.n) - require.Equal(t, tc.trunc, trunc) - require.Equal(t, tc.out, s) - }) + for _, fn := range []truncateFunc{TruncateInBytes, TruncateInRunes} { + var truncated bool + var out string + + fnPath := runtime.FuncForPC(reflect.ValueOf(fn).Pointer()).Name() + fnName := path.Base(fnPath) + switch fnName { + case "notify.TruncateInRunes": + truncated = tc.runes.trunc + out = tc.runes.out + case "notify.TruncateInBytes": + truncated = tc.bytes.trunc + out = tc.bytes.out + default: + t.Fatalf("unknown function") + } + + t.Run(fmt.Sprintf("%s(%s,%d)", fnName, tc.in, tc.n), func(t *testing.T) { + s, trunc := fn(tc.in, tc.n) + require.Equal(t, truncated, trunc) + require.Equal(t, out, s) + }) + } } } diff --git a/notify/victorops/victorops.go b/notify/victorops/victorops.go index 1de7cf2c80..0da5db816c 100644 --- a/notify/victorops/victorops.go +++ b/notify/victorops/victorops.go @@ -134,7 +134,8 @@ func (n *Notifier) createVictorOpsPayload(ctx context.Context, as ...*types.Aler messageType = victorOpsEventResolve } - stateMessage, truncated := notify.Truncate(stateMessage, 20480) + // https://help.victorops.com/knowledge-base/incident-fields-glossary/ - 20480 characters. + stateMessage, truncated := notify.TruncateInRunes(stateMessage, 20480) if truncated { level.Debug(n.logger).Log("msg", "truncated stateMessage", "truncated_state_message", stateMessage, "incident", key) }