This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch CAMEL-26797-chaos-lost-handoff in repository https://gitbox.apache.org/repos/asf/camel.git
commit 04b36d08bd81a20ca5144114ac54ed8ee826805f Author: Claus Ibsen <[email protected]> AuthorDate: Tue Nov 10 09:07:01 2026 +0100 CAMEL-26797: camel-chaos - an experiment does not pass while the route of its expectation failed uncounted, and a lost expectation between routes that are no hand-off warns Found by camel cmd chaos-explore on the chaos example: refunds->payment:lost<=0 passed while 7 refunds failed, as refunds calls payment over HTTP (no hand-off), the loss only counts completed exchanges, and payment also answered the orders. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../org/apache/camel/catalog/docs/chaos.adoc | 13 +++- components/camel-chaos/src/main/docs/chaos.adoc | 13 +++- .../apache/camel/component/chaos/ChaosAdvisor.java | 73 ++++++++++++++++++++++ .../camel/component/chaos/ChaosExperiment.java | 41 +++++++++++- .../apache/camel/component/chaos/ChaosManager.java | 4 ++ .../apache/camel/component/chaos/ChaosTargets.java | 2 +- .../camel/component/chaos/ChaosExperimentTest.java | 53 ++++++++++++++++ 7 files changed, 193 insertions(+), 6 deletions(-) diff --git a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/chaos.adoc b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/chaos.adoc index 3a4f00f07c39..f833886500ab 100644 --- a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/chaos.adoc +++ b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/chaos.adoc @@ -165,6 +165,13 @@ A rule that expects failed exchanges in a route whose error handler is a dead le (`expect=parked:failed<=0`) has a warning too: the dead letter channel handles what fails there, so nothing fails, and the expectation is always met. `handled` counts what the dead letter channel handled (`expect=parked:handled<=0`). +So does a rule that expects no message lost between two routes (`expect=refunds->payment:lost<=0`) when the first +route does not hand messages off to the second (over `seda`, a broker or a `wireTap`): when it calls the second over +`direct` and waits, a failure there fails the first route and nothing is lost between them; when it does not send to it +at all (an HTTP call to a `platform-http` route), the failures of the first route are not in the loss. `failed` counts +them (`expect=refunds:failed<=0`). A route with a send that is known only when it sends (a `toD`, `wireTap` or +`recipientList`) is not checked. + To see what a rule would hit before it is added, `chaos-add --dry-run` checks it as `chaos-add` would, and adds nothing. It prints the rule in plain words, then each send, poll, route or step it matches now, with its route (and the step that sends), the sends known only when they send that may match (a `toD`), and the warnings: @@ -400,7 +407,9 @@ report does. * `INCONCLUSIVE`: it ended before an expectation was decided, so the messages may still have come; none of its faults fired, so its expectations say nothing about them; or an expectation was met on a route that had no messages while it ran (`refunds:failed<=0` when the traffic went to another route), or is a time (`maxTime`, `meanTime`) of -such a route. The `reason` says which. +such a route; or a route of an expectation failed exchanges that no expectation of it counts +(`refunds->payment:lost<=0` while refunds failed: the loss counts what completed). The `reason` says which, and names +the expectation that counts them. The routes that failed while it ran, and whose failures no expectation of the experiment counts, are listed as `alsoFailed`, with how many failed. An experiment that expects `refunds:maxTime<=30s` passes while 2 refunds failed, @@ -1011,6 +1020,8 @@ delay rule slows the route as it should). difference is the messages lost (never below 0, as the second route may also get messages from elsewhere). Messages still on their way when the time is up count as lost too: let `within=` run past the `until=` of the faults, so the last ones can arrive. The messages it counts as lost are added to the lost of the report. The op is `<=` or `=`. +* A failed exchange of the first route is not counted either: it did not complete, so the loss does not see it. An +experiment with such failures in the route of its expectation is `INCONCLUSIVE`, not `PASS`. * An exchange of the first route that its error handler handled, such as an order the dead letter channel parked, is not counted in the first route: it is not meant to reach the second one, so it is not lost. In the second route a handled exchange counts as completed, as it arrived. diff --git a/components/camel-chaos/src/main/docs/chaos.adoc b/components/camel-chaos/src/main/docs/chaos.adoc index 3a4f00f07c39..f833886500ab 100644 --- a/components/camel-chaos/src/main/docs/chaos.adoc +++ b/components/camel-chaos/src/main/docs/chaos.adoc @@ -165,6 +165,13 @@ A rule that expects failed exchanges in a route whose error handler is a dead le (`expect=parked:failed<=0`) has a warning too: the dead letter channel handles what fails there, so nothing fails, and the expectation is always met. `handled` counts what the dead letter channel handled (`expect=parked:handled<=0`). +So does a rule that expects no message lost between two routes (`expect=refunds->payment:lost<=0`) when the first +route does not hand messages off to the second (over `seda`, a broker or a `wireTap`): when it calls the second over +`direct` and waits, a failure there fails the first route and nothing is lost between them; when it does not send to it +at all (an HTTP call to a `platform-http` route), the failures of the first route are not in the loss. `failed` counts +them (`expect=refunds:failed<=0`). A route with a send that is known only when it sends (a `toD`, `wireTap` or +`recipientList`) is not checked. + To see what a rule would hit before it is added, `chaos-add --dry-run` checks it as `chaos-add` would, and adds nothing. It prints the rule in plain words, then each send, poll, route or step it matches now, with its route (and the step that sends), the sends known only when they send that may match (a `toD`), and the warnings: @@ -400,7 +407,9 @@ report does. * `INCONCLUSIVE`: it ended before an expectation was decided, so the messages may still have come; none of its faults fired, so its expectations say nothing about them; or an expectation was met on a route that had no messages while it ran (`refunds:failed<=0` when the traffic went to another route), or is a time (`maxTime`, `meanTime`) of -such a route. The `reason` says which. +such a route; or a route of an expectation failed exchanges that no expectation of it counts +(`refunds->payment:lost<=0` while refunds failed: the loss counts what completed). The `reason` says which, and names +the expectation that counts them. The routes that failed while it ran, and whose failures no expectation of the experiment counts, are listed as `alsoFailed`, with how many failed. An experiment that expects `refunds:maxTime<=30s` passes while 2 refunds failed, @@ -1011,6 +1020,8 @@ delay rule slows the route as it should). difference is the messages lost (never below 0, as the second route may also get messages from elsewhere). Messages still on their way when the time is up count as lost too: let `within=` run past the `until=` of the faults, so the last ones can arrive. The messages it counts as lost are added to the lost of the report. The op is `<=` or `=`. +* A failed exchange of the first route is not counted either: it did not complete, so the loss does not see it. An +experiment with such failures in the route of its expectation is `INCONCLUSIVE`, not `PASS`. * An exchange of the first route that its error handler handled, such as an order the dead letter channel parked, is not counted in the first route: it is not meant to reach the second one, so it is not lost. In the second route a handled exchange counts as completed, as it arrived. diff --git a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java index 1dedff27ba84..de1f9690d3d0 100644 --- a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java +++ b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java @@ -16,8 +16,11 @@ */ package org.apache.camel.component.chaos; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Comparator; +import java.util.Deque; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -518,6 +521,76 @@ final class ChaosAdvisor { return query > 0 ? uri.substring(0, query) : uri; } + /** + * Why an expectation of the messages lost between two routes ({@code orders->shipping:lost<=0}) says nothing, or + * null when it does (CAMEL-26797). Lost counts what the first route completed and the next did not, which tells + * only when the first hands the message off to the next (over seda, a broker or a wireTap) and goes on: a route + * that calls the next over direct waits for it and fails with it, and a route that does not send to it at all (such + * as an HTTP call to a platform-http route) loses nothing that lost counts, while its own failures are not counted. + * The routes reached through a send that is known only when it sends (toD, wireTap, recipientList...) are not + * known, so then it says nothing. + */ + static String lostWithoutHandOff(CamelContext camelContext, ChaosRule.Expectation expectation) { + if (expectation == null || expectation.toRoute == null || camelContext.getRoute(expectation.route) == null + || camelContext.getRoute(expectation.toRoute) == null) { + return null; + } + Map<String, String> consumers = new HashMap<>(); + for (Route route : camelContext.getRoutes()) { + if (route.getEndpoint() != null) { + consumers.put(uri(route.getEndpoint()), route.getRouteId()); + } + } + // the routes the first one reaches, and whether a hand-off was on the way there (one that was wins) + Map<String, Boolean> reached = new HashMap<>(); + reached.put(expectation.route, false); + Deque<String> todo = new ArrayDeque<>(List.of(expectation.route)); + while (!todo.isEmpty()) { + String id = todo.poll(); + Route route = camelContext.getRoute(id); + if (route == null) { + continue; + } + if (route.getRoute() instanceof RouteDefinition definition) { + List<String> dynamic = new ArrayList<>(); + ChaosTargets.dynamicSends(definition.getOutputs(), dynamic); + if (!dynamic.isEmpty()) { + return null; + } + } + for (Processor processor : route.filter("*")) { + if (!(processor instanceof EndpointAware aware) || aware.getEndpoint() == null) { + continue; + } + String uri = uri(aware.getEndpoint()); + String next = consumers.get(uri); + if (next == null) { + continue; + } + boolean handedOff = reached.get(id) || !"direct".equals(ChaosPatterns.scheme(uri)); + Boolean before = reached.get(next); + if (before == null || !before && handedOff) { + reached.put(next, handedOff); + todo.add(next); + } + } + } + Boolean handedOff = reached.get(expectation.toRoute); + if (handedOff == null) { + return "expects messages lost between routes " + expectation.route + " and " + expectation.toRoute + + ", but " + expectation.route + " does not hand messages to " + expectation.toRoute + + " (over seda, a broker or a wireTap), so lost counts nothing of what fails in " + expectation.route + + ": expect=" + expectation.route + ":failed<=0 counts it"; + } + if (!handedOff) { + return "expects messages lost between routes " + expectation.route + " and " + expectation.toRoute + + ", but " + expectation.route + " calls " + expectation.toRoute + + " over direct and waits for it, so a failure there fails " + expectation.route + + " and nothing is lost between them: expect=" + expectation.route + ":failed<=0 counts it"; + } + return null; + } + /** The dead letter channel of the route, without its options, or null when its error handler has none. */ static String deadLetterUri(Route route) { if (route.getErrorHandlerFactory() instanceof DeadLetterChannelDefinition dlc && dlc.getDeadLetterUri() != null) { diff --git a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosExperiment.java b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosExperiment.java index 94ef7db1fc9c..36207843ee80 100644 --- a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosExperiment.java +++ b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosExperiment.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.StringJoiner; +import java.util.TreeMap; import java.util.TreeSet; import java.util.function.Function; @@ -63,6 +64,22 @@ final class ChaosExperiment { + " nothing: feed " + (routes.size() == 1 ? "it" : "them"); } + /** + * Why an experiment is INCONCLUSIVE when the route of an expectation failed exchanges that no expectation counts + * (CAMEL-26797), such as refunds->payment:lost<=0 while refunds failed: lost counts what one route completed and + * the next did not, so a failure in the first route is not in it. + */ + static String uncountedFailures(Map<String, Long> routes) { + StringJoiner failed = new StringJoiner(", "); + StringJoiner expect = new StringJoiner(" "); + routes.forEach((route, n) -> { + failed.add(routes.size() == 1 ? route + " failed " + n + (n == 1 ? " exchange" : " exchanges") : route + " " + n); + expect.add("expect=" + route + ":failed<=0"); + }); + return (routes.size() == 1 ? "route " + failed : "routes " + failed + " failed exchanges") + + " that its expectations do not count, so they say nothing about the failures: " + expect + " counts them"; + } + /** How an experiment ended. */ enum End { // endExperiment @@ -163,6 +180,10 @@ final class ChaosExperiment { long firedAll = 0; // the routes whose failures an expectation counts, and the routes of the expectations that are met Set<String> failedExpected = new HashSet<>(); + // the routes of the expectations, and the routes whose failures an expectation counts in some way: failed, + // completed (a failure is not completed), or the next route of lost (what it did not complete) + Set<String> expectRoutes = new HashSet<>(); + Set<String> failuresCounted = new HashSet<>(); Set<String> metRoutes = new TreeSet<>(); Set<String> unmetTimes = new HashSet<>(); for (String id : rules) { @@ -187,6 +208,12 @@ final class ChaosExperiment { if (e.kind == Expectation.Kind.FAILED) { failedExpected.add(e.route); } + expectRoutes.add(e.route); + if (e.kind == Expectation.Kind.FAILED || e.kind == Expectation.Kind.COMPLETED) { + failuresCounted.add(e.route); + } else if (e.kind == Expectation.Kind.LOST) { + failuresCounted.add(e.toRoute); + } Expectation.Reading reading = e.read(now); // an expectation the experiment did not wait for is undecided, not cancelled Outcome outcome = reading.outcome() == Outcome.CANCELLED ? Outcome.PENDING : reading.outcome(); @@ -219,6 +246,7 @@ final class ChaosExperiment { JsonArray routes = new JsonArray(); JsonObject alsoFailed = new JsonObject(); Set<String> active = new HashSet<>(); + Map<String, Long> uncounted = new TreeMap<>(); counts.forEach((route, c) -> { long[] now0 = read(c); long[] then = before.getOrDefault(route, new long[5]); @@ -241,6 +269,9 @@ final class ChaosExperiment { if (delta[1] > 0 && !failedExpected.contains(route)) { alsoFailed.put(route, delta[1]); } + if (delta[1] > 0 && expectRoutes.contains(route) && !failuresCounted.contains(route)) { + uncounted.put(route, delta[1]); + } } }); jo.put("routes", routes); @@ -263,12 +294,16 @@ final class ChaosExperiment { } metRoutes.removeAll(active); boolean idle = !metRoutes.isEmpty(); + // an expectation met while its route failed exchanges it does not count says nothing either (CAMEL-26797) + boolean hidden = !uncounted.isEmpty(); String verdict = unmet || lost > 0 - ? FAIL : !undecided && !neverFired && !idle ? PASS : ended ? INCONCLUSIVE : PENDING; + ? FAIL : !undecided && !neverFired && !idle && !hidden ? PASS : ended ? INCONCLUSIVE : PENDING; jo.put("verdict", verdict); if (INCONCLUSIVE.equals(verdict)) { - // never fired comes first: when no fault ran, the expectations that did not decide are a consequence of it - jo.put("reason", neverFired ? NEVER_FIRED : idle ? noMessages(metRoutes) : NOT_DECIDED); + // never fired comes first: when no fault ran, the expectations that did not decide are a consequence of it; + // failures not counted come before not decided, as waiting longer would not count them either + jo.put("reason", neverFired + ? NEVER_FIRED : idle ? noMessages(metRoutes) : hidden ? uncountedFailures(uncounted) : NOT_DECIDED); } if (countsReset) { jo.put("countsReset", true); diff --git a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java index f711af4f1304..db42b6c96976 100644 --- a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java +++ b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java @@ -452,6 +452,10 @@ public final class ChaosManager { if (target != null) { answer.add(target); } + String handOff = ChaosAdvisor.lostWithoutHandOff(camelContext, rule.expectation); + if (handOff != null) { + answer.add(handOff); + } String dead = deadLetterOfFailedExpectation(rule); if (dead != null) { String route = rule.expectation.route; diff --git a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java index 8d9bc3631f12..51c5d18014c8 100644 --- a/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java +++ b/components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java @@ -391,7 +391,7 @@ final class ChaosTargets { return new Reach(uris, dynamic, polled, byRoute); } - private static void dynamicSends(List<ProcessorDefinition<?>> outputs, List<String> uris) { + static void dynamicSends(List<ProcessorDefinition<?>> outputs, List<String> uris) { for (ToDynamicDefinition to : ProcessorDefinitionHelper.filterTypeInOutputs(outputs, ToDynamicDefinition.class)) { String uri = to.getUri(); EndpointProducerBuilder builder = to.getEndpointProducerBuilder(); diff --git a/components/camel-chaos/src/test/java/org/apache/camel/component/chaos/ChaosExperimentTest.java b/components/camel-chaos/src/test/java/org/apache/camel/component/chaos/ChaosExperimentTest.java index afbf20d548cb..8cfe9cbe40d6 100644 --- a/components/camel-chaos/src/test/java/org/apache/camel/component/chaos/ChaosExperimentTest.java +++ b/components/camel-chaos/src/test/java/org/apache/camel/component/chaos/ChaosExperimentTest.java @@ -57,6 +57,13 @@ class ChaosExperimentTest extends ChaosTestSupport { .errorHandler(deadLetterChannel("direct:parked")) .to("mock:charged"); from("direct:parked").routeId("parked").to("mock:parked"); + // refunds calls the payment service (as over HTTP), checkout calls the payment route and waits, and + // the shop hands the orders to shipping over seda (CAMEL-26797) + from("direct:refunds").routeId("refunds").to("mock:pay"); + from("direct:payment").routeId("payment").to("mock:paid"); + from("direct:checkout").routeId("checkout").to("direct:payment"); + from("direct:shop").routeId("shop").to("seda:shipping"); + from("seda:shipping").routeId("shipping").to("mock:sent"); } }; } @@ -249,6 +256,52 @@ class ChaosExperimentTest extends ChaosTestSupport { } } + @Test + void anExpectationWhoseRouteFailedUncountedDoesNotPass() { + chaos().startExploration("10m", null); + // lost counts what refunds completed and payment did not: the refunds that failed are not in it (CAMEL-26797) + chaos().beginExperiment("refunds handle a failed payment", + "endpoint=mock:pay error rate=every:2 expect=refunds->payment:lost<=0 within=1m"); + for (int i = 0; i < 4; i++) { + template.send("direct:refunds", e -> e.getMessage().setBody("refund")); + // the payment route has other callers (the orders): it completes more than refunds, so 0 lost + template.send("direct:payment", e -> e.getMessage().setBody("order")); + } + clock.advance(61_000); + + Map<String, Object> ended = chaos().endExperiment(null); + + assertThat(ended).containsEntry("verdict", "INCONCLUSIVE").containsEntry("reason", + "route refunds failed 2 exchanges that its expectations do not count, so they say nothing about the" + + " failures: expect=refunds:failed<=0 counts them"); + assertThat(first(new JsonObject(ended), "expectations").getString("state")).isEqualTo("met"); + assertThat(((JsonObject) ended.get("alsoFailed")).getLong("refunds")).isEqualTo(2); + + // an expectation that counts them decides it + chaos().beginExperiment(null, "endpoint=mock:pay error rate=every:2 expect=refunds:failed<=0 within=1m"); + template.send("direct:refunds", e -> e.getMessage().setBody("refund")); + template.send("direct:refunds", e -> e.getMessage().setBody("refund")); + assertThat(chaos().endExperiment(null)).containsEntry("verdict", "FAIL"); + } + + @Test + void aLostExpectationBetweenRoutesThatDoNotHandOffIsWarned() { + assertThat(warnings("endpoint=mock:pay error expect=refunds->payment:lost<=0")).singleElement().asString() + .contains("expects messages lost between routes refunds and payment, but refunds does not hand messages" + + " to payment (over seda, a broker or a wireTap)", + "expect=refunds:failed<=0 counts it"); + assertThat(warnings("endpoint=mock:paid error expect=checkout->payment:lost<=0")).singleElement().asString() + .contains("checkout calls payment over direct and waits for it", + "expect=checkout:failed<=0 counts it"); + // a hand-off over seda: no warning + assertThat(warnings("route=shipping error expect=shop->shipping:lost<=0")).isEmpty(); + } + + private List<Object> warnings(String rule) { + JsonObject checked = (JsonObject) list(new JsonObject(chaos().checkRules(rule)), "rules").get(0); + return checked.containsKey("warnings") ? list(checked, "warnings") : List.of(); + } + @Test void oneAtATimeWithAnExpectationAndInAnExploration() { String rule = "endpoint=mock:shipped error expect=orders:failed<=0 within=1m";
