You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/09/22 05:43:13 UTC

[PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

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

   ## Problem / Evidence
   
   `AlertService.findRelatedAlerts` queried the ±30-minute correlation window with `transition="FIRING"` and page size 100 (single page). Two observable gaps:
   
   1. A cluster incident that fired more than 30 minutes ago and is still active only through its in-window REMINDER events is invisible to the related-alerts panel — even though `AlertNotificationSuppressionService.findSuppressingClusterAlert` deliberately treats that same incident as active, with an explicit comment: "REMINDER is emitted only while the state stays FIRING, so an incident whose latest in-window event is a REMINDER is still active."
   2. When more than 100 candidate events exist in the window, everything beyond the first page was silently dropped (the suppression counterpart pages until `total` is reached).
   
   Reproduced by a regression test: cluster incident with FIRING at T−40min and REMINDER at T−10min, business alert at T → the related panel omitted the incident before the fix.
   
   ## Root cause / Fix
   
   The window query was written for a narrower case than the suppression counterpart, which shares the same correlation-window semantics. The query no longer constrains `transition`; candidates are filtered in memory against `FIRING || REMINDER` (same rule and comment as `AlertNotificationSuppressionService`), and the query now pages through the window like the suppression service.
   
   ## Priority / scoring
   
   PRIORITY 55 = impact 20 (user-visible correlation gap: a live incident is missing from the panel while suppression acts on it) + blast radius 15 (system-alerts related panel for every cross-domain alert) + reproducibility 15 (deterministic once a reminder-only incident exists) + maintenance value 5 (aligns the two implementations of one semantic). FIX_CONFIDENCE 85.
   
   ## Tests
   
   - `mvn test -Dtest='AlertServiceTest#relatedAlertsShouldIncludeAnIncidentWhoseInWindowEventIsAReminderTest+relatedAlertsShouldReturnOtherDomainFiringEventsInTheSameInstanceAndScopeTest'`
     - pre-fix (fix stashed): `Tests run: 2, Failures: 2` (new test failed with the reminder-only incident missing; the existing test failed on the now-removed transition query argument)
     - post-fix: both pass
   - `mvn test -Dtest='AlertServiceTest,AlertNotificationSuppressionServiceTest,SystemAlertControllerTest'` → `Tests run: 102, Failures: 0, Errors: 0` (post-fix), BUILD SUCCESS
   
   ## Risk
   
   Low. Adds REMINDER-only incidents (which are genuinely active) to the panel and removes the 100-row ceiling; RESOLVED/other transitions are still excluded. No API shape change.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4873:
URL: https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-5929226691

   Thanks for the review — all three blockers are addressed in 406900ee:
   
   1. **Page-2 coverage** — new `relatedAlertsShouldFetchACandidateThatSitsOnPageTwoTest` stubs page 1 with 100 non-matching rows (total=150) and the matching candidate on page 2, and asserts `findAlertsPage` is called exactly twice. Against a single-page read this fails (the candidate never appears), so the paging half is now pinned.
   2. **Scan cap** — added `MAX_RELATED_CANDIDATE_PAGES = 5` (500 candidates); the loop breaks with `log.warn("Related-alert candidate scan for alert {} hit the {} page cap; candidates beyond page {} were not considered", ...)` when the cap is reached. New `relatedAlertsShouldStopTheCandidateScanAtThePageCapTest` pins it: with total=1000 stubbed, exactly 5 `findAlertsPage` calls happen (uncapped, the loop would walk to page 10 and the assertion fails).
   3. **Dedup** — window matches now merge by `fingerprint` keeping the latest event, mirroring `AlertNotificationSuppressionService`'s `later()` semantics (same null-safe comparison, as a private `laterEvent` helper). Rows without a fingerprint cannot collide and keep their own identity. New `relatedAlertsShouldMergeTheInWindowRowsOfOneIncidentKeepingTheLatestTest` builds one incident with a FIRING (-5 min) and a REMINDER (-2 min) row sharing a fingerprint and asserts the panel lists only the REMINDER row. One deliberate addition: explicit suppression causes are pinned by id and never dropped by the merge, because they can sit outside the display window and the UI points at them directly.
   
   `AlertServiceTest` 87/87 (3 new cases), checkstyle clean.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 closed pull request #4873: fix(alert): correlate a reminder-only cluster incident into related alerts
URL: https://github.com/apache/rocketmq-dashboard/pull/4873


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4873:
URL: https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-5772215271

   **Verification pass (fresh checkout of `fix/related-alerts-reminder-window` @ f47e512d)**
   
   Code citations in the description were re-checked against the branch:
   
   - `AlertService.findRelatedAlerts` → `SystemAlertController.java:70` (`GET /api/system-alerts/{id}/related`) is the only caller; the fix changes that read path only.
   - `AlertNotificationSuppressionService.java:67-69` carries the FIRING/REMINDER in-memory rule and comment the fix mirrors; its query also passes `transition=null`, so both implementations of the correlation-window semantic now agree.
   - The paging loop mirrors the suppression service's `fetched >= result.getTotal()` termination (re-verified here as `page * RELATED_CANDIDATE_PAGE_SIZE >= total`), removing the single-page 100-row ceiling.
   
   Tests re-executed from a clean checkout of this branch:
   
   - `mvn test -Dtest='AlertServiceTest,AlertNotificationSuppressionServiceTest,SystemAlertControllerTest'` → **Tests run: 102, Failures: 0, Errors: 0** (84+7+11), BUILD SUCCESS.
   - Pre-fix failures (development run, repository change stashed with the tests in place): the new `relatedAlertsShouldIncludeAnIncidentWhoseInWindowEventIsAReminderTest` failed because the reminder-only incident was missing from the panel, and `relatedAlertsShouldReturnOtherDomainFiringEventsInTheSameInstanceAndScopeTest` failed on the now-removed `transition="FIRING"` query assertion — 2/2 failed as documented.
   
   One wording correction was made to the description: the original reproduction sentence described a "FIRING at T−40min" event; the regression test actually models the incident by its in-window REMINDER row at T−10min (with a RESOLVED row at T−5min that must stay excluded), and the FIRING event itself is outside the window. The body now states this precisely.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4873:
URL: https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-5965575402

   ### CI status of this PR
   
   `Backend Tests (Java 21)` — **pass**; the change-specific command and its result are recorded in the Tests section above.
   
   The remaining red jobs are red for every branch, including `rocketmq-studio` itself, with the same logs:
   
   - `Backend Build (Java 21)` — fails inside the `binary-license-gate` antrun step (`exec returned: 1`); it also fails on a docs-only PR of this batch, and is addressed independently by #5341.
   - `Frontend Build (Node 20)` — fails after a successful build in the license manifest check (`license gate: build artifact verification failed: assets/index-<hash>.js`); identical on trunk and on a docs-only PR of this batch (run 37088829422), addressed independently by #5346.
   - `Frontend Docker Build` — depends on the frontend build above.
   
   This diff touches `server`, so none of the three can be caused by it.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4873:
URL: https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-6086400522

   ### CI status of this PR
   
   `Backend Tests (Java 21)` — **pass**; the change-specific commands and their results are in the Tests section of the description.
   
   The four remaining red jobs are red for every branch, including `rocketmq-studio` itself:
   
   - `Backend Build (Java 21)` — the `binary-license-gate` antrun step fails on a list of third-party jars (`spring-ai-*`, `swagger-ui`, `tencentcloud-sdk-*`, `zstd-jni`, ... : "missing complete upstream LICENSE/COPYING; a POM name or link is not a substitute"). This PR touches no build configuration; tracked by #5341.
   - `Frontend Build (Node 20)` / `Frontend Docker Build` — the frontend licence manifest check fails the same way on branches that do not touch `web/` at all; tracked by #5346.
   - `rmqctl Build (Go)` — `cmd` `TestMessageQueryCompletenessWarningTest` yaml subtests: `got {"items":[{"size":"3",...}]}, want {"size":3,...}`, i.e. the YAML renderer writes numbers as quoted strings where the test expects integer literals. Reproduced locally with `cd rmqctl && go test ./cmd/` from a checkout whose `rmqctl/` tree is byte-identical to `rocketmq-studio`'s, and owned by open PR #5850 ("keep yaml numbers unquoted").


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(alert): correlate a reminder-only cluster incident into related alerts [rocketmq-dashboard]

Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4873:
URL: https://github.com/apache/rocketmq-dashboard/pull/4873#issuecomment-6268053548

   Rebased the branch on the current `rocketmq-studio` (merge commit `1912a077`) to clear the Mockito import conflict in `AlertServiceTest`.
   
   Resolution: keep the static imports added on both sides (`times` from this PR; `lenient` and `verifyNoInteractions` from the base).
   
   Verified at `1912a077`: `mvn -Dtest=AlertServiceTest test` → 105 tests, 0 failures.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org