From 1a116a1edf3e0f3fdf48a5907f3e80b8a8f62aee Mon Sep 17 00:00:00 2001 From: TJ Hoplock Date: Sat, 16 Nov 2024 00:10:50 -0500 Subject: [PATCH] chore(notify): make logging more consistent, converge on `group_key` This changes the majority of our `Notify()` implementations to set up a new logger with the group key attached under the key name `group_key`, and then to use that logger in all subsequent calls to the logger, including passing it through to further functions that accept loggers via params. A few of the notify implementations are more complicated; they either extract the key later in their `Notify()` implementation or within sub methods, or even conditionally like with sns. I left those mostly as is for now, as they seem to be more snow-flake-y. Signed-off-by: TJ Hoplock --- notify/discord/discord.go | 13 +++++++------ notify/jira/jira.go | 1 + notify/msteams/msteams.go | 5 +++-- notify/msteamsv2/msteamsv2.go | 5 +++-- notify/opsgenie/opsgenie.go | 7 ++++--- notify/pagerduty/pagerduty.go | 5 +++-- notify/pushover/pushover.go | 14 +++++++------- notify/rocketchat/rocketchat.go | 15 +++++++++------ notify/slack/slack.go | 16 ++++++++++------ notify/telegram/telegram.go | 19 +++++++++++-------- notify/victorops/victorops.go | 2 +- notify/webex/webex.go | 7 ++++--- notify/webhook/webhook.go | 3 ++- notify/wechat/wechat.go | 8 +++++--- 14 files changed, 70 insertions(+), 50 deletions(-) diff --git a/notify/discord/discord.go b/notify/discord/discord.go index 42d8724e..bc7a6e19 100644 --- a/notify/discord/discord.go +++ b/notify/discord/discord.go @@ -95,10 +95,11 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } - n.logger.Debug("extracted group key", "key", key) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") alerts := types.Alerts(as...) - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) if err != nil { return false, err @@ -109,14 +110,14 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } if truncated { - n.logger.Warn("Truncated title", "key", key, "max_runes", maxTitleLenRunes) + logger.Warn("Truncated title", "max_runes", maxTitleLenRunes) } description, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), maxDescriptionLenRunes) if err != nil { return false, err } if truncated { - n.logger.Warn("Truncated message", "key", key, "max_runes", maxDescriptionLenRunes) + logger.Warn("Truncated message", "max_runes", maxDescriptionLenRunes) } content, truncated := notify.TruncateInRunes(tmpl(n.conf.Content), maxContentLenRunes) @@ -124,7 +125,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } if truncated { - n.logger.Warn("Truncated message", "key", key, "max_runes", maxContentLenRunes) + logger.Warn("Truncated message", "max_runes", maxContentLenRunes) } color := colorGrey @@ -160,7 +161,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if _, err := netUrl.Parse(n.conf.AvatarURL); err == nil { w.AvatarURL = n.conf.AvatarURL } else { - n.logger.Warn("Bad avatar url", "key", key) + logger.Warn("Bad avatar url", "key", key) } } diff --git a/notify/jira/jira.go b/notify/jira/jira.go index 00d67027..e5420f8b 100644 --- a/notify/jira/jira.go +++ b/notify/jira/jira.go @@ -72,6 +72,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } logger := n.logger.With("group_key", key.String()) + logger.Debug("extracted group key") var ( alerts = types.Alerts(as...) diff --git a/notify/msteams/msteams.go b/notify/msteams/msteams.go index f53f020b..b46ee07a 100644 --- a/notify/msteams/msteams.go +++ b/notify/msteams/msteams.go @@ -85,9 +85,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } - n.logger.Debug("extracted group key", "key", key) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) if err != nil { return false, err diff --git a/notify/msteamsv2/msteamsv2.go b/notify/msteamsv2/msteamsv2.go index b25bf11d..e24b498f 100644 --- a/notify/msteamsv2/msteamsv2.go +++ b/notify/msteamsv2/msteamsv2.go @@ -104,9 +104,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } - n.logger.Debug("extracted group key", "key", key) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) if err != nil { return false, err diff --git a/notify/opsgenie/opsgenie.go b/notify/opsgenie/opsgenie.go index 75549846..9ff2f1b2 100644 --- a/notify/opsgenie/opsgenie.go +++ b/notify/opsgenie/opsgenie.go @@ -132,9 +132,10 @@ func (n *Notifier) createRequests(ctx context.Context, as ...*types.Alert) ([]*h if err != nil { return nil, false, err } - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") - n.logger.Debug("extracted group key", "key", key) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) @@ -174,7 +175,7 @@ func (n *Notifier) createRequests(ctx context.Context, as ...*types.Alert) ([]*h default: message, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), maxMessageLenRunes) if truncated { - n.logger.Warn("Truncated message", "alert", key, "max_runes", maxMessageLenRunes) + logger.Warn("Truncated message", "alert", key, "max_runes", maxMessageLenRunes) } createEndpointURL := n.conf.APIURL.Copy() diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index abab5a70..34f15959 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -308,17 +308,18 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if err != nil { return false, err } + logger := n.logger.With("group_key", key) var ( alerts = types.Alerts(as...) - data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data = notify.GetTemplateData(ctx, n.tmpl, as, logger) eventType = pagerDutyEventTrigger ) if alerts.Status() == model.AlertResolved { eventType = pagerDutyEventResolve } - n.logger.Debug("extracted group key", "key", key, "eventType", eventType) + logger.Debug("extracted group key", "eventType", eventType) details := make(map[string]string, len(n.conf.Details)) for k, v := range n.conf.Details { diff --git a/notify/pushover/pushover.go b/notify/pushover/pushover.go index 5445b977..e7890d7f 100644 --- a/notify/pushover/pushover.go +++ b/notify/pushover/pushover.go @@ -72,10 +72,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if !ok { return false, fmt.Errorf("group key missing") } - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") - // @tjhop: should this use `group` for the keyval like most other notify implementations? - n.logger.Debug("extracted group key", "incident", key) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) var ( err error @@ -113,7 +113,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) title, truncated := notify.TruncateInRunes(tmpl(n.conf.Title), maxTitleLenRunes) if truncated { - n.logger.Warn("Truncated title", "incident", key, "max_runes", maxTitleLenRunes) + logger.Warn("Truncated title", "incident", key, "max_runes", maxTitleLenRunes) } parameters.Add("title", title) @@ -126,7 +126,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) message, truncated = notify.TruncateInRunes(message, maxMessageLenRunes) if truncated { - n.logger.Warn("Truncated message", "incident", key, "max_runes", maxMessageLenRunes) + logger.Warn("Truncated message", "incident", key, "max_runes", maxMessageLenRunes) } message = strings.TrimSpace(message) if message == "" { @@ -137,7 +137,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) supplementaryURL, truncated := notify.TruncateInRunes(tmpl(n.conf.URL), maxURLLenRunes) if truncated { - n.logger.Warn("Truncated URL", "incident", key, "max_runes", maxURLLenRunes) + logger.Warn("Truncated URL", "incident", key, "max_runes", maxURLLenRunes) } parameters.Add("url", supplementaryURL) parameters.Add("url_title", tmpl(n.conf.URLTitle)) @@ -163,7 +163,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } u.RawQuery = parameters.Encode() // Don't log the URL as it contains secret data (see #1825). - n.logger.Debug("Sending message", "incident", key) + logger.Debug("Sending message", "incident", key) resp, err := notify.PostText(ctx, n.client, u.String(), nil) if err != nil { return true, notify.RedactURL(err) diff --git a/notify/rocketchat/rocketchat.go b/notify/rocketchat/rocketchat.go index 8f5b3ef8..841e8e5c 100644 --- a/notify/rocketchat/rocketchat.go +++ b/notify/rocketchat/rocketchat.go @@ -140,7 +140,14 @@ func getToken(c *config.RocketchatConfig) (string, error) { func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { var err error - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + key, err := notify.ExtractGroupKey(ctx) + if err != nil { + return false, err + } + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") + + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmplText := notify.TmplText(n.tmpl, data, &err) if err != nil { return false, err @@ -152,11 +159,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) title, truncated := notify.TruncateInRunes(title, maxTitleLenRunes) if truncated { - key, err := notify.ExtractGroupKey(ctx) - if err != nil { - return false, err - } - n.logger.Warn("Truncated title", "key", key, "max_runes", maxTitleLenRunes) + logger.Warn("Truncated title", "max_runes", maxTitleLenRunes) } att := &Attachment{ Title: title, diff --git a/notify/slack/slack.go b/notify/slack/slack.go index c335ab5f..973ccc93 100644 --- a/notify/slack/slack.go +++ b/notify/slack/slack.go @@ -93,8 +93,16 @@ type attachment struct { // Notify implements the Notifier interface. func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { var err error + + key, err := notify.ExtractGroupKey(ctx) + if err != nil { + return false, err + } + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") + var ( - data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data = notify.GetTemplateData(ctx, n.tmpl, as, logger) tmplText = notify.TmplText(n.tmpl, data, &err) ) var markdownIn []string @@ -107,11 +115,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) title, truncated := notify.TruncateInRunes(tmplText(n.conf.Title), maxTitleLenRunes) if truncated { - key, err := notify.ExtractGroupKey(ctx) - if err != nil { - return false, err - } - n.logger.Warn("Truncated title", "key", key, "max_runes", maxTitleLenRunes) + logger.Warn("Truncated title", "max_runes", maxTitleLenRunes) } att := &attachment{ Title: title, diff --git a/notify/telegram/telegram.go b/notify/telegram/telegram.go index 3bd359cc..6a1547e9 100644 --- a/notify/telegram/telegram.go +++ b/notify/telegram/telegram.go @@ -64,9 +64,17 @@ func New(conf *config.TelegramConfig, t *template.Template, l *slog.Logger, http } func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, error) { + key, ok := notify.GroupKey(ctx) + if !ok { + return false, fmt.Errorf("group key missing") + } + + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") + var ( err error - data = notify.GetTemplateData(ctx, n.tmpl, alert, n.logger) + data = notify.GetTemplateData(ctx, n.tmpl, alert, logger) tmpl = notify.TmplText(n.tmpl, data, &err) ) @@ -74,14 +82,9 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err tmpl = notify.TmplHTML(n.tmpl, data, &err) } - key, ok := notify.GroupKey(ctx) - if !ok { - return false, fmt.Errorf("group key missing") - } - messageText, truncated := notify.TruncateInRunes(tmpl(n.conf.Message), maxMessageLenRunes) if truncated { - n.logger.Warn("Truncated message", "alert", key, "max_runes", maxMessageLenRunes) + logger.Warn("Truncated message", "max_runes", maxMessageLenRunes) } n.client.Token, err = n.getBotToken() @@ -98,7 +101,7 @@ func (n *Notifier) Notify(ctx context.Context, alert ...*types.Alert) (bool, err if err != nil { return true, err } - n.logger.Debug("Telegram message successfully published", "message_id", message.ID, "chat_id", message.Chat.ID) + logger.Debug("Telegram message successfully published", "message_id", message.ID, "chat_id", message.Chat.ID) return false, nil } diff --git a/notify/victorops/victorops.go b/notify/victorops/victorops.go index ac89222e..d676d3d8 100644 --- a/notify/victorops/victorops.go +++ b/notify/victorops/victorops.go @@ -141,7 +141,7 @@ func (n *Notifier) createVictorOpsPayload(ctx context.Context, as ...*types.Aler stateMessage, truncated := notify.TruncateInRunes(stateMessage, maxMessageLenRunes) if truncated { - n.logger.Warn("Truncated state_message", "incident", key, "max_runes", maxMessageLenRunes) + n.logger.Warn("Truncated state_message", "group_key", key, "max_runes", maxMessageLenRunes) } msg := map[string]string{ diff --git a/notify/webex/webex.go b/notify/webex/webex.go index 8ec916df..e45667fa 100644 --- a/notify/webex/webex.go +++ b/notify/webex/webex.go @@ -72,9 +72,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } - n.logger.Debug("extracted group key", "key", key) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) if err != nil { return false, err @@ -87,7 +88,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) message, truncated := notify.TruncateInBytes(message, maxMessageSize) if truncated { - n.logger.Debug("message truncated due to exceeding maximum allowed length by webex", "truncated_message", message) + logger.Debug("message truncated due to exceeding maximum allowed length by webex", "truncated_message", message) } w := webhook{ diff --git a/notify/webhook/webhook.go b/notify/webhook/webhook.go index 9464d71c..53aab564 100644 --- a/notify/webhook/webhook.go +++ b/notify/webhook/webhook.go @@ -85,7 +85,8 @@ func (n *Notifier) Notify(ctx context.Context, alerts ...*types.Alert) (bool, er return false, err } - n.logger.Debug("extracted group key", "incident", groupKey) + logger := n.logger.With("group_key", groupKey) + logger.Debug("extracted group key") msg := &Message{ Version: "4", diff --git a/notify/wechat/wechat.go b/notify/wechat/wechat.go index 4aab58ca..79ae2e29 100644 --- a/notify/wechat/wechat.go +++ b/notify/wechat/wechat.go @@ -86,8 +86,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) return false, err } - n.logger.Debug("extracted group key", "key", key) - data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger) + logger := n.logger.With("group_key", key) + logger.Debug("extracted group key") + + data := notify.GetTemplateData(ctx, n.tmpl, as, logger) tmpl := notify.TmplText(n.tmpl, data, &err) if err != nil { @@ -174,7 +176,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if err != nil { return true, err } - n.logger.Debug(string(body), "incident", key) + logger.Debug(string(body)) var weResp weChatResponse if err := json.Unmarshal(body, &weResp); err != nil {