CAMEL-24328: Show JFR runtime data in TUI JFR tab - #25312
Conversation
Add a snapshot command to CamelJfrDevConsole that takes a point-in-time JFR recording snapshot, aggregates Camel events server-side, and returns JSON with five data sections: routes, processors, endpoints, failures, and redeliveries. Restructure the TUI JfrTab to display this data in navigable tables with view switching (1-5), sort cycling (s/S), and route-to-processor drill-down (Enter). Data refresh is on-demand via F5. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…ble data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…n TUI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet
left a comment
There was a problem hiding this comment.
🤖 AI-assisted review — Claude Code on behalf of @gnodet
Review: CAMEL-24328 — Show JFR runtime data in TUI JFR tab
Well-designed PR. The JFR snapshot command, dev console integration, and rich TUI tab with five navigable views (Routes, Processors, Endpoints, Failures, Redeliveries) are cleanly separated. Route drill-down, sorting, and JSON export for MCP are nice touches. Test coverage is solid.
Two observations — neither blocking:
1. [MINOR] Doc inconsistency — JFR tab group name
In camel-jbang-tui.adoc, the new "JFR Runtime Profiling" section says:
The JFR tab (under More > JVM)
But this PR moves the JFR tab from the JVM group to Observability (visible in TabRegistry.java where the group parameter changed from "JVM" to "Observability"). The bullet list earlier in the same doc file was correctly updated. This line should read "under More > Observability".
2. [MINOR] PageDown handler — getRowCount() recomputed 20× per keypress
The PageDown handler calls getRowCount() inside a loop that iterates 20 times:
for (int i = 0; i < 20; i++) {
tableState.selectNext(getRowCount());
}Each getRowCount() call delegates to sortedRoutes().size() / filteredProcessors().size() / sortedEndpoints().size(), which creates, populates, and sorts a new ArrayList — then discards it after calling .size(). That's 20 throwaway sorted lists per PageDown. Hoisting into a local would fix it:
int count = getRowCount();
for (int i = 0; i < 20; i++) {
tableState.selectNext(count);
}(Note: the PageUp handler doesn't have this issue since selectPrevious() takes no argument.)
Nits (non-blocking)
getTableDataAsJson()has no test coverage — since it exposes data to MCP agents, a simple assertion on the JSON structure would be a useful additionsortedRoutes()/filteredProcessors()/sortedEndpoints()are each called twice per render cycle (once for row count, once for rendering) — a per-frame cache could eliminate the redundancy if data sets grow
Overall, solid work 👍
…cling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…selector Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
- Routes view uses split layout with processor panel below for selected route - Remove numbered # column from all views, use >> selection indicator - Add per-view sorting with visual column indicators (s/S keys) - Remove Enter drill-down in favor of the split layout Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
- Routes sorts by route name alphabetically by default - Processors sorts by processor ID alphabetically by default - Endpoints sorts by endpoint URI alphabetically by default - All name columns show sort indicator in header Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
- Remove Loading... screen, show status header immediately - Auto-take snapshot when an active recording is found on tab open - F5 still available for manual snapshot refresh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Add MIN column, remove confusing RATE column, apply accent colors for IDs and timing-aware styles for durations. Add performance bars like the Routes tab. Fix loading state to show Loading instead of Disabled, fix race condition where snapshot prompt flashed during auto-load. Change F5 hint to refresh and title-case recording state with green color. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 549 tested, 27 compile-only — current: 547 all testedMaveniverse Scalpel detected 576 affected modules (current approach: 547).
|
gnodet
left a comment
There was a problem hiding this comment.
Thorough, well-structured PR — the JFR snapshot aggregation, TUI data views, and the CamelContextAware injection fix are all solid.
A few informational observations:
-
JSON API field rename:
registered→runtimeEventsand recording state casingRUNNING→Runningare technically breaking changes to the dev console JSON output. No other in-tree consumers were found (onlyJfrTab, updated here), but if any external scripts parse this output, it would be worth noting in the upgrade guide. -
Removed keybindings: The E (enable all), D (disable all), and J (generate .jfc) keybindings from the original JfrTab are removed as the tab shifts from event toggling to data display. The new controls are well documented in the help text.
-
Pre-existing bug fix: The
CamelContextAware.trySetCamelContext(startupStepRecorder, this)addition correctly fixes a bug wherecamelContextwas never injected intoFlightRecorderStartupStepRecorderbeforedoStart(), causing the runtime instrumentation install check to always fail during startup. Nice catch.
CI failures are unrelated Surefire fork timeouts in camel-jms.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
Summary
getTableDataAsJson()so AI agents (MCP) see the same data users seejfr.adocand TUI JFR profiling incamel-jbang-tui.adocskey across all views (route, processor, endpoint columns plus min/mean/max)Test plan
CamelJfrDevConsoleTest)JfrTabRenderTest)camel run --jfrwith a route, open TUI, verify auto-snapshot loads dataskey in Routes, Processors, Endpoints viewsClaude Code on behalf of davsclaus
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com