fix(providers): key and match StatefulSet replicas as one workload

internal/curator/draft.go

@@ -168,7 +168,7 @@ func alertResourceIfDistinct(inv providers.Investigation) string {
 // exact source; only the value the model happened to produce differed, so fixing
 // one path and not the other would leave the same defect armed here.
 func normalizeResource(w providers.Workload) string {
-	w.Name = normalizeResourceName(w.Name)
+	w.Name = normalizeResourceName(w)
 	ref, _ := kbvalidate.DraftResource(w.Ref())
 	return ref
 }

internal/curator/fingerprint.go

@@ -38,21 +38,24 @@ func normalizeText(s string) string {
 	return strings.Join(strings.Fields(s), " ")
 }
 
-// normalizeResourceName reduces a resource name to the identity the curator groups
+// normalizeResourceName reduces a workload's name to the identity the curator groups
 // by: an AWS ARN collapses to the resource identifier its CloudWatch dimension
-// carries, and a trailing pod-name hash is stripped so a per-pod name reduces to its
-// controller family. It is a thin alias for providers.NormalizeResourceName, the
-// single source of truth shared with the instant-recall path (CORE-681) so the two
-// can never drift.
+// carries, and a trailing per-instance suffix is stripped so a per-pod name reduces
+// to its controller family — a pod hash, a CronJob run stamp and, when the Kind says
+// the name is a Pod's, a StatefulSet replica ordinal (#513). It is a thin alias for
+// providers.Workload.IdentityName, the single source of truth shared with the
+// instant-recall path (CORE-681) so the two can never drift.
 //
-// It is deliberately NOT named normalizeWorkloadName any more: providers also
-// exports a NormalizeWorkloadName that does only the per-instance-suffix half — pod
-// hashes and CronJob run stamps — and a local alias wearing that exact name would
-// read as a pass-through to it while meaning something wider. Read providers.NormalizeResourceName for the accepted consequence of the ARN
-// collapse — the account and region are dropped, and the surrounding key fields do
-// not reliably put them back.
-func normalizeResourceName(name string) string {
-	return providers.NormalizeResourceName(name)
+// It takes the Workload and not the bare name because the Kind is part of the rule:
+// on a kind-less name a trailing ordinal is the name, and no string-level normalizer
+// may fold it. It is deliberately NOT named normalizeWorkloadName: providers also
+// exports a NormalizeWorkloadName that does only the per-instance-suffix half, and a
+// local alias wearing that exact name would read as a pass-through to it while
+// meaning something wider. Read providers.NormalizeResourceName for the accepted
+// consequence of the ARN collapse — the account and region are dropped, and the
+// surrounding key fields do not reliably put them back.
+func normalizeResourceName(w providers.Workload) string {
+	return w.IdentityName()
 }
 
 // IncidentKey builds a host-invariant, per-class dedup key for an alert: the alert
@@ -88,12 +91,15 @@ func normalizeResourceName(name string) string {
 // The same one-time reset now also reaches CronJob-generated workloads:
 // NormalizeWorkloadName folds the <unix-minutes> run suffix into the CronJob family,
 // so github-teams-sync-29787720 and -29790030 key alike where they used to key apart.
+// StatefulSet replicas reach it the same way (#513): with Kind "Pod",
+// vmagent-vmagent-0 and -1 key alike where they used to key apart.
 func IncidentKey(alertname string, w providers.Workload, cluster string) string {
+	w.Name = strings.TrimSpace(w.Name)
 	parts := []string{
 		strings.TrimSpace(alertname),
 		strings.TrimSpace(w.Namespace),
 		strings.TrimSpace(w.Kind),
-		normalizeResourceName(strings.TrimSpace(w.Name)),
+		normalizeResourceName(w),
 		strings.TrimSpace(cluster),
 	}
 	if account := strings.TrimSpace(w.Account); account != "" {
@@ -123,7 +129,7 @@ func Fingerprint(inv providers.Investigation) string {
 	}
 	if len(inv.Changes) > 0 {
 		w := inv.Changes[0].Workload
-		b.WriteString(" " + w.Namespace + " " + normalizeResourceName(w.Name))
+		b.WriteString(" " + w.Namespace + " " + normalizeResourceName(w))
 	}
 	return strings.TrimSpace(b.String())
 }
@@ -174,7 +180,7 @@ func DupFingerprint(inv providers.Investigation) string {
 	// incident on a different pod/node keys alike (CORE-681). The TriggerKey is
 	// already a host-invariant per-class key for alert sources (see IncidentKey).
 	res := inv.Resource
-	res.Name = normalizeResourceName(res.Name)
+	res.Name = normalizeResourceName(res)
 	ref := strings.ToLower(res.Ref())
 	// The AWS account qualifies the ref, so one instance name in two accounts does not
 	// curate as one incident. Appended only when present, so a Kubernetes ref — and

internal/curator/fingerprint_test.go

@@ -198,7 +198,7 @@ func TestNormalizeResourceNameAlias(t *testing.T) {
 		"":                                             "",
 	}
 	for in, want := range cases {
-		if got := normalizeResourceName(in); got != want {
+		if got := normalizeResourceName(providers.Workload{Name: in}); got != want {
 			t.Errorf("normalizeResourceName(%q) = %q, want %q", in, got, want)
 		}
 	}

internal/curator/replica_identity_test.go

@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package curator
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/Smana/runlore/internal/providers"
+)
+
+// TestIncidentKeyFoldsStatefulSetReplicas is #513 on the WRITE side: the same fault
+// on two replicas of one StatefulSet is one incident identity, exactly as it already
+// is on two pods of one Deployment — so the recurrence chain, suppression and the
+// dedup key stop restarting per replica. The Kind is what licenses the fold: a
+// kind-less numbered name is a name, and two databases must not key alike.
+func TestIncidentKeyFoldsStatefulSetReplicas(t *testing.T) {
+	pod := func(name string) providers.Workload {
+		return providers.Workload{Kind: "Pod", Namespace: "observability", Name: name}
+	}
+	a := IncidentKey("VMAgentHighMemory", pod("vmagent-vmagent-0"), "prod")
+	b := IncidentKey("VMAgentHighMemory", pod("vmagent-vmagent-1"), "prod")
+	if a == "" || a != b {
+		t.Fatalf("two replicas of one StatefulSet must key alike: %q vs %q", a, b)
+	}
+	if !strings.Contains(a, "|vmagent-vmagent|") {
+		t.Errorf("key must carry the StatefulSet family, got %q", a)
+	}
+	db := func(name string) providers.Workload {
+		return providers.Workload{Namespace: "observability", Name: name, Account: "111111111111"}
+	}
+	if IncidentKey("RDSCPUHigh", db("aurora-serverless-postgres-old-1"), "prod") ==
+		IncidentKey("RDSCPUHigh", db("aurora-serverless-postgres-old-2"), "prod") {
+		t.Fatal("two databases that differ by a trailing digit must not key alike")
+	}
+}
+
+// TestDupFingerprintFoldsStatefulSetReplicas: the curator's dedup fingerprint reads
+// the same identity, so a finding on replica 1 coalesces with the entry filed from
+// replica 0 instead of filing a second KB entry saying the same thing.
+func TestDupFingerprintFoldsStatefulSetReplicas(t *testing.T) {
+	base := providers.Investigation{
+		Resource:   providers.Workload{Kind: "Pod", Namespace: "observability", Name: "vmagent-vmagent-0"},
+		RootCauses: []providers.Hypothesis{{Summary: "scrape cardinality grew past the memory limit"}},
+	}
+	other := base
+	other.Resource = providers.Workload{Kind: "Pod", Namespace: "observability", Name: "vmagent-vmagent-1"}
+	if fa, fb := DupFingerprint(base), DupFingerprint(other); fa == "" || fa != fb {
+		t.Fatalf("same cause on two replicas must fingerprint alike: %q vs %q", fa, fb)
+	}
+}
+
+// TestDraftKBEntryFoldsStatefulSetReplica is #513 at the stored resource: an entry
+// written from a StatefulSet pod is filed under the StatefulSet, not the replica, so
+// the human-facing ref stops naming one of N identical pods and the READ side has
+// one family to match. A kind-less numbered resource is written as named.
+func TestDraftKBEntryFoldsStatefulSetReplica(t *testing.T) {
+	cases := []struct {
+		name     string
+		resource providers.Workload
+		want     string
+	}{
+		{"a StatefulSet pod is filed under its StatefulSet",
+			providers.Workload{Kind: "Pod", Namespace: "observability", Name: "vmagent-vmagent-1"},
+			"observability/vmagent-vmagent"},
+		{"a kind-less numbered resource is its own name",
+			providers.Workload{Namespace: "observability", Name: "aurora-serverless-postgres-old-1"},
+			"observability/aurora-serverless-postgres-old-1"},
+	}
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			inv := providers.Investigation{
+				Title:      "VMAgentHighMemory",
+				Confidence: 0.9,
+				Resource:   tt.resource,
+				RootCauses: []providers.Hypothesis{{Summary: "scrape cardinality grew", Evidence: []string{"e"}}},
+			}
+			if got := draftKBEntry(inv).Resource; got != tt.want {
+				t.Fatalf("KBEntry.Resource = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}

internal/investigate/recall.go

@@ -706,8 +706,10 @@ func refsAgree(reqW providers.Workload, b string) bool {
 		return true
 	}
 	// The request's name half is not re-cut out of the ref: reqW.ResourceID already
-	// holds it, resolved and qualified, and a namespace never contains a "/".
-	return reqW.ResourceID().Agrees(providers.ParseResourceID(bname))
+	// holds it, resolved and qualified, and a namespace never contains a "/". The
+	// entry's name is read the way the request's Kind licenses (#513: a pod-scoped
+	// request also agrees with an entry filed under a sibling StatefulSet replica).
+	return reqW.AgreesWithEntryName(bname)
 }
 
 func clampF(v, lo, hi float64) float64 {

internal/investigate/replica_identity_test.go

@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package investigate
+
+import (
+	"testing"
+
+	"github.com/Smana/runlore/internal/providers"
+)
+
+// TestResourceAgreesAcrossStatefulSetReplicas is #513 on the READ side. Live: 56
+// recall evaluations over 7 days, 0 accepted, 38 of them no_resource_match — a KB
+// holding observability/vmagent-vmagent-0 and -1 as two entries that an incident on
+// -2 could match neither of. A pod-scoped request folds its replica ordinal, and an
+// entry is read both as written and folded once, so an entry filed under a sibling
+// replica (legacy) and one filed under the StatefulSet (new) both agree.
+//
+// The Kind is the evidence: a kind-less request keeps its ordinal, so two databases
+// that differ only by a trailing digit stay two resources.
+func TestResourceAgreesAcrossStatefulSetReplicas(t *testing.T) {
+	pod := func(name string) providers.Workload {
+		return providers.Workload{Kind: "Pod", Namespace: "observability", Name: name}
+	}
+	cases := []struct {
+		name      string
+		reqW      providers.Workload
+		entry     string
+		requireWL bool
+		want      matchStrength
+	}{
+		{"a replica matches an entry filed under a sibling replica",
+			pod("vmagent-vmagent-2"), "observability/vmagent-vmagent-0", false, matchExact},
+		{"a replica matches an entry filed under the StatefulSet",
+			pod("vmagent-vmagent-2"), "observability/vmagent-vmagent", false, matchExact},
+		{"strict mode agrees too — it is one workload",
+			pod("vmagent-vmagent-2"), "observability/vmagent-vmagent-0", true, matchExact},
+		{"a StatefulSet whose own name ends in a digit is not folded past itself",
+			pod("shard-1-0"), "observability/shard-1", false, matchExact},
+		{"the same replica in another namespace is another workload",
+			pod("vmagent-vmagent-2"), "other/vmagent-vmagent-0", false, matchNone},
+		{"a replica does not agree with a different StatefulSet",
+			pod("vmagent-vmagent-2"), "observability/vmalert-vmalert-0", false, matchNone},
+		{"without a Kind the ordinal is the name: two databases stay two",
+			providers.Workload{Namespace: "observability", Name: "aurora-serverless-postgres-old-1"},
+			"observability/aurora-serverless-postgres-old-2", false, matchNone},
+		{"without a Kind a numbered name matches only itself",
+			providers.Workload{Namespace: "observability", Name: "vmagent-vmagent-2"},
+			"observability/vmagent-vmagent-0", false, matchNone},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := resourceAgrees(c.reqW, c.entry, c.requireWL); got != c.want {
+				t.Errorf("resourceAgrees(%+v, %q, %v) = %v, want %v", c.reqW, c.entry, c.requireWL, got, c.want)
+			}
+		})
+	}
+}

internal/providers/pod_name_test.go

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package providers_test
+
+import (
+	"testing"
+
+	"github.com/Smana/runlore/internal/providers"
+)
+
+// TestNormalizePodNameFoldsStatefulSetOrdinal pins the rule NormalizeWorkloadName
+// deliberately does NOT apply (#513): a StatefulSet pod is <name>-<ordinal>, and for
+// a name KNOWN to be a Pod's that ordinal is a per-replica suffix like a pod hash.
+// Only the Kind can say so — the same shape on a kind-less name is the name
+// (aurora-serverless-postgres-old-1 and -2 are two databases), which is why the
+// kind-less TestNormalizeWorkloadName keeps "vmagent-vmagent-0" and this does not.
+//
+// Exactly ONE ordinal is folded, not a fixed point: a StatefulSet named sts-0 has
+// pods sts-0-N, and folding to a fixed point would erase the StatefulSet's own name.
+// The function is therefore NOT idempotent on such a family, and the recall gate
+// compares an entry both as written and folded once rather than folding blindly.
+func TestNormalizePodNameFoldsStatefulSetOrdinal(t *testing.T) {
+	cases := map[string]string{
+		"vmagent-vmagent-0":  "vmagent-vmagent",
+		"vmagent-vmagent-1":  "vmagent-vmagent",
+		"vmagent-vmagent-12": "vmagent-vmagent",
+		"vmagent-vmagent":    "vmagent-vmagent", // the StatefulSet itself
+		"sts-0-3":            "sts-0",           // one ordinal: the family keeps its own digit
+
+		// Everything NormalizeWorkloadName already folds still folds, first.
+		"web-7d9c8b6f5-abcde":                       "web",
+		"harbor-registry-59598dbd57-ltkzw":          "harbor-registry",
+		"github-teams-sync-aqemia-29787720-3-mdft8": "github-teams-sync-aqemia", // Indexed CronJob pod
+
+		// A strip must never leave debris — the same guard as every other rule.
+		"x-0": "x-0",
+		"-0":  "-0",
+		"":    "",
+	}
+	for in, want := range cases {
+		if got := providers.NormalizePodName(in); got != want {
+			t.Errorf("NormalizePodName(%q) = %q, want %q", in, got, want)
+		}
+	}
+}

internal/providers/providers.go

@@ -246,7 +246,9 @@ const minFamilyName = 2
 // pod of an Indexed one) all collapse to <name>. The rules run to a FIXED POINT, so
 // a CronJob's pod sheds both its hash and its run stamp. Names without
 // such a suffix are returned unchanged, so real trailing words (e.g. "redis-cache")
-// and short numeric tails (e.g. "vmagent-vmagent-0") are preserved. It is idempotent.
+// and short numeric tails (e.g. "vmagent-vmagent-0") are preserved — a StatefulSet
+// replica ordinal is folded only where the Kind is known to be a Pod's (see
+// NormalizePodName and Workload.IdentityName). It is idempotent.
 //
 // The CronJob case was the expensive omission. A CronJob that fails once emits a
 // Job named for THAT run, and the run number reaches four separate consumers — the
@@ -277,6 +279,56 @@ func NormalizeWorkloadName(name string) string {
 	}
 }
 
+// NormalizePodName is NormalizeWorkloadName for a name KNOWN to be a Pod's: after
+// the per-instance rules it also folds ONE StatefulSet replica ordinal
+// (<statefulset>-<ordinal>) into the StatefulSet family, so vmagent-vmagent-0, -1
+// and -2 are one workload the way three pods of a Deployment already are (#513).
+//
+// It is a separate function, not another rule in NormalizeWorkloadName, because
+// only the Kind licenses the fold: on a name of unknown kind a short numeric tail is
+// ordinary naming — aurora-serverless-postgres-old-1 and -2 are two databases, and
+// ip-10-20-0-144 is a node — and folding it would fuse distinct resources in the
+// dedup key and the suppression chain. Callers that hold a Workload should read
+// Workload.IdentityName, which consults the Kind; this is the string-level rule.
+//
+// ONE ordinal, not a fixed point: a StatefulSet named sts-0 has pods sts-0-N, and
+// folding to a fixed point would erase the StatefulSet's own name. So this is NOT
+// idempotent on such a family — Workload.AgreesWithEntryName reads a stored entry
+// both as written and folded once for exactly that reason.
+func NormalizePodName(name string) string {
+	return stripReplicaOrdinal(NormalizeWorkloadName(name))
+}
+
+// stripReplicaOrdinal removes ONE trailing -<digits> segment, or returns its input
+// unchanged. It runs after the per-instance rules, so the tail it sees on a real pod
+// name is a StatefulSet ordinal: a hash or run stamp has already been folded.
+func stripReplicaOrdinal(name string) string {
+	i := strings.LastIndexByte(name, '-')
+	if i < 0 || !isOrdinal(name[i+1:]) {
+		return name
+	}
+	// The same debris guard as stripInstanceSuffix: a strip that leaves too little to
+	// be a name is not a family.
+	family := strings.TrimRight(name[:i], "-")
+	if len(family) < minFamilyName {
+		return name
+	}
+	return family
+}
+
+// isOrdinal reports whether s is a non-empty run of ASCII digits.
+func isOrdinal(s string) bool {
+	if s == "" {
+		return false
+	}
+	for i := range len(s) {
+		if s[i] < '0' || s[i] > '9' {
+			return false
+		}
+	}
+	return true
+}
+
 // stripInstanceSuffix removes at most ONE trailing per-instance suffix, or returns
 // its input unchanged. NormalizeWorkloadName drives it to a fixed point.
 func stripInstanceSuffix(name string) string {

internal/providers/replica_identity_test.go

@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package providers
+
+import "testing"
+
+// TestWorkloadIdentityFoldsReplicaOrdinalForPodsOnly pins #513 at the identity seam
+// every key and comparison reads through: a StatefulSet replica ordinal is a
+// per-instance suffix, but ONLY the Kind can say so, because the same shape on a
+// cloud resource or a controller is the name. Ingestion still keeps the raw pod name
+// (normalization is a comparison-time concern); this is where the Kind is consulted.
+func TestWorkloadIdentityFoldsReplicaOrdinalForPodsOnly(t *testing.T) {
+	cases := []struct {
+		name string
+		w    Workload
+		want string
+	}{
+		{"a StatefulSet pod folds to its StatefulSet",
+			Workload{Kind: "Pod", Namespace: "observability", Name: "vmagent-vmagent-2"}, "vmagent-vmagent"},
+		{"a Deployment pod still folds to its Deployment",
+			Workload{Kind: "Pod", Namespace: "tooling", Name: "harbor-registry-59598dbd57-ltkzw"}, "harbor-registry"},
+		{"a kind-less name keeps its numeric tail: two databases, not two replicas",
+			Workload{Namespace: "observability", Name: "aurora-serverless-postgres-old-1"}, "aurora-serverless-postgres-old-1"},
+		{"a controller whose own name ends in a digit is its own name",
+			Workload{Kind: "StatefulSet", Namespace: "apps", Name: "shard-1"}, "shard-1"},
+		{"an ARN-spelled cloud resource is unaffected",
+			Workload{Name: "arn:aws:rds:us-east-1:111111111111:db:datagrok-1"}, "datagrok-1"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := c.w.IdentityName(); got != c.want {
+				t.Errorf("IdentityName() = %q, want %q", got, c.want)
+			}
+			if got := c.w.ResourceID().Name; got != c.want {
+				t.Errorf("ResourceID().Name = %q, want %q", got, c.want)
+			}
+		})
+	}
+}