CAMEL-24590: Route group MBean aggregates performance statistics across all member routes - #26038
Merged
Conversation
…across all member routes The ManagedRouteGroup MBean extends ManagedPerformanceCounter and is fed via a CompositePerformanceCounter wired onto each member route. However a fresh ManagedRouteGroup instance was created per route, and only the first one was registered as the JMX MBean. Every route's counter was therefore wired to a different (mostly unregistered) instance, so the group MBean reflected only the first-registered member route instead of aggregating. This caused group-level counters such as getFailuresHandled() and getLastExchangeFailureHandledTimestamp() (and getExchangesCompleted/Failed, processing times, etc.) to be silently wrong. Fix by caching a single ManagedRouteGroup per group name so all member routes share the same counter, producing a true aggregate as documented. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 475 tested, 27 compile-only — current: 475 all testedMaveniverse Scalpel detected 502 affected modules (current approach: 475).
|
davsclaus
commented
Sep 2, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
Author
There was a problem hiding this comment.
Rules & conventions review — CAMEL-24590
Thanks for the fix, Claus — nicely diagnosed and well-tested. The root cause (a fresh ManagedRouteGroup per call, so only the first-registered route's instance became the JMX MBean while siblings' composite counters fed unregistered instances) is spot on, and caching one instance per group name is the right fix.
Verified:
- Null-group behavior is unchanged:
getManagedObjectForRouteGroupreturnsnullforgroup == null, and the newgroup != nullguard yields the samemrg == nullpath for ungrouped routes — no regression. - Eviction (
removeon last route,clearon stop) mirrors the previous unmanage logic. - Restarting a member route within a multi-route group now re-wires to the surviving cached instance.
- Test assertion style (JUnit) matches this module's convention; the updated
ManagedRouteGroupTestcorrectly replaces the buggy per-route expectation with the aggregate.
Two non-blocking notes below (one inline). No changes required.
Upgrade guide (optional): group-level MBean statistics now report aggregated (correct) values instead of a single arbitrary member route's counters. It's a bug fix restoring the documented semantics, so not strictly a migration concern — but operators monitoring these MBeans will see a step change in reported values. Might be worth a one-line note in the 4.x upgrade guide; your call.
Scope note: this is a project rules/conventions review — it does not replace CodeRabbit/Sourcery or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
| private final Map<Object, Object> managedThreadPools = new HashMap<>(); | ||
| // route group MBean is shared by all routes in the same group, so its performance counters | ||
| // aggregate the statistics across all the member routes | ||
| private final Map<String, ManagedRouteGroup> managedRouteGroups = new HashMap<>(); |
Contributor
Author
There was a problem hiding this comment.
Non-blocking: managedRouteGroups is a plain HashMap, so computeIfAbsent/remove here aren't synchronized. This is consistent with the existing convention in this class (managedThreadPools, managedBacklogTracers, managedBacklogDebuggers are all plain HashMap) and route lifecycle events aren't concurrent, so no new risk is introduced — just noting it for the record. No change requested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes CAMEL-24590.
ManagedRouteGroupMBean.getFailuresHandled()/getLastExchangeFailureHandledTimestamp()(and in fact all group-level performance statistics) did not aggregate across the group's member routes. They silently reported the counters of a single arbitrary member route — whichever route in the group was registered first.Root cause
ManagedRouteGroupextendsManagedPerformanceCounter(it owns a counter; it does not sum member routes on read). The group counter is fed through aCompositePerformanceCounterwired onto every member route inJmxManagementLifecycleStrategy.onRoutesAdd.The problem:
getManagedObjectForRouteGroup(...)creates a newManagedRouteGroupinstance on every call, so each member route's composite counter was wired to a different instance. Only the first route's instance was actually registered as the JMX MBean (subsequent registrations are skipped because the ObjectName already exists). Every other member route's events went to unregistered instances, sogetManagedRouteGroup(group)returned a proxy reflecting just the first-registered route.This is why the reproducer showed group
completed=1(a single member's count, not the sum of 3) andfailuresHandled=0(the failing route was not the first-registered one).Fix
Cache a single
ManagedRouteGroupper group name inJmxManagementLifecycleStrategyso all member routes share the same counter instance. The composite counter then aggregates every member route's events into the one registered MBean, producing a true group-wide total — matching the documented semantics ("total number of Exchange(s) that has failed within this group", seeroute-group.adoc). The cached instance is evicted when the group's last route is removed and cleared on stop.Tests
ManagedRouteGroupFailuresHandledTestmirrors the reporter's reproducer (handled failure on a trigger route that hops to sibling routes in the same group) and asserts the group reportsfailuresHandled=1, a non-null last-handled-failure timestamp, andexchangesCompleted=3(true aggregate).ManagedRouteGroupTestwhoseExchangesTotalexpectation previously encoded the buggy per-route value (1) — now asserts the aggregate (3for groupfirst,2for groupsecond).*RouteGroup* / *ManagedRoute* / *Statistic* / ManagedCamelContext*pass.Generated by Claude Code on behalf of davsclaus