fix(nb-36647): fingerprint findings per resource, not per instance - #580
Merged
Conversation
Three matchers mixed an instance-unique value into their fingerprint, so two occurrences of the same problem never looked alike and the server's occurrence chain (event_duplicates, keyed on fingerprint) never formed. - ConfigurationChange hashed metadata.resourceVersion, which advances on every write. All 8824 of these in 30d of prod had occurrence_number=1. Now keyed on (namespace, kind, name). - job_failure hashed metadata.uid, new per run. Now prefers the CronJob owner and falls back to the job family. - image_pull_backoff resolved the owner but stopped at the Job, whose name carries a per-run suffix. Now collapses a Job owner to its family. Adds JobFamily(), which strips generated per-run Job-name suffixes (-<digits>, -<8+ hex>, -<uuid>) in the same spirit as the existing stripPodTemplateHash heuristic. Directly-created Jobs have no ownerReferences to walk, so the name is the only identity available. job_failure also gains a 10m rate limit; its RateLimit:0 was justified by per-UID fingerprints, and prod shows single failed Jobs emitting ~68 times because the transition gate cannot be relied on (kubewatch pointer aliasing). The occurrence chain still counts every repeat. Replica collapse is unchanged and now covered by a regression test. Removes metaUID, orphaned by the job_failure change. Projected against 30d of prod: ConfigurationChange 5028 -> 267 fingerprints, job_failure 538 -> 36, image_pull_backoff 4008 -> 72.
There was a problem hiding this comment.
Code Review
This pull request modifies how fingerprints are generated for Kubernetes Jobs and resource changes to ensure that repeated runs of the same logical job or repeated edits to the same resource share a single fingerprint. It introduces a JobFamily helper to strip generated per-run suffixes (such as UUIDs or timestamp-based digits) from Job names. Additionally, it updates the imagePullBackoffMatcher, jobFailureMatcher, and babysitterChangeMatcher to leverage this family-based grouping instead of using unique identifiers like resourceVersion or uid. Corresponding unit tests have been added to verify these fingerprinting and collapsing behaviors. There are no review comments to address.
Contributor
|
📦 Image Tags Updated |
blue4209211
approved these changes
Aug 20, 2026
mayankpande88
added a commit
that referenced
this pull request
Aug 20, 2026
Follow-up to #580, caught verifying that change on the dev cluster. #580 collapsed a Job owner to its family so repeated runs would share a fingerprint, but kept the failing image in the hash. For a one-Job-per- image creator the image IS the per-run identity, so it re-forked the fingerprint immediately: after #580 shipped, every trivy-image-scan Job in nudgebee-agent-dev still had its own fingerprint, each pulling a different image under scan. Drop the image only when we collapsed a Job owner. Ordinary workloads keep it, so a single typo'd container is still its own Finding. The failing image remains in the evidence blocks either way. Measured on 30d of prod: job-owned 3961 -> 9 fingerprints. Every other owner kind is unchanged (deployment 35, daemonset 8, statefulset 3) — the image component only ever forked identity for Job-owned Pods.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three agent matchers built each problem's identity from a value unique to that one instance, so two occurrences of the same problem never produced the same fingerprint.
Dedup in this product is not the
eventsupsert — that keys onfinding_id, which is a fresh UUID per finding by design (one row per occurrence). Dedup is the occurrence chain inevent_duplicates, keyed on(fingerprint, cloud_account_id). A fingerprint that changes every time therefore starts a fresh chain of length 1 every time, and the recurring problem never collapses into one entry with a repeat count.ConfigurationChange/KubernetesResource/Changemetadata.resourceVersionjob_failuremetadata.uidimage_pull_backoff_reporterAdds
JobFamily(), which strips generated per-run Job-name suffixes (-<digits>,-<8+ hex>,-<uuid>) in the same spirit as the existingstripPodTemplateHashheuristic. This is needed because a directly-created Job has noownerReferencesto walk — its own per-run name is the only identity available, and that is the dominant case in production.job_failurealso gains a 10m rate limit. ItsRateLimit: 0was justified by per-UID fingerprints ("terminal — fingerprint by UID is enough"); with a shared fingerprint, leaving it unlimited would re-emit for as long as the failed Job lingers. The transition gate can't be relied on to prevent that — same kubewatch pointer-aliasing caveat already documented forpod_crash_loop, and prod shows single failed Jobs emitting ~68 times. The occurrence chain still counts every repeat; this only bounds emission rate.Evidence
Occurrence chains in production before this change (30d, 8 accounts):
report_crash_loopis the control case — it chains correctly, proving the mechanism works when the fingerprint is stable.Replaying the new identity rules over the same 30 days of production rows:
I also reviewed every Job name the new suffix rule rewrites in that window; the transformations are all of the intended shape (
trivy-image-scan-24e032a5→trivy-image-scan,blinq-api-healthchecks-29764215→blinq-api-healthchecks,pinot-segment-push-v2-29751385→pinot-segment-push-v2). The UUID rule was added after that review caughtnb-llm-ct-<uuid>being only partially stripped.cd runner && make validate— exit 0, 37 packages, no failures.Not doing
finding_iddeterministic to force theeventsupsert to collapse rows. The idempotency guard intriage/processor.goreturns early when a chain row already exists for anevent_id, so a stablefinding_idwould freezeoccurrence_numberat 1 permanently — destroying the signal this PR restores. #35979 hit the same symptom for Prometheus alerts and chose a notification cooldown instead.fp(). The chain query is already scoped bycloud_account_id, and there are zero same-account cross-cluster fingerprint collisions in 30d of prod. Rotating every fingerprint would orphan every open chain for no observable gain.node_unschedulable(per cordon episode, ~6 fingerprints/day) orreport_crash_loop(hour-bucketed, chains correctly). Neither shows fanout in the data.Risk
JobFamilyis a heuristic and errs toward collapsing: two genuinely different Jobs differing only by a generated-looking tail become one family. That is the grouping we want here, and the 5-digit floor on the numeric form keeps meaningful tails likepostgres-15intact. Existing open chains for the three affected matchers will not match the new fingerprints, so each affected problem starts one fresh chain after rollout — a one-time effect, not ongoing.Replica collapse (already working) is unchanged and now has a regression test.
Fixes nudgebee/nudgebee-enterprise#36647