lizhimins commented on PR #4998: URL: https://github.com/apache/rocketmq-dashboard/pull/4998#issuecomment-5806622953
This should not be merged: the capability already exists on the frontend, and the way the aggregator is wired breaks both compilation and application startup. 1. `MessageService` is `@Service @RequiredArgsConstructor` with five final fields on this branch, the fifth being `ResourceOwnershipGuard` (`MessageService.java:33-49`). This PR deletes `@RequiredArgsConstructor` and hand-writes two constructors (4-arg and 5-arg with the aggregator), neither annotated `@Autowired` and neither accepting `ownershipGuard`. That leaves a final field uninitialized (compile error), gives Spring two equally valid constructors to choose from (startup failure), and `new MessageTraceLifecycleAggregator()` inside the 4-arg one bypasses the `@Component` bean entirely. `MessageServiceTest` constructs the service as `(provider, registry, history, audit, ownershipGuard())` in eight places (lines 82-204), which no longer matches. Please keep `@RequiredArgsConstructor` and add the aggregator as one more final field. 2. Trace waterfall / latency-bottleneck analysis is already implemented and tested: `web/src/utils/messageTraceDiagnostics.ts` builds per-phase `costTimeMs`, `latencyFromPreviousMs`, `latencyFromStartMs`, an end-to-end latency, `TraceLatencyHotspot` (slowest node and slowest gap) and issue classification via `analyzeMessageTrace`. A backend parallel that only recognises two nodes (title contains PUB/SEND vs SUB/CONSUME, last match wins) and drops the intermediate `SubBefore` spans reports strictly less, with a second, divergent vocabulary. 3. `transitDuration = subTime - (pubTime + pubCost)` mixes timestamps reported by different clients with no clock-skew handling, then labels the result "broker storage transit" — on a skewed producer/consumer pair this attributes client clock error to the broker. 4. All five `MessageTraceLifecycleAggregatorTest` methods are named `testXxx` instead of the `...Test` suffix, and they feed hand-built `TraceNodeVO` fixtures straight into `aggregate()`, so nothing covers the real `getMessageTrace` → provider path. 5. `GET /api/messages/trace-waterfall` has no consumer (no frontend, no AI tool) and is missing from `docs/api-spec.md`. The branch also currently conflicts with `MessageService.java`. If the goal is to expose stage latency to the AI/CLI surface, the better slice is to reuse the existing phase/hotspot model (or lift it into a shared contract) and add it to `rmq.message.trace`'s output rather than a new endpoint. -- 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]
