unbridled-41 opened a new pull request, #5889:
URL: https://github.com/apache/rocketmq-dashboard/pull/5889
### Which Issue(s) This PR Fixes
Fixes #5888
### Problem / Evidence
`AlertNotificationTemplate` renders `${value}` (every metric except the
percent branch) and `${threshold}` with `String.valueOf(double)`:
```java
// server/.../ops/alert/AlertNotificationTemplate.java:54
values.put("threshold", rule == null ? "" :
String.valueOf(rule.getThreshold()));
...
return String.valueOf(currentValue); // :76, non-ratio branch
```
`Double.toString` switches to computerized scientific notation at 1e7, so a
consumer-lag alert whose current value is 12,500,000 - the number this alert
family exists for - is delivered to DingTalk/SMS/email as `1.25E7`, and the
threshold as `1.2E7`:
```
expected: "12500000/12000000messages"
but was: "1.25E7/1.2E7messages"
```
The operator cannot see it coming: the rule dialog previews the same two
variables with JavaScript `String(context.value)`
(`web/src/utils/alertTemplatePreview.ts:139`), which prints `12500000`, and the
unit is appended verbatim, so the delivered text reads `1.25E7messages`. The
same file already fixed this divergence for the percent branch through
BigDecimal, with a comment naming the artifact ("plain double arithmetic
renders the stored 0.29 as 28.999999999999996"); the plain branch was the
residual.
### Root cause / Fix
The plain branch was never given a decimal renderer. Route both placeholders
through one `formatNumber` that returns
`BigDecimal.valueOf(value).toPlainString()` for a finite value and keeps
`String.valueOf` only for a non-finite one (which has no decimal form). The
percent branch keeps its existing `movePointRight(2)` scaling.
### Priority and scoring
**PRIORITY 48** — impact 16/40 (a delivered notification body that
contradicts the preview; no data loss, no crash), blast radius 14/20 (every
rule template that interpolates the measurement, for the metrics whose values
are large), reproducibility 18/20 (deterministic, pinned by the new test),
maintenance value 10/20 (removes the last `Double.toString` display path in an
alert file that had already fixed its sibling).
**FIX_CONFIDENCE 92** — one helper, same technique the file already uses,
all existing assertions unaffected.
### Tests
`cd server && mvn -o -B -ntp test
-Dtest='org.apache.rocketmq.studio.ops.alert.**'`
| Test | Before | After |
|---|---|---|
|
`AlertNotificationTemplateTest#rendersLargeValuesAndThresholdsWithoutScientificNotationTest`
| FAIL `was: "1.25E7/1.2E7messages"` | PASS |
|
`AlertNotificationTemplateTest#rendersNonFiniteValuesWithoutAPlainDecimalFormTest`
| PASS (guards the new guard) | PASS |
`Tests run: 328, Failures: 0, Errors: 0` for the alert package, including
the two assertions that pin the existing rendering of ordinary values
(`86.5%/85.0`, `Disk warning: 86.0 on local`) - both untouched. `mvn -o -B -ntp
checkstyle:check` passes.
### Risk
The only observable change is the notation for finite values that
`Double.toString` would have written with an exponent; every other rendering is
byte-identical, which the two pre-existing pinned assertions demonstrate.
--
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]
