Conversation
Rules whose stored instanceId has surrounding whitespace fire via NativeAlertRuleScopeMatcher (which trims) but were dropped from findActive and reconcileMissingActiveStates, so the operator saw no active alert while FIRING rows kept being written. Trim the stored value in both filters, matching the metric filter on the line above. Fixes apache#4297 Testing: MybatisPlusAlertStateRepositoryTest#findsActiveStatesForRulesWithPaddedStoredInstanceIdTest
RockteMQ-AI
left a comment
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Summary
Correct fix for the inconsistency between NativeAlertRuleScopeMatcher (which trims) and the query/reconcile paths (which didn't). The trim is now applied consistently in both findActive and reconcileMissingActiveStates, matching the metric filter pattern on the preceding line.
One minor observation: in NativeAlertProcessor.java, the original code used rule.getInstanceId() == null as the guard, while the fix changes it to !StringUtils.hasText(rule.getInstanceId()). This is a slight behavioral broadening (also treats empty string as "no instance filter"), but it's consistent with the MybatisPlusAlertStateRepository change and is the safer choice — an empty instanceId would never match a real scope anyway.
LGTM — well-tested with a clear reproduction scenario.
Automated review by github-manager-bot
2 participants
Summary
Fixes #4297
A rule whose stored
instanceIdhas surrounding whitespace fires viaNativeAlertRuleScopeMatcher(trims) but was dropped fromMybatisPlusAlertStateRepository.findActiveandNativeAlertProcessor.reconcileMissingActiveStates. Operators saw "no active alert" while FIRING rows kept being written.Trim the stored instance id in both filters, matching the metric filter on the previous line.
Testing
MybatisPlusAlertStateRepositoryTest#findsActiveStatesForRulesWithPaddedStoredInstanceIdTest(new)MybatisPlusAlertStateRepositoryTest: 6 passed locallyAI disclosure
Implemented with the assistance of an AI coding agent.