This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch CAMEL-26790-experiment-checks in repository https://gitbox.apache.org/repos/asf/camel.git
commit b69c0858c8f174630ee6114c4bd3919233267016 Author: Claus Ibsen <[email protected]> AuthorDate: Mon Nov 9 22:34:40 2026 +0100 CAMEL-26790: camel-chaos - an experiment that cannot tell does not pass - an experiment whose faults never fired is INCONCLUSIVE, not PASS, with the reason (and an INCONCLUSIVE one that ended undecided says so too) - the routes that failed outside the expectations of an experiment are listed as alsoFailed - a rule expecting failed exchanges in a route whose error handler is a dead letter channel has a warning: nothing fails there, handled counts it Found in the recorded runs of camel cmd chaos-explore (CAMEL-26760). Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- components/camel-chaos/src/main/docs/chaos.adoc | 20 +++++-- .../apache/camel/component/chaos/ChaosAdvisor.java | 2 +- .../camel/component/chaos/ChaosExperiment.java | 46 ++++++++++++++-- .../apache/camel/component/chaos/ChaosManager.java | 19 +++++++ .../camel/component/chaos/ChaosExperimentTest.java | 63 ++++++++++++++++++++++ .../dsl/jbang/core/commands/process/ListChaos.java | 10 ++++ .../jbang/core/commands/process/ListChaosTest.java | 19 ++++++- 7 files changed, 168 insertions(+), 11 deletions(-) diff --git a/components/camel-chaos/src/main/docs/chaos.adoc b/components/camel-chaos/src/main/docs/chaos.adoc index 8b7289afec57..efa8b1d3495b 100644 --- a/components/camel-chaos/src/main/docs/chaos.adoc +++ b/components/camel-chaos/src/main/docs/chaos.adoc @@ -161,6 +161,10 @@ names some of the endpoints, routes or steps there are. It is not an error, as t creates its endpoints when it sends, and a route can be added later). The rules of `camel.chaos.rules` are checked when the context has started. +A rule that expects failed exchanges in a route whose error handler is a dead letter channel +(`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`). + 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: @@ -391,9 +395,14 @@ faults of each rule, the state of each expectation, and a verdict: The dead letter channel then handles the message without delivering it, so no expectation of a route sees it: the experiment counts it as lost (`lostAtDeadLetterChannel`, and per rule `lostAtDeadLetterChannelPerRule`), as the report does. -* `PASS`: all expectations are met. -* `PENDING`: it is still open and an expectation is not decided yet. -* `INCONCLUSIVE`: it ended before an expectation was decided, so the messages may still have come. +* `PASS`: all expectations are met, and a fault fired. +* `PENDING`: it is still open, and an expectation is not decided yet or no fault has fired yet. +* `INCONCLUSIVE`: it ended before an expectation was decided, so the messages may still have come, or none of its +faults fired, so its expectations say nothing about them. The `reason` says which. + +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, +and `alsoFailed` says so. The experiment is ended with the exploration when that finishes, or when its time is up, with what came of it so far. @@ -406,8 +415,9 @@ The journal has them as `experiments`, each with: * `number`, `hypothesis`, the `rules` (their ids) and `ruleTexts` * `open`, `startedAt` and `elapsed` -* the `verdict` and the `expectations`: per rule its `expect`, `state` (`met`, `unmet` or `pending`), the `actual` value, -and the `value` in words +* the `verdict` (and the `reason` of an `INCONCLUSIVE` one) and the `expectations`: per rule its `expect`, `state` +(`met`, `unmet` or `pending`), the `actual` value, and the `value` in words +* `alsoFailed`: the routes that failed outside the expectations, when there are such * the `faults` per rule * the `routes` that did something while it ran, with the same counts as the baseline * once ended, `endedAt`, `endedBy` (`ended`, `finished` or `deadline`) and the `note` 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 5c88f23aca89..1dedff27ba84 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 @@ -519,7 +519,7 @@ final class ChaosAdvisor { } /** The dead letter channel of the route, without its options, or null when its error handler has none. */ - private static String deadLetterUri(Route route) { + static String deadLetterUri(Route route) { if (route.getErrorHandlerFactory() instanceof DeadLetterChannelDefinition dlc && dlc.getDeadLetterUri() != null) { String uri = dlc.getDeadLetterUri(); return uri.indexOf('?') > 0 ? uri.substring(0, uri.indexOf('?')) : uri; 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 33f6d676dc53..a1ab5243a4ba 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 @@ -16,9 +16,11 @@ */ package org.apache.camel.component.chaos; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.StringJoiner; import java.util.function.Function; @@ -36,8 +38,11 @@ import org.apache.camel.util.json.JsonObject; * The verdict is {@code FAIL} when an expectation is unmet, or a rule of it failed or dropped the send of a dead letter * channel, which then handled the message without delivering it (lost, as the report counts it, CAMEL-26789), and * {@code PASS} when all are met. While it runs and an expectation is not decided it is {@code PENDING}, and an - * experiment that ended before an expectation was decided is {@code INCONCLUSIVE}: the messages may still have come. - * The manager guards it with its lock. + * experiment that ended before an expectation was decided is {@code INCONCLUSIVE}: the messages may still have come. An + * experiment whose faults never fired is not a {@code PASS} either, as its expectations say nothing about the faults + * ({@code INCONCLUSIVE} once it ended, CAMEL-26790). The routes that failed while it ran, and that no expectation of it + * counts the failures of, are listed ({@code alsoFailed}), so a verdict on another count does not hide them. The + * manager guards it with its lock. */ final class ChaosExperiment { @@ -46,6 +51,10 @@ final class ChaosExperiment { static final String PENDING = "PENDING"; static final String INCONCLUSIVE = "INCONCLUSIVE"; + // why an experiment is INCONCLUSIVE + static final String NEVER_FIRED = "its faults never fired, so its expectations say nothing about them"; + static final String NOT_DECIDED = "it ended before an expectation was decided, so the messages may still have come"; + /** How an experiment ended. */ enum End { // endExperiment @@ -143,6 +152,9 @@ final class ChaosExperiment { JsonArray texts = new JsonArray(); JsonObject faults = new JsonObject(); JsonArray expectations = new JsonArray(); + long firedAll = 0; + // the routes whose failures an expectation counts + Set<String> failedExpected = new HashSet<>(); for (String id : rules) { ChaosRule r = rule.apply(id); if (r == null) { @@ -154,6 +166,7 @@ final class ChaosExperiment { fired += r.firedAt(hook); } faults.put(id, fired); + firedAll += fired; long kept = r.lostAtDlqCount(); if (kept > 0) { lostAtDlq.put(id, kept); @@ -161,6 +174,9 @@ final class ChaosExperiment { } Expectation e = r.expectation; if (e != null) { + if (e.kind == Expectation.Kind.FAILED) { + failedExpected.add(e.route); + } 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(); @@ -182,8 +198,16 @@ final class ChaosExperiment { jo.put("lostAtDeadLetterChannel", lost); jo.put("lostAtDeadLetterChannelPerRule", lostAtDlq); } - jo.put("verdict", unmet || lost > 0 ? FAIL : !undecided ? PASS : ended ? INCONCLUSIVE : PENDING); + // nothing fired: the expectations were not put to the test (an unmet one is still a FAIL, with its evidence) + boolean neverFired = firedAll == 0; + String verdict = unmet || lost > 0 + ? FAIL : !undecided && !neverFired ? PASS : ended ? INCONCLUSIVE : PENDING; + jo.put("verdict", verdict); + if (INCONCLUSIVE.equals(verdict)) { + jo.put("reason", neverFired ? NEVER_FIRED : NOT_DECIDED); + } JsonArray routes = new JsonArray(); + JsonObject alsoFailed = new JsonObject(); counts.forEach((route, c) -> { long[] now0 = read(c); long[] then = before.getOrDefault(route, new long[5]); @@ -202,9 +226,15 @@ final class ChaosExperiment { r.put("handled", delta[3]); r.put("redeliveries", delta[4]); routes.add(r); + if (delta[1] > 0 && !failedExpected.contains(route)) { + alsoFailed.put(route, delta[1]); + } } }); jo.put("routes", routes); + if (!alsoFailed.isEmpty()) { + jo.put("alsoFailed", alsoFailed); + } if (countsReset) { jo.put("countsReset", true); } @@ -239,11 +269,21 @@ final class ChaosExperiment { sb.append("\n expect ").append(x.getString("expect")).append(": ").append(x.getString("state")) .append(", ").append(x.getString("value")); } + String reason = jo.getString("reason"); + if (reason != null) { + sb.append("\n inconclusive: ").append(reason); + } long lost = jo.getLongOrDefault("lostAtDeadLetterChannel", 0); if (lost > 0) { sb.append("\n lost at the dead letter channel: ").append(lost) .append(lost == 1 ? " message" : " messages").append(", handled there without being delivered"); } + JsonObject alsoFailed = (JsonObject) jo.get("alsoFailed"); + if (alsoFailed != null) { + StringJoiner also = new StringJoiner(", "); + alsoFailed.forEach((route, n) -> also.add(route + " " + n)); + sb.append("\n also failed, in no expectation: ").append(also); + } StringJoiner sj = new StringJoiner(", "); for (Object o : jo.getCollection("routes")) { JsonObject r = (JsonObject) o; 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 404e77c66d24..f711af4f1304 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,9 +452,28 @@ public final class ChaosManager { if (target != null) { answer.add(target); } + String dead = deadLetterOfFailedExpectation(rule); + if (dead != null) { + String route = rule.expectation.route; + answer.add("expects failed exchanges in route " + route + ", but its error handler is the dead letter channel " + + dead + ", which handles what fails there, so nothing fails: expect=" + route + + ":handled<=0 counts what the dead letter channel handled"); + } return answer; } + /** + * The dead letter channel of the route of an expectation of failed exchanges ({@code expect=parked:failed<=0}), as + * such an expectation is always met there (CAMEL-26790), or null. + */ + private String deadLetterOfFailedExpectation(ChaosRule rule) { + if (rule.expectation == null || rule.expectation.kind != ChaosRule.Expectation.Kind.FAILED) { + return null; + } + Route route = camelContext.getRoute(rule.expectation.route); + return route != null ? ChaosAdvisor.deadLetterUri(route) : null; + } + /** * A rule in plain words, such as {@code fail half of the sends to http:stock* with HTTP status 503, for 5 minutes}, * at the hooks where it can fire in the context now (see the {@code hooks} of the report). 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 aa4ff153c363..d337afc3ba0c 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 @@ -161,6 +161,69 @@ class ChaosExperimentTest extends ChaosTestSupport { "lost at the dead letter channel: 3 messages, handled there without being delivered"); } + @Test + void anExperimentWhoseFaultsNeverFiredDoesNotPass() { + chaos().startExploration("10m", null); + // the fault is on a step the orders do not reach (CAMEL-26790): its expectation is met, but says nothing + chaos().beginExperiment("orders keep going when parking fails", + "endpoint=mock:parked error expect=orders:2 within=1m"); + send(2); + assertThat(experiment(1).getString("verdict")).isEqualTo("PENDING"); + + Map<String, Object> ended = chaos().endExperiment(null); + + assertThat(ended).containsEntry("verdict", "INCONCLUSIVE").containsEntry("reason", + "its faults never fired, so its expectations say nothing about them"); + assertThat(first(new JsonObject(ended), "expectations").getString("state")).isEqualTo("met"); + assertThat(chaos().report().toText()).contains("1 INCONCLUSIVE orders keep going when parking fails", + "inconclusive: its faults never fired"); + + // ended before its expectation was decided: the other reason + chaos().beginExperiment(null, "endpoint=mock:shipped delay duration=10ms expect=orders:5 within=1m"); + send(1); + assertThat(chaos().endExperiment(null)).containsEntry("verdict", "INCONCLUSIVE").containsEntry("reason", + "it ended before an expectation was decided, so the messages may still have come"); + } + + @Test + void theRoutesThatFailedOutsideTheExpectationsAreListed() { + chaos().startExploration("10m", null); + // the expectation counts the completed orders, not the failed ones (CAMEL-26790) + chaos().beginExperiment("orders keep coming", "endpoint=mock:shipped error rate=every:2 expect=orders:2 within=1m"); + send(4); + + Map<String, Object> ended = chaos().endExperiment(null); + + assertThat(ended).containsEntry("verdict", "PASS"); + assertThat(((JsonObject) ended.get("alsoFailed")).getLong("orders")).isEqualTo(2); + assertThat(chaos().report().toText()).contains("also failed, in no expectation: orders 2"); + + // an expectation of the failures counts them: not listed again + chaos().beginExperiment(null, "endpoint=mock:shipped error rate=every:2 expect=orders:failed<=5 within=1m"); + send(2); + clock.advance(61_000); + assertThat(chaos().endExperiment(null)).containsEntry("verdict", "PASS").doesNotContainKey("alsoFailed"); + } + + @Test + void anExpectationOfFailuresInARouteWithADeadLetterChannelIsWarned() { + // billing fails to its dead letter channel, which handles the message: failed stays 0 there (CAMEL-26790) + JsonObject checked = (JsonObject) list( + new JsonObject(chaos().checkRules("endpoint=mock:charged error expect=billing:failed<=0")), "rules") + .get(0); + assertThat(list(checked, "warnings")).singleElement().asString().contains( + "expects failed exchanges in route billing, but its error handler is the dead letter channel" + + " direct:parked", + "expect=billing:handled<=0 counts what the dead letter channel handled"); + + // what the dead letter channel handled, or a route without one: no warning + for (String rule : List.of("endpoint=mock:charged error expect=billing:handled<=0", + "endpoint=mock:shipped error expect=orders:failed<=0")) { + JsonObject ok = (JsonObject) list(new JsonObject(chaos().checkRules(rule)), "rules").get(0); + assertThat(ok.containsKey("warnings")).as(rule).isFalse(); + } + } + @Test void oneAtATimeWithAnExpectationAndInAnExploration() { String rule = "endpoint=mock:shipped error expect=orders:failed<=0 within=1m"; diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaos.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaos.java index 0905f5dc9cec..dd6026ea45ca 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaos.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaos.java @@ -377,11 +377,21 @@ public class ListChaos extends ProcessWatchCommand { + x.getString("value")); } } + String reason = experiment.getString("reason"); + if (reason != null) { + printer().println(" inconclusive: " + reason); + } long lost = experiment.getLongOrDefault("lostAtDeadLetterChannel", 0); if (lost > 0) { printer().println(" lost at the dead letter channel: " + lost + (lost == 1 ? " message" : " messages") + ", handled there without being delivered"); } + JsonObject alsoFailed = (JsonObject) experiment.get("alsoFailed"); + if (alsoFailed != null && !alsoFailed.isEmpty()) { + StringJoiner also = new StringJoiner(", "); + alsoFailed.forEach((route, n) -> also.add(route + " " + n)); + printer().println(" also failed, in no expectation: " + also); + } List<Object> routes = experiment.getCollection("routes"); if (routes != null && !routes.isEmpty()) { printer().println(" routes: " + routeCounts(routes)); diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaosTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaosTest.java index 6c06f3e46c9c..f3e9a502f341 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaosTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/process/ListChaosTest.java @@ -221,7 +221,20 @@ class ListChaosTest extends ProcessCommandTestSupport { experiment.put("routes", new JsonArray(List.of(during))); experiment.put("note", "orders has no error handler"); experiment.put("lostAtDeadLetterChannel", 58L); - exploration.put("experiments", new JsonArray(List.of(experiment))); + JsonObject alsoFailed = new JsonObject(); + alsoFailed.put("refunds", 2L); + experiment.put("alsoFailed", alsoFailed); + // its fault never fired + JsonObject second = new JsonObject(); + second.put("number", 2); + second.put("open", false); + second.put("endedBy", "ended"); + second.put("elapsed", 41_000L); + second.put("rules", new JsonArray(List.of("r3"))); + second.put("verdict", "INCONCLUSIVE"); + second.put("reason", "its faults never fired, so its expectations say nothing about them"); + second.put("expectations", new JsonArray()); + exploration.put("experiments", new JsonArray(List.of(experiment, second))); ((JsonObject) chaos.get("report")).put("exploration", exploration); writeStatus(chaos); @@ -233,8 +246,10 @@ class ListChaosTest extends ProcessCommandTestSupport { "experiments:", "1 FAIL an order never fails (r1, r2, ended after 1m4s)", "expect orders:failed<=0: unmet, 9 failed", "lost at the dead letter channel: 58 messages, handled there without being delivered", + "also failed, in no expectation: refunds 2", "routes: orders 41 completed, 9 failed", - "note: orders has no error handler"); + "note: orders has no error handler", "2 INCONCLUSIVE", + "inconclusive: its faults never fired, so its expectations say nothing about them"); } @Test
