fixes

internal/alerts/notify.go

@@ -6,14 +6,19 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"html"
 	"io"
 	"net/http"
 	"net/url"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
 
 	"git.tornberg.me/go-otel/internal/features"
+	"git.tornberg.me/go-otel/internal/topology"
+	"git.tornberg.me/go-otel/internal/tracectx"
+	"git.tornberg.me/go-otel/internal/triage"
 	"git.tornberg.me/go-otel/internal/uaclass"
 )
 
@@ -28,14 +33,21 @@ type Alert struct {
 
 // PatternReport is a spike pattern with its triage verdict, ready to show.
 type PatternReport struct {
-	Category string  `json:"category"`
-	Rule     string  `json:"rule"`
-	New      bool    `json:"new"`
-	Count    int     `json:"count"`
-	Baseline float64 `json:"baseline"`
-	Clients  int     `json:"clients,omitempty"`
-	UAClass  string  `json:"uaClass,omitempty"`
-	What     string  `json:"what"`
+	Category string            `json:"category"`
+	Priority Priority          `json:"priority"`
+	IssueID  string            `json:"issueId,omitempty"`
+	Status   int               `json:"status,omitempty"`
+	Host     string            `json:"host,omitempty"`
+	Path     string            `json:"path,omitempty"`
+	Services []string          `json:"services,omitempty"` // every service failing in these requests
+	Routes   []topology.Target `json:"routes,omitempty"`   // ingress -> service -> deployment behind an edge error
+	Rule     string            `json:"rule"`
+	New      bool              `json:"new"`
+	Count    int               `json:"count"`
+	Baseline float64           `json:"baseline"`
+	Clients  int               `json:"clients,omitempty"`
+	UAClass  string            `json:"uaClass,omitempty"`
+	What     string            `json:"what"`
 }
 
 // Notifier delivers an alert somewhere. Implementations must not put secrets
@@ -50,15 +62,35 @@ func NewAlert(spike features.Spike, at time.Time) Alert {
 	a := Alert{At: at, Spike: spike, ID: alertID(spike, at)}
 	for _, p := range spike.Patterns {
 		res := TriageOf(p.Example)
-		r := PatternReport{Category: string(res.Category), Rule: res.Rule, New: p.Novel, Count: p.Count, Baseline: p.BaselineMean, Clients: p.Clients, What: describe(p.Example)}
+		r := PatternReport{Category: string(res.Category), Priority: PriorityOf(p.Example, res.Category), IssueID: p.Example.IssueFingerprint, Status: p.Example.StatusCode, Host: p.Example.Host, Path: p.Example.Route, Services: p.Services, Rule: res.Rule, New: p.Novel, Count: p.Count, Baseline: p.BaselineMean, Clients: p.Clients, What: describe(p.Example)}
 		if p.Example.UAClass != "" && uaclass.IsBot(p.Example.UAClass) {
 			r.UAClass = p.Example.UAClass
 		}
 		a.Patterns = append(a.Patterns, r)
 	}
+	// Most urgent first; the detector's excess order is kept within a priority.
+	sort.SliceStable(a.Patterns, func(i, j int) bool { return prioRank(a.Patterns[i].Priority) < prioRank(a.Patterns[j].Priority) })
 	return a
 }
 
+func prioRank(p Priority) int {
+	switch p {
+	case PriorityHigh:
+		return 0
+	case PriorityMedium:
+		return 1
+	}
+	return 2
+}
+
+// Priority is the most urgent priority among the alert's patterns.
+func (a Alert) Priority() Priority {
+	if len(a.Patterns) == 0 {
+		return PriorityLow
+	}
+	return a.Patterns[0].Priority
+}
+
 func alertID(s features.Spike, at time.Time) string {
 	return strconv.FormatInt(at.Unix(), 36) + "-" + strings.ReplaceAll(s.Service, " ", "_")
 }
@@ -81,7 +113,10 @@ func describe(e features.Event) string {
 		parts = append(parts, truncate(e.Message, 120))
 	}
 	if len(parts) == 0 {
-		return "(no detail)"
+		if e.Service != "" {
+			return "Errors in " + e.Service + " without a message, route or status (see the issue for the trace)"
+		}
+		return "Errors without a message, route or status"
 	}
 	return strings.Join(parts, " | ")
 }
@@ -94,35 +129,106 @@ func truncate(s string, n int) string {
 	return string(r[:n-1]) + "…"
 }
 
+// advice says what a human should do about a category. Wording is plain so it
+// can be acted on without knowing the detector's vocabulary.
+func advice(category string, status int) string {
+	if status >= 400 && status < 500 {
+		return "Client errors (4xx): usually bad requests or stale links, rarely our fault. Only act if the count keeps growing."
+	}
+	switch triage.Category(category) {
+	case triage.DependencyUnreachable:
+		return "Something this service depends on (database, upstream service or network) is failing. Check the health of the services listed here, starting with the deepest one."
+	case triage.CapacitySaturation:
+		return "Requests are timing out or resources are exhausted. Check load, connection pools and CPU/memory limits."
+	case triage.ServiceDefect:
+		return "Most likely a bug or crash in this service. Open the issue for the stack trace and check whether a recent release introduced it."
+	case triage.ClientCredentialIssue:
+		return "Callers are being rejected as unauthenticated. Check for an expired token or key."
+	}
+	return "Unclassified error. Open the issue and decide whether it matters."
+}
+
+// routeLine explains where an edge error was routed and, for backends that
+// reported no failing spans, how to check them directly: an uninstrumented
+// service is invisible to the trace, so it must be looked at by hand.
+func routeLine(t topology.Target, failing []string) string {
+	where := t.Host + t.Path + " -> service " + t.Namespace + "/" + t.Service
+	switch {
+	case t.Missing:
+		return where + ": this Service does not exist, so the ingress rule points at nothing."
+	case t.External != "":
+		return where + " -> external " + t.External + " (outside the cluster)."
+	case len(t.Deployments) == 0:
+		return where + " (no Deployment selected)."
+	}
+	var silent []string
+	for _, d := range t.Deployments {
+		if !contains(failing, d) && !contains(failing, t.Service) {
+			silent = append(silent, d)
+		}
+	}
+	line := where + " -> deployment " + strings.Join(t.Deployments, ", ")
+	if len(silent) > 0 {
+		line += ". No errors from it in the failing requests, it may not be instrumented: kubectl -n " + t.Namespace + " get pods -l <selector> / logs deploy/" + silent[0]
+	}
+	return line + "."
+}
+
+func contains(list []string, v string) bool {
+	for _, x := range list {
+		if x == v {
+			return true
+		}
+	}
+	return false
+}
+
+func verb(p PatternReport) string {
+	if p.New {
+		return "New error, never seen in the last few hours."
+	}
+	return fmt.Sprintf("Rising: %d now, normally about %.0f per window.", p.Count, p.Baseline)
+}
+
 // Text renders a plain-text message (no markup, so nothing needs escaping).
+// Each pattern says what failed, how it differs from normal, who and what is
+// affected, what to do, and where to look.
 func (a Alert) Text(baseURL string) string {
 	var b strings.Builder
-	prefix := "ALERT"
+	prefix := strings.ToUpper(string(a.Priority()))
 	if a.Test {
 		prefix = "TEST"
 	}
-	fmt.Fprintf(&b, "%s %s: %d errors in %s (baseline %.1f/window, excess %.0f)\n",
-		prefix, a.Spike.Service, a.Spike.ErrorCount, a.Spike.WindowEnd.Sub(a.Spike.WindowStart).Round(time.Minute), a.Spike.BaselineMean, a.Spike.Excess)
-	for _, p := range a.Patterns {
-		tag := "rising"
-		if p.New {
-			tag = "NEW"
+	window := a.Spike.WindowEnd.Sub(a.Spike.WindowStart).Round(time.Minute)
+	fmt.Fprintf(&b, "%s: %s is failing (%d errors in the last %s, normally about %.0f)\n",
+		prefix, a.Spike.Service, a.Spike.ErrorCount, window, a.Spike.BaselineMean)
+	base := strings.TrimRight(baseURL, "/")
+	for i, p := range a.Patterns {
+		fmt.Fprintf(&b, "\n%d. %s\n", i+1, p.What)
+		fmt.Fprintf(&b, "   %s\n", verb(p))
+		var who []string
+		if len(p.Services) > 1 {
+			who = append(who, "Services with errors in the same requests: "+strings.Join(p.Services, ", "))
 		}
-		fmt.Fprintf(&b, "\n%s x%d (base %.1f) [%s]\n%s", tag, p.Count, p.Baseline, p.Category, p.What)
-		var extra []string
 		if p.Clients > 0 {
-			extra = append(extra, fmt.Sprintf("%d client%s", p.Clients, plural(p.Clients)))
+			who = append(who, fmt.Sprintf("%d client%s affected", p.Clients, plural(p.Clients)))
 		}
 		if p.UAClass != "" {
-			extra = append(extra, p.UAClass)
+			who = append(who, "client looks like a bot ("+p.UAClass+")")
 		}
-		if len(extra) > 0 {
-			fmt.Fprintf(&b, "\n(%s)", strings.Join(extra, ", "))
+		for _, w := range who {
+			fmt.Fprintf(&b, "   %s.\n", w)
+		}
+		for _, t := range p.Routes {
+			fmt.Fprintf(&b, "   %s\n", routeLine(t, p.Services))
+		}
+		fmt.Fprintf(&b, "   What to do: %s\n", advice(p.Category, p.Status))
+		if base != "" && p.IssueID != "" {
+			fmt.Fprintf(&b, "   %s/issues/%s\n", base, url.PathEscape(p.IssueID))
 		}
-		b.WriteString("\n")
 	}
-	if baseURL != "" {
-		fmt.Fprintf(&b, "\n%s/issues", strings.TrimRight(baseURL, "/"))
+	if base != "" {
+		fmt.Fprintf(&b, "\nAll issues: %s/issues", base)
 	}
 	return strings.TrimSpace(b.String())
 }
@@ -134,6 +240,85 @@ func plural(n int) string {
 	return "s"
 }
 
+func priorityIcon(p Priority) string {
+	switch p {
+	case PriorityHigh:
+		return "🔴"
+	case PriorityMedium:
+		return "🟠"
+	}
+	return "🟡"
+}
+
+// HTML renders the alert for Telegram's HTML parse mode. Every dynamic value
+// is escaped, and patterns are added whole until the message would exceed max
+// characters, so a cut never lands inside a tag.
+func (a Alert) HTML(baseURL string, max int) string {
+	esc := html.EscapeString
+	window := a.Spike.WindowEnd.Sub(a.Spike.WindowStart).Round(time.Minute)
+	icon, label := priorityIcon(a.Priority()), strings.ToUpper(string(a.Priority()))
+	if a.Test {
+		icon, label = "🧪", "TEST"
+	}
+	head := fmt.Sprintf("%s <b>%s</b> · <b>%s</b> is failing\n%d errors in the last %s (normally about %.0f)\n",
+		icon, label, esc(a.Spike.Service), a.Spike.ErrorCount, window, a.Spike.BaselineMean)
+	base := strings.TrimRight(baseURL, "/")
+	foot := ""
+	if base != "" {
+		foot = fmt.Sprintf("\n<a href=\"%s\">All issues</a>", esc(base+"/issues"))
+	}
+	var b strings.Builder
+	b.WriteString(head)
+	for i, p := range a.Patterns {
+		var s strings.Builder
+		fmt.Fprintf(&s, "\n<b>%d.</b> <code>%s</code>\n", i+1, esc(p.What))
+		if p.New {
+			s.WriteString("🆕 New error, not seen in the last few hours\n")
+		} else {
+			fmt.Fprintf(&s, "📈 Rising: %d now, normally about %.0f per window\n", p.Count, p.Baseline)
+		}
+		if len(p.Services) > 1 {
+			fmt.Fprintf(&s, "🧩 Failing together: %s\n", esc(strings.Join(p.Services, ", ")))
+		}
+		for _, t := range p.Routes {
+			fmt.Fprintf(&s, "🧭 %s\n", esc(routeLine(t, p.Services)))
+		}
+		if p.Clients > 0 {
+			fmt.Fprintf(&s, "👥 %d client%s affected\n", p.Clients, plural(p.Clients))
+		}
+		if p.UAClass != "" {
+			fmt.Fprintf(&s, "🤖 Client looks like a bot (%s)\n", esc(p.UAClass))
+		}
+		fmt.Fprintf(&s, "🛠 <i>%s</i>\n", esc(advice(p.Category, p.Status)))
+		if base != "" && p.IssueID != "" {
+			fmt.Fprintf(&s, "🔗 <a href=\"%s\">Open issue</a>\n", esc(base+"/issues/"+url.PathEscape(p.IssueID)))
+		}
+		if plainLen(b.String())+plainLen(s.String())+plainLen(foot)+40 > max {
+			fmt.Fprintf(&b, "\n… and %d more pattern%s, see the issues list", len(a.Patterns)-i, plural(len(a.Patterns)-i))
+			break
+		}
+		b.WriteString(s.String())
+	}
+	b.WriteString(foot)
+	return strings.TrimSpace(b.String())
+}
+
+// plainLen counts what Telegram counts: characters after tags are parsed away.
+func plainLen(s string) int {
+	n, in := 0, false
+	for _, r := range html.UnescapeString(s) {
+		switch {
+		case r == '<':
+			in = true
+		case r == '>' && in:
+			in = false
+		case !in:
+			n++
+		}
+	}
+	return n
+}
+
 // Telegram posts via the Bot API sendMessage method.
 type Telegram struct {
 	Token    string
@@ -154,7 +339,7 @@ func (t *Telegram) Send(ctx context.Context, a Alert) error {
 	if api == "" {
 		api = "https://api.telegram.org"
 	}
-	body := map[string]any{"chat_id": t.ChatID, "text": truncate(a.Text(t.BaseURL), telegramMax), "disable_web_page_preview": true}
+	body := map[string]any{"chat_id": t.ChatID, "text": a.HTML(t.BaseURL, telegramMax), "parse_mode": "HTML", "disable_web_page_preview": true}
 	if t.ThreadID != "" {
 		if id, err := strconv.Atoi(t.ThreadID); err == nil {
 			body["message_thread_id"] = id
@@ -233,3 +418,22 @@ func scrub(s string, secrets []string) string {
 	}
 	return s
 }
+
+// Resolver maps a request host and path to the backends behind it.
+type Resolver interface {
+	Resolve(host, path string) []topology.Target
+}
+
+// Enrich attaches the ingress -> service -> deployment path to patterns of an
+// edge service (nginx, ingress-nginx, ...), whose errors say little about
+// which workload is at fault.
+func (a *Alert) Enrich(r Resolver) {
+	if r == nil || !tracectx.IsEdgeService(a.Spike.Service) {
+		return
+	}
+	for i := range a.Patterns {
+		if p := &a.Patterns[i]; p.Host != "" {
+			p.Routes = r.Resolve(p.Host, p.Path)
+		}
+	}
+}