unbridled-41 opened a new pull request, #4900:
URL: https://github.com/apache/rocketmq-dashboard/pull/4900
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Which Issue(s) This PR Fixes
- No issue tracks this defect; the searches under *Duplicate check* found
none.
### Brief Description
The trace diagnostics badge claims a delivery failure for a trace whose
deliveries all succeeded.
`STATUS_KEY` maps the whole `critical` status to the delivery wording, but
`critical` has two
unrelated sources:
- a *failure*: `FAILED_TRACE_NODE`
(`web/src/utils/messageTraceDiagnostics.ts:203-205`) or
`FAILED_CONSUMER_DELIVERY` (`:363-365`), and
- a *latency hotspot*: `SLOW_TRACE_NODE` with `costTimeMs >=
criticalNodeCostMs` (`:191-193`,
used at `:252-256`) or `SLOW_END_TO_END_TRACE` with `endToEndLatencyMs >=
criticalEndToEndMs`
(`:522-536`).
Before this change (`web/src/utils/messageTraceDiagnostics.ts:118-122`):
```ts
const STATUS_KEY: Record<TraceDiagnosticStatus, string> = {
healthy: 'messagePage.traceStatusHealthy',
warning: 'messagePage.traceStatusWarning',
critical: 'messagePage.traceStatusDeliveryCritical',
};
...
statusKey: STATUS_KEY[status], // :565
```
`statusKey` is rendered verbatim as the badge text by
`web/src/pages/instance/message.tsx:229`
(`t(diagnostics.statusKey)`). So a trace of 31 s end to end with a
successful delivery and no
failed phase showed:
> 消息轨迹诊断 **[投递异常]** 首尾轨迹阶段相隔 31000 ms …
The tag next to the badge reports a latency problem while the badge reports
a delivery failure,
although nothing failed to be delivered. The dictionary already carries the
neutral key
`messagePage.traceStatusCritical` = `异常 / Critical`
(`web/src/i18n/translations.ts:575`), and
`message.tsx:211-215` even defines a `diagnosticStatusKey` map that uses it
— it is simply never
reached, because the component renders `diagnostics.statusKey`.
### Fix
Pick the label from the critical findings instead of from the status alone:
keep
`messagePage.traceStatusDeliveryCritical` when a critical
`FAILED_TRACE_NODE` or
`FAILED_CONSUMER_DELIVERY` finding is present, otherwise use the generic
`messagePage.traceStatusCritical`. Healthy and warning labels are unchanged,
and a failed step
still reads as a delivery failure exactly as before.
### Evidence (pre-fix, on the base commit `1ef5d860`)
```
$ cd web && npx vitest run src/utils/messageTraceDiagnostics.test.ts
✓ summarizes a complete trace as healthy
✓ returns guidance when trace nodes are missing
✓ flags failed and waiting trace phases with latency hotspots
× does not claim a delivery failure when only the latency is critical
AssertionError: expected 'messagePage.traceStatusDeliveryCritic…' to be
'messagePage.traceStatusCritical'
Tests 1 failed | 5 passed (6)
```
The new case builds a 3-node trace spanning 31 s with every phase far below
the 5 s critical node
cost and a single successful delivery; it asserts `status: 'critical'` with
`SLOW_END_TO_END_TRACE` and *no* failure finding, so the pre-fix badge text
is provably about a
latency-only critical.
### Duplicate check
Searched issues and PRs (open and closed) for `trace status`, `Delivery
Critical`, `投递异常`,
`statusKey`, `SLOW_END_TO_END_TRACE`, and read the open PRs that touch
`web/src/utils/messageTraceDiagnostics.ts` (none) and
`pages/instance/message.tsx`
(#4829 is a timestamp formatter, #4834/#4857 are byte sizes and Aliyun
bodies). The nearby
#4746 (mine) maps trace *node* statuses on the key lookup — a different
field. #2502 / #2576
merged earlier mapped failed nodes to an error step, which is what this
badge then mislabels.
### Scoring (AGENTS.md)
`PRIORITY` = impact 18 + reach 12 + reproducibility 18 + maintenance value
12 = **60**;
`FIX_CONFIDENCE` = **90**. (Impact: an operator reading a slow trace is told
a delivery failed,
which sends them to the wrong subsystem; reach: the diagnostics panel of
every trace lookup;
reproducibility: pure function, pinned by a deterministic test.)
### How Did You Test This Change?
```
$ cd web && npx vitest run src/utils/messageTraceDiagnostics.test.ts
Test Files 1 passed (1)
Tests 6 passed (6)
$ cd web && npx vitest run src/utils
src/pages/instance/__tests__/MessagePage.test.tsx
src/pages/instance/__tests__/MessagePageAsyncState.test.tsx
Test Files 22 passed (22)
Tests 164 passed (164)
$ cd web && npx tsc -b && npx eslint src/utils/messageTraceDiagnostics.ts
src/utils/messageTraceDiagnostics.test.ts
(no output, exit 0)
```
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added for the changed behaviour
- [x] New UI text: none — the change reuses the existing
`messagePage.traceStatusCritical` key that is already translated in both
languages
- [x] Architecture constraints stay green (no server change)
- [x] New source files: none
- [x] Documentation: not touched
### Risk
Low. Only the badge text of a critical trace changes, and only for criticals
that carry no failure
finding: a failed step keeps the delivery wording. Nothing keys off `status`
itself, which is why
the status, colour, score and issue list are untouched.
--
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]
