You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "iloveeyjafjalla (via GitHub)" <gi...@apache.org> on 2026/10/16 14:46:12 UTC

[PR] fix(alerts): evaluate a rule test on the window the evaluator uses [rocketmq-dashboard]

iloveeyjafjalla opened a new pull request, #6886:
URL: https://github.com/apache/rocketmq-dashboard/pull/6886

   Fixes #6875
   
   ## The defect
   
   The rule test run evaluated the freshly collected sample, while the live evaluation first aggregates the samples stored for the same series over the rule's window. For a rule with a window and an aggregation other than `LAST` the preview therefore answered a different question than production: with `AVG` over 300s it reported the sample of the moment instead of the window average.
   
   ## The fix
   
   The aggregation now lives in one place, `NativeAlertWindowAggregation`, which both call sites use:
   
   - `NativeAlertEvaluationService` keeps the snapshot lookup and delegates the arithmetic to it — the behaviour it had, unchanged.
   - `NativeAlertRuleTestService` reads the same window (`MetricSnapshotRepository.findRecent(sample, collectedAt - windowSeconds)`) and evaluates the aggregated sample, so a rule's `窗口聚合` / `窗口秒数` mean the same thing in the preview and in production. When no window is stored — the same condition the evaluator falls back on — the raw sample is evaluated as before.
   
   ## Tests
   
   Red before the change:
   
   ```
   [ERROR] NativeAlertRuleTestServiceTest.aggregatesTheStoredWindowBeforeEvaluatingARuleTest:170
   Expecting actual:
     10.0
   to be close to:
     336.67
   ```
   
   The new case collects `10`, stores `[500, 500, 10]` for the window and asks for `AVG`/300s; it pins both the reported value and `conditionMatched`. The five existing cases of the class keep their meaning (a mocked repository returns an empty window).
   
   Green after, with the whole alert package:
   
   ```
   [INFO] Tests run: 427, Failures: 0, Errors: 0, Skipped: 0
   [INFO] You have 0 Checkstyle violations.
   ```
   
   `NativeAlertEvaluationTransactionTest` needs MySQL; it passes with the datasource the deployment uses (`SPRING_DATASOURCE_URL=...:13306/rocketmq`), which is also how it fails on a pristine checkout without it — the failure is the connection, not this 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(alerts): evaluate a rule test on the window the evaluator uses [rocketmq-dashboard]

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

   Same fix as this one, and we took #6876: it parameterises all five window aggregations and covers the empty-window, disabled-window and unavailable-sample edges (147 lines of tests against 35 here), on the same base.
   
   Worth porting into a follow-up if you want to: your pure `aggregate(rule, sample, window)` function signature is a nicer test seam than the helper in #6876, which reaches into the repository directly.
   
   Closing as superseded — thanks for the overlap, and sorry for the duplicated effort.


-- 
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(alerts): evaluate a rule test on the window the evaluator uses [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #6886: fix(alerts): evaluate a rule test on the window the evaluator uses
URL: https://github.com/apache/rocketmq-dashboard/pull/6886


-- 
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