Repository navigation
Dedup analysis findings: fold anomalies into incidents + per-incident alerts (Stage B change 3) - #1493
Merged
Conversation
… alerts (Stage B change 3) Anomaly detections and the regular finding for the same symptom stopped rendering as two cards / firing two emails for one event. - New shared AnomalyIncidentReconciler (pure): after clustering + stamping, rewrites each ANOMALY_* story's incident id onto the same-symptom, same-run, same-database regular finding. Fold-not-suppress; a no-parent anomaly stays solo; a db-scoped ANOMALY_OBJECT_* never folds across databases. Family map mirrors FactCollectorHelpers grouping exactly (CX*->CXPACKET, general lock modes->LCK, LCK_M_S/range/schema kept separate); ANOMALY_WAIT_PROFILE resolves to the family of its dominant contrib_<TYPE> driver. - Wired one Reconcile call into all three analysis services (Darling, Lite, Dashboard) between StampClusters and FilterMuted. - AnalysisNotificationService now cools down and groups by incident_id (falling back to StoryPathHash for legacy/absolution findings so those never collapse together), sending one alert per incident led by the highest-severity finding and naming the co-fired rest via CoFiredSummary. - InferenceEngine.ClusterIntoIncidents no longer unions two db-scoped findings from DIFFERENT databases into one incident (server-scoped findings still bridge freely, so same-db/server-wide incidents unaffected). Tests: new AnomalyIncidentReconcilerTests in Lite.Tests + Dashboard.Tests; cross-DB clustering tests in InferenceEngineTests; incident email-dedup + legacy-empty-id regression tests in AnalysisNotificationTests. Full suites green (Dashboard 714, Lite 1067, Darling 2066 + 133 live-PG skipped). Does not touch FactScorer (the severity cap is change 4, a separate PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed co-fired wording
Finding 2 (effectiveness gap): AnomalyIncidentReconciler indexed fold targets
only by each regular story's RootFactKey, so an anomaly folded only when a story
was ROOTED on its family key. But mapped families (CPU_SQL_PERCENT,
RESOURCE_SEMAPHORE, BLOCKING_EVENTS, ...) are frequently consumed as NON-ROOT
members of a larger story when a higher-severity neighbor roots it
(SOS_SCHEDULER_YIELD -> CPU_SQL_PERCENT, THREADPOOL/LCK -> BLOCKING_EVENTS) --
exactly the correlated runs where dedup matters most -- and the fold missed
there. Now index fold targets by EVERY fact key in each regular story's Path
(root + mid/leaf members). BuildStories consumes each fact into exactly one
story, so a family key belongs to at most one story's path: unambiguous, no
over-fold. Also: ANOMALY_CPU_SPIKE now accepts either CPU family target --
sustained CPU_SQL_PERCENT or burst CPU_SPIKE -- so a run keyed on either folds.
All existing guards preserved (same-run, same-DatabaseName, parent-exists,
ANOMALY_OBJECT_* never cross-DB, fold-not-suppress).
Finding 4 (cosmetic): the co-fired AlertDetailItem heading is "Co-fired in this
incident" but its body came from CoFiredSummary.Line's hardcoded window wording
("Also surfaced in this analysis window:") -- inaccurate for an incident-scoped
list. Added an optional leadIn param to Line (defaulting to the window wording,
so viewer/Dashboard/Lite readers stay byte-identical) and pass an incident-scoped
label from AnalysisNotificationService. Lone-finding messages unchanged.
Tests: added 6 cases each to Lite.Tests + Dashboard.Tests
AnomalyIncidentReconcilerTests exercising the untested non-root-member fold path
(CPU_SQL_PERCENT / BLOCKING_EVENTS as members, CPU_SPIKE root + member) plus
no-over-fold cases; updated the Lite notification test to assert the new
incident-scoped body wording.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #1493 incident dedup held a NEW notify-worthy symptom that co-clustered into an already-alerted incident for the full re-notify cooldown (~6h), because NotifyAsync cooled and skipped the whole incident group once its IncidentId bucket was stamped. A new CRITICAL could sit silently for hours. Escalate-on-CRITICAL: key the re-notify bucket per-member. A CRITICAL-band finding (severity >= the shared 1.5 cutoff the Dashboard/Lite/Darling recommendation readers band CRITICAL by) is cooled on its OWN StoryPathHash bucket, so a new critical joining an already-alerted incident e-mails fresh, led by that critical. A below-critical finding stays on the shared per-incident bucket, so co-fired non-critical symptoms remain one-e-mail-per-incident. Send-if-any-bucket-fresh, stamp-all (the rule the live IncidentCooldown path already uses): one e-mail per incident per cycle led by the highest-severity member whose bucket is not cooling, naming the rest as co-fired, then every member's bucket is stamped. This preserves intra-cycle collapse (two criticals co-firing in one cycle still send one e-mail) while guaranteeing cross-cycle escalation. Keyed off a named CriticalSeverityCutoff constant (no importable shared constant exists; PerformanceMonitor.Analysis has none and the readers each spell 1.5 inline). Guarantees covered by new/updated Lite.Tests cases: a lone critical e-mails once and respects its own cooldown; a new critical (incl. a lower-severity one) joining an already-alerted incident sends a fresh e-mail naming it; below- critical co-fired symptoms stay held under the incident bucket; the empty- IncidentId fallback still works; distinct incidents never collapse. The prior all-severities incident-cooldown test is re-scoped to below-critical (critical now escalates). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Stage B, change 3 — dedup (incident reconcile + per-incident alerts + cross-DB fix)
Implements the dedup slice of the recs-engine remainder (Round-2 Proposal 1, VETTED). An
ANOMALY_*detection and the REGULAR finding for the same symptom stopped surfacing as two separate viewer cards and (below the default alert threshold, or when it's lowered) two separate e-mails for one event. Fold, don't suppress; don't add graph edges. Does not touchFactScorerseverity/cap logic — that is change 4, a separate PR awaiting an owner decision.What changed
AnomalyIncidentReconciler(PerformanceMonitor.Analysis, pure/testable). AfterClusterIntoIncidents+StampClusters, for each survivingANOMALY_*story it rewrites the story'sIncidentIdonto the REGULAR finding of the matching symptom family — only when that parent exists in the same run and same database. A no-parent anomaly stays solo; a db-scopedANOMALY_OBJECT_*never folds across databases. The family map mirrors the collector's grouping exactly via a newFactCollectorHelpers.WaitFamilyKey(CX* → CXPACKET, general lock modes → LCK,LCK_M_S/range/schema kept separate);ANOMALY_WAIT_PROFILEresolves to the family of its dominantcontrib_<TYPE>driver. Nothing is dropped — only the incident tag is reconciled, preserving the anomaly's vs-baseline delta and any sub-threshold / no-regular-path signal.Reconcilecall into each of the three analysis services (Darling / Lite / Dashboard), at the identical site betweenStampClustersandFilterMuted.AnalysisNotificationService.NotifyAsync: the re-notify cooldown key moves fromStoryPathHashtoIncidentId(falling back toStoryPathHashwhen the id is empty, so legacy/absolution findings never collapse into one bucket), and within a batch findings are grouped by incident to send one alert per incident — led by the highest-severity primary, with the co-fired rest named in that one message via the sharedCoFiredSummary.InferenceEngine.ClusterIntoIncidentsno longer unions two db-scoped findings from different databases into one incident. Server-scoped findings (no database) still bridge freely, so same-database and server-wide incidents are unaffected (verified — no over-fragmentation).Tests
AnomalyIncidentReconcilerTestsin both Lite.Tests and Dashboard.Tests: CX-profile folds into a CXPACKET incident, general-lock/reader-lock family routing, CPU fold, DB-awareness (no fold across differing DB), no-parent stays solo,ANOMALY_OBJECT_*never cross-DB folds, unmapped anomalies stay solo, absolution ignored, null/single no-op.InferenceEngineTests(different-DB do not merge; same-DB still merge; server-scoped bridges to db-scoped).AnalysisNotificationTests(one alert per incident + co-fired naming; lone finding has no co-fired item; cooldown keyed by incident; legacy empty-id keeps per-hash buckets).Full suites green: Dashboard 714/714, Lite 1067/1067, Darling 2066 passed + 133 live-PG skipped, 0 failures. All affected projects build clean.
🤖 Generated with Claude Code