This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 80dd201302ea CAMEL-25063: camel-management - Route and CamelContext
MBeans: fix bugs found in a deep review (#26946)
80dd201302ea is described below
commit 80dd201302eaffde681e37b56b9f3a117c3cae12
Author: Claus Ibsen <[email protected]>
AuthorDate: Mon Sep 28 09:13:40 2026 +0200
CAMEL-25063: camel-management - Route and CamelContext MBeans: fix bugs
found in a deep review (#26946)
- dumpStepStatsAsXml of the CamelContext is well-formed XML
- the generatedIds flag of dumpRouteAsXml/dumpRoutesAsXml/dumpRoutesAsYaml
is used (regression in 4.16)
- reset(true) of a route also resets its steps
- reset(true) of a route with * or ? in its id no longer resets other routes
- the percentiles no longer fail when concurrent exchanges take the count
past the window
- reset() also resets the load averages of a route, the context and a route
group
- the XML stats dumps escape quotes and the source location
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Signed-off-by: Claus Ibsen <[email protected]>
---
.../management/mbean/ManagedCamelContext.java | 20 +--
.../mbean/ManagedPerformanceCounter.java | 8 +-
.../camel/management/mbean/ManagedRoute.java | 40 +++--
.../camel/management/mbean/ManagedRouteGroup.java | 6 +
.../ManagedRouteAndContextEdgeCasesTest.java | 163 +++++++++++++++++++++
5 files changed, 208 insertions(+), 29 deletions(-)
diff --git
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedCamelContext.java
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedCamelContext.java
index 3bbcfe8b7805..c6ca85c46fc1 100644
---
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedCamelContext.java
+++
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedCamelContext.java
@@ -53,6 +53,7 @@ import org.apache.camel.spi.ManagementStrategy;
import org.apache.camel.spi.UnitOfWork;
import org.apache.camel.support.CamelContextHelper;
import org.apache.camel.support.PluginHelper;
+import org.apache.camel.util.StringHelper;
import org.apache.camel.util.json.JsonArray;
import org.apache.camel.util.json.JsonObject;
@@ -95,6 +96,7 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
@Override
public void reset() {
super.reset();
+ load.reset();
remoteExchangesTotal.reset();
remoteExchangesCompleted.reset();
remoteExchangesFailed.reset();
@@ -561,7 +563,7 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
@Override
public String dumpRoutesAsXml(boolean resolvePlaceholders, boolean
generatedIds) throws Exception {
- return dumpRoutesAsXml(resolvePlaceholders, true, false);
+ return dumpRoutesAsXml(resolvePlaceholders, generatedIds, false);
}
@Override
@@ -601,7 +603,7 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
@Override
public String dumpRoutesAsYaml(boolean resolvePlaceholders, boolean
uriAsParameters, boolean generatedIds)
throws Exception {
- return dumpRoutesAsYaml(resolvePlaceholders, uriAsParameters, true,
false);
+ return dumpRoutesAsYaml(resolvePlaceholders, uriAsParameters,
generatedIds, false);
}
@Override
@@ -710,7 +712,7 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
sb.append(String.format(" group=\"%s\"",
escapeXml(route.getRouteGroup())));
}
if (route.getSourceLocation() != null) {
- sb.append(String.format(" sourceLocation=\"%s\"",
route.getSourceLocation()));
+ sb.append(String.format(" sourceLocation=\"%s\"",
escapeXml(route.getSourceLocation())));
}
// use substring as we only want the attributes
@@ -871,7 +873,7 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
sb.append(String.format(" group=\"%s\"",
escapeXml(route.getRouteGroup())));
}
if (route.getSourceLocation() != null) {
- sb.append(String.format(" sourceLocation=\"%s\"",
route.getSourceLocation()));
+ sb.append(String.format(" sourceLocation=\"%s\"",
escapeXml(route.getSourceLocation())));
}
// use substring as we only want the attributes
@@ -895,9 +897,9 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
sb.append("
exchangesInflight=\"").append(step.getExchangesInflight()).append("\"");
sb.append(" ").append(stat, 7,
stat.length()).append("\n");
}
- sb.append(" </stepStats>\n");
}
- sb.append(" </stepStat>\n");
+ sb.append(" </stepStats>\n");
+ sb.append(" </routeStat>\n");
}
sb.append(" </routeStats>\n");
}
@@ -1016,10 +1018,8 @@ public class ManagedCamelContext extends
ManagedPerformanceCounter implements Ma
}
private static String escapeXml(String text) {
- return text
- .replace("&", "&")
- .replace("<", "<")
- .replace(">", ">");
+ // also quotes as the values are used in attributes
+ return StringHelper.xmlEncode(text);
}
}
diff --git
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedPerformanceCounter.java
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedPerformanceCounter.java
index 9bca27e455b6..40675eba41d5 100644
---
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedPerformanceCounter.java
+++
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedPerformanceCounter.java
@@ -390,10 +390,12 @@ public abstract class ManagedPerformanceCounter extends
ManagedCounter
if (percentileWindow == null || percentileCount == 0) {
return -1;
}
- long[] snapshot = new long[percentileCount];
- System.arraycopy(percentileWindow, 0, snapshot, 0, percentileCount);
+ // the count is updated without a lock by concurrent exchanges, so it
may go past the window size
+ int count = Math.min(percentileCount, PERCENTILE_WINDOW_SIZE);
+ long[] snapshot = new long[count];
+ System.arraycopy(percentileWindow, 0, snapshot, 0, count);
Arrays.sort(snapshot);
- int index = (int) Math.ceil(percentile * percentileCount) - 1;
+ int index = (int) Math.ceil(percentile * count) - 1;
return snapshot[Math.max(0, index)];
}
diff --git
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRoute.java
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRoute.java
index 49775a6e3976..780ea82cea6a 100644
---
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRoute.java
+++
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRoute.java
@@ -63,6 +63,7 @@ import org.apache.camel.spi.ManagementStrategy;
import org.apache.camel.spi.RoutePolicy;
import org.apache.camel.support.PluginHelper;
import org.apache.camel.util.ObjectHelper;
+import org.apache.camel.util.StringHelper;
import org.apache.camel.util.json.JsonArray;
import org.apache.camel.util.json.JsonObject;
import org.apache.camel.xml.LwModelHelper;
@@ -436,7 +437,7 @@ public class ManagedRoute extends ManagedPerformanceCounter
implements ManagedRo
@Override
public String dumpRouteAsXml(boolean resolvePlaceholders, boolean
generatedIds) throws Exception {
- return dumpRouteAsXml(resolvePlaceholders, true, false);
+ return dumpRouteAsXml(resolvePlaceholders, generatedIds, false);
}
@Override
@@ -589,7 +590,7 @@ public class ManagedRoute extends ManagedPerformanceCounter
implements ManagedRo
answer.append(String.format(" group=\"%s\"",
escapeXml(getRouteGroup())));
}
if (sourceLocation != null) {
- answer.append(String.format(" sourceLocation=\"%s\"",
getSourceLocation()));
+ answer.append(String.format(" sourceLocation=\"%s\"",
escapeXml(getSourceLocation())));
}
// use substring as we only want the attributes
String stat = dumpStatsAsXml(fullStats);
@@ -759,7 +760,7 @@ public class ManagedRoute extends ManagedPerformanceCounter
implements ManagedRo
answer.append(String.format(" group=\"%s\"",
escapeXml(getRouteGroup())));
}
if (sourceLocation != null) {
- answer.append(String.format(" sourceLocation=\"%s\"",
getSourceLocation()));
+ answer.append(String.format(" sourceLocation=\"%s\"",
escapeXml(getSourceLocation())));
}
// use substring as we only want the attributes
String stat = dumpStatsAsXml(fullStats);
@@ -812,7 +813,7 @@ public class ManagedRoute extends ManagedPerformanceCounter
implements ManagedRo
sb.append("\n <routeLocation")
.append(String.format(
" routeId=\"%s\" id=\"%s\" index=\"%s\"
sourceLocation=\"%s\" sourceLineNumber=\"%s\"/>",
- escapeXml(route.getRouteId()), id, 0,
location, line));
+ escapeXml(route.getRouteId()), escapeXml(id),
0, escapeXml(location), line));
}
for (ManagedProcessorMBean processor : processors) {
// the step must belong to this route
@@ -823,7 +824,7 @@ public class ManagedRoute extends ManagedPerformanceCounter
implements ManagedRo
.append(String.format(
" routeId=\"%s\" id=\"%s\" index=\"%s\"
sourceLocation=\"%s\" sourceLineNumber=\"%s\"/>",
escapeXml(route.getRouteId()),
escapeXml(processor.getProcessorId()), processor.getIndex(),
- location, line));
+ escapeXml(location), line));
}
}
}
@@ -831,10 +832,15 @@ public class ManagedRoute extends
ManagedPerformanceCounter implements ManagedRo
return sb.toString();
}
+ @Override
+ public void reset() {
+ super.reset();
+ load.reset();
+ }
+
@Override
public void reset(boolean includeProcessors) throws Exception {
reset();
- load.reset();
// and now reset all processors for this route
if (includeProcessors) {
@@ -842,12 +848,16 @@ public class ManagedRoute extends
ManagedPerformanceCounter implements ManagedRo
if (server != null) {
// get all the processor mbeans and sort them accordingly to
their index
String prefix =
getContext().getManagementStrategy().getManagementAgent().getIncludeHostName()
? "*/" : "";
- ObjectName query = ObjectName.getInstance(
- jmxDomain + ":context=" + prefix +
getContext().getManagementName() + ",type=processors,*");
- QueryExp queryExp = Query.match(new
AttributeValueExp("RouteId"), new StringValueExp(getRouteId()));
- Set<ObjectName> names = server.queryNames(query, queryExp);
- for (ObjectName name : names) {
- server.invoke(name, "reset", null, null);
+ // the route id must be equal (match would treat * and ? in
the route id as wildcards)
+ QueryExp queryExp = Query.eq(new AttributeValueExp("RouteId"),
new StringValueExp(getRouteId()));
+ // steps are registered as their own type
+ for (String type : new String[] { "processors", "steps" }) {
+ ObjectName query = ObjectName.getInstance(
+ jmxDomain + ":context=" + prefix +
getContext().getManagementName() + ",type=" + type + ",*");
+ Set<ObjectName> names = server.queryNames(query, queryExp);
+ for (ObjectName name : names) {
+ server.invoke(name, "reset", null, null);
+ }
}
}
}
@@ -1029,10 +1039,8 @@ public class ManagedRoute extends
ManagedPerformanceCounter implements ManagedRo
}
private static String escapeXml(String text) {
- return text
- .replace("&", "&")
- .replace("<", "<")
- .replace(">", ">");
+ // also quotes as the values are used in attributes
+ return StringHelper.xmlEncode(text);
}
}
diff --git
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRouteGroup.java
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRouteGroup.java
index fc42869151c0..dd554ff13bb4 100644
---
a/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRouteGroup.java
+++
b/core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedRouteGroup.java
@@ -45,6 +45,12 @@ public class ManagedRouteGroup extends
ManagedPerformanceCounter implements Mana
this.jmxDomain =
context.getManagementStrategy().getManagementAgent().getMBeanObjectDomainName();
}
+ @Override
+ public void reset() {
+ super.reset();
+ load.reset();
+ }
+
@Override
public void init(ManagementStrategy strategy) {
super.init(strategy);
diff --git
a/core/camel-management/src/test/java/org/apache/camel/management/ManagedRouteAndContextEdgeCasesTest.java
b/core/camel-management/src/test/java/org/apache/camel/management/ManagedRouteAndContextEdgeCasesTest.java
new file mode 100644
index 000000000000..1f94325126a6
--- /dev/null
+++
b/core/camel-management/src/test/java/org/apache/camel/management/ManagedRouteAndContextEdgeCasesTest.java
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.management;
+
+import java.io.StringReader;
+import java.lang.reflect.Field;
+
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import org.w3c.dom.Document;
+
+import org.xml.sax.InputSource;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.ManagementStatisticsLevel;
+import org.apache.camel.api.management.ManagedCamelContext;
+import org.apache.camel.api.management.mbean.ManagedCamelContextMBean;
+import org.apache.camel.api.management.mbean.ManagedRouteMBean;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.management.mbean.LoadTriplet;
+import org.apache.camel.management.mbean.ManagedPerformanceCounter;
+import org.apache.camel.management.mbean.ManagedRoute;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@DisabledOnOs(OS.AIX)
+public class ManagedRouteAndContextEdgeCasesTest extends ManagementTestSupport
{
+
+ @Override
+ protected CamelContext createCamelContext() throws Exception {
+ CamelContext context = super.createCamelContext();
+
context.getManagementStrategy().getManagementAgent().setStatisticsLevel(ManagementStatisticsLevel.Extended);
+ return context;
+ }
+
+ private ManagedCamelContext mcc() {
+ return
context.getCamelContextExtension().getContextPlugin(ManagedCamelContext.class);
+ }
+
+ private static Document parse(String xml) throws Exception {
+ return
DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new
InputSource(new StringReader(xml)));
+ }
+
+ @Test
+ public void testDumpStepStatsAsXmlIsWellFormed() throws Exception {
+ template.sendBody("direct:start", "Hello");
+ template.sendBody("direct:other", "Hello");
+
+ Document doc =
parse(mcc().getManagedCamelContext().dumpStepStatsAsXml(true));
+ // every route, with the steps of the route
+ assertEquals(3, doc.getElementsByTagName("routeStat").getLength());
+ assertEquals(3, doc.getElementsByTagName("stepStats").getLength());
+ assertEquals(2, doc.getElementsByTagName("stepStat").getLength());
+ }
+
+ @Test
+ public void testDumpWithoutGeneratedIds() throws Exception {
+ ManagedRouteMBean route = mcc().getManagedRoute("route1");
+ assertFalse(route.dumpRouteAsXml(false, false).contains("id=\"to"));
+ assertTrue(route.dumpRouteAsXml(false, true).contains("id=\"to"));
+
+ ManagedCamelContextMBean camel = mcc().getManagedCamelContext();
+ assertFalse(camel.dumpRoutesAsXml(false, false).contains("id=\"to"));
+ assertFalse(camel.dumpRoutesAsYaml(false, false, false).contains("id:
to"));
+ }
+
+ @Test
+ public void testResetIncludesSteps() throws Exception {
+ template.sendBody("direct:start", "Hello");
+ assertEquals(1, mcc().getManagedStep("foo").getExchangesTotal());
+
+ mcc().getManagedRoute("route1").reset(true);
+ assertEquals(0, mcc().getManagedStep("foo").getExchangesTotal());
+ }
+
+ @Test
+ public void testResetOfRouteIdWithWildcard() throws Exception {
+ template.sendBody("direct:star", "Hello");
+ template.sendBody("direct:start", "Hello");
+
+ mcc().getManagedRoute("rou*").reset(true);
+ assertEquals(0,
mcc().getManagedProcessor("starLog").getExchangesTotal());
+ // route1 is not the route rou* (but matches it as a wildcard)
+ assertEquals(1,
mcc().getManagedProcessor("fooLog").getExchangesTotal());
+ }
+
+ @Test
+ public void testPercentileWhenCountGoesPastTheWindow() throws Exception {
+ ManagedRoute route = new ManagedRoute(context,
context.getRoute("route1"));
+ route.init(context.getManagementStrategy());
+ route.completedExchange(template.send("direct:start", e ->
e.getMessage().setBody("Hello")), 5);
+
+ // the count is updated without a lock, so concurrent exchanges can
take it one past the window
+ Field count =
ManagedPerformanceCounter.class.getDeclaredField("percentileCount");
+ count.setAccessible(true);
+ count.setInt(route, 1025);
+
+ route.getProcessingTimeP50();
+ parse(route.dumpStatsAsXml(true));
+ }
+
+ @Test
+ public void testResetResetsLoad() throws Exception {
+ ManagedRoute route = new ManagedRoute(context,
context.getRoute("route1"));
+ route.init(context.getManagementStrategy());
+ Field field = ManagedRoute.class.getDeclaredField("load");
+ field.setAccessible(true);
+ LoadTriplet load = (LoadTriplet) field.get(route);
+ load.update(5);
+ assertFalse(route.getLoad01().isEmpty());
+
+ route.reset();
+ assertEquals("", route.getLoad01());
+ }
+
+ @Test
+ public void testDumpRouteStatsWithQuoteInGroup() throws Exception {
+ template.sendBody("direct:other", "Hello");
+ parse(mcc().getManagedRoute("other").dumpRouteStatsAsXml(true, true));
+ parse(mcc().getManagedCamelContext().dumpRoutesStatsAsXml(true, true));
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ from("direct:start").routeId("route1")
+ .step("foo")
+ .to("log:foo").id("fooLog")
+ .end()
+ .to("mock:result");
+
+ from("direct:other").routeId("other").group("my\"group")
+ .step("bar")
+ .to("log:bar").id("otherLog")
+ .end();
+
+ from("direct:star").routeId("rou*")
+ .to("log:star").id("starLog");
+ }
+ };
+ }
+}
