unbridled-41 opened a new pull request, #3145:
URL: https://github.com/apache/rocketmq-dashboard/pull/3145

   ## Problem / Evidence
   
   Fixes #3104 (filed today by ai-yang, no existing PR claims it).
   
   Native alert reconciliation 
(`NativeAlertProcessor.reconcileMissingActiveStates`) builds `presentKeys` by 
applying `NativeAlertRuleScopeMatcher`, which intentionally matches only the 
collection scope, instance id, and resource labels 
(consumerGroup/topic/brokerName/clusterName). It never requires the sample's 
metric to equal the rule's metric.
   
   Because an `AlertStateKey` fingerprint is derived from `ruleId + instanceId 
+ labels`, any other metric emitted with the same labels keeps a missing 
metric's stale state in `FIRING`/`ACKED` forever: no `RESOLVED` transition, no 
lifecycle event, and no recovery notification.
   
   Verified in the current tree before the fix: the new regression test
   `resolvesMissingMetricEvenWhenAnotherMetricSharesTheSameLabelsTest` fails 
because `AlertStateRepository.save` is never called for the active key (the 
`consumer.lag.total` sample keeps the `consumer.delay.seconds` rule's 
fingerprint present).
   
   ## Root cause / Fix
   
   `reconcileMissingActiveStates` now filters candidates with 
`sample.metricKey().equals(rule.getMetric())` before building `presentKeys`. A 
missing metric in a successful collection scope now resolves its prior active 
state even when another same-label metric remains. Behavior for a genuinely 
present target metric is unchanged 
(`keepsActiveFingerprintWhenItAppearsInSuccessfulCollectionScopeTest` still 
passes), and the full-scope-unavailable guard (`containsWholeScopeFailure`) is 
untouched.
   
   ## Priority & scoring
   
   - PRIORITY 83/100: impact 34/40 (alert state and recovery-notification path 
untrustworthy, operators see false incidents and miss recoveries), scope 15/20 
(all native alert users), reproducibility 18/20 (deterministic given the Apache 
business collector's stable `consumer.lag.total` vs conditional 
`consumer.delay.seconds` emission), maintenance value 16/20 (one-line filter, 
clear semantics).
   - FIX_CONFIDENCE 92/100: minimal filter at the exact root-cause site called 
out by the issue; covered by a red→green regression test.
   
   ## Tests
   
   - New: 
`NativeAlertProcessorTest.resolvesMissingMetricEvenWhenAnotherMetricSharesTheSameLabelsTest`
 — asserts `RESOLVED` state persistence, a `RESOLVED` lifecycle event carrying 
the original labels, and invocation of the recovery-notification enqueue path.
   - `mvn test -Dtest='NativeAlert*Test'`: 40/40 pass (18 in 
`NativeAlertProcessorTest`, regression test red before fix, green after).
   - `mvn test '-Dtest=org.apache.rocketmq.studio.ops.alert.*Test'`: all alert 
package tests pass.
   
   ## Risk
   
   Low: single predicate added to reconciliation key construction; 
evaluation-time behavior, suppression, and the whole-scope-unavailable guard 
are unchanged. The fix strictly widens when a stale state may resolve (only 
when the rule's own metric is absent from a successful scope).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to