gnodet-bot commented on code in PR #29397: URL: https://github.com/apache/camel/pull/29397#discussion_r4505447818
########## components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java: ########## @@ -0,0 +1,240 @@ +/* + * 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.component.chaos; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.camel.CamelContext; +import org.apache.camel.Endpoint; +import org.apache.camel.EndpointAware; +import org.apache.camel.Processor; +import org.apache.camel.Route; +import org.apache.camel.model.CircuitBreakerDefinition; +import org.apache.camel.model.ProcessorDefinitionHelper; +import org.apache.camel.model.RouteDefinition; +import org.apache.camel.model.errorhandler.DeadLetterChannelDefinition; +import org.apache.camel.support.ScheduledPollConsumer; + +/** + * What to try (CAMEL-26727): chaos rules suggested per route from what the route does, ranked, each a known rule of the + * chaos model ({@link ChaosModel#recipeList()}) with its target filled in and a line that says why. The facts come from + * the running routes: their sends, their circuit breakers, their dead letter channel, their input, and the hand-offs + * between them. + */ +final class ChaosAdvisor { + + /** The most suggestions in all, so a big integration does not flood a tool. */ + static final int MAX = 40; + + // the inputs that can deliver a message again (at least once), so a call out may be made twice + private static final Set<String> AT_LEAST_ONCE = Set.of( + "kafka", "jms", "activemq", "activemq6", "amqp", "sjms", "sjms2", "aws2-sqs", "google-pubsub", + "spring-rabbitmq", "rabbitmq", "nats", "paho", "paho-mqtt5", "azure-servicebus", "ibm-mq"); + // a hand-off to another route of the same application, which does not wait for it + private static final Set<String> HAND_OFF = Set.of("seda", "vm", "disruptor", "disruptor-vm"); + + /** The kinds of faults, which the report also uses for what has been tried. */ + enum Kind { + ERRORS("errors"), + SLOW("slow"), + LOST("lost"), + CORRUPT("corrupt"), + DUPLICATE("duplicate"), + CONSUMER("consumer"), + LIFECYCLE("lifecycle"), + DEAD_LETTER("dead letter channel"); + + final String key; + + Kind(String key) { + this.key = key; + } + } + + /** + * A rule to try. + * + * @param route the route it is about + * @param kind the kind of fault + * @param recipe the id of the known rule it comes from (null for one of its own) + * @param title the known rule in plain words, such as "Returns an error (HTTP status)" + * @param rule the rule, ready to add + * @param why why it is worth trying for this route + * @param rank higher is tried first + */ + record Suggestion(String route, Kind kind, String recipe, String title, String rule, String why, int rank) { + } + + private ChaosAdvisor() { + } + + /** The rules to try for the routes of the context, the most worth trying first, at most {@link #MAX}. */ + static List<Suggestion> suggest(CamelContext camelContext) { + Map<String, ChaosModel.Recipe> recipes = new LinkedHashMap<>(); + for (ChaosModel.Recipe recipe : ChaosModel.recipeList()) { + recipes.put(recipe.id(), recipe); + } + List<Route> routes = camelContext.getRoutes(); + // the routes that consume an endpoint, for the hand-offs + Map<String, String> consumers = new LinkedHashMap<>(); + for (Route route : routes) { + if (route.getEndpoint() != null) { + consumers.put(uri(route.getEndpoint()), route.getRouteId()); + } + } + List<Suggestion> answer = new ArrayList<>(); + for (Route route : routes) { + try { + suggest(route, recipes, consumers, answer); + } catch (RuntimeException e) { + // the route changed while it was read (such as a reload): nothing to say for it now + } Review Comment: ⚠️ **Silent exception swallowing (still open):** The catch at lines 109–111 is intentionally broad to guard against `ConcurrentModificationException` during route reloads, but it logs nothing. Any genuine bug — `NullPointerException`, `ClassCastException`, `IllegalStateException` — is silently discarded with no log output. At minimum, a `LOG.debug()` would surface unexpected exceptions without changing the guard behaviour: ```suggestion } catch (RuntimeException e) { // the route changed while it was read (such as a reload): nothing to say for it now LOG.debug("Skipping route suggestion due to exception", e); } ``` (Requires adding `private static final Logger LOG = LoggerFactory.getLogger(ChaosAdvisor.class);`.) ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ChaosSuggestList.java: ########## @@ -0,0 +1,171 @@ +/* + * 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.dsl.jbang.core.commands.tui; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; + +import dev.tamboui.layout.Constraint; +import dev.tamboui.layout.Layout; +import dev.tamboui.layout.Rect; +import dev.tamboui.style.Overflow; +import dev.tamboui.style.Style; +import dev.tamboui.terminal.Frame; +import dev.tamboui.text.Line; +import dev.tamboui.text.Span; +import dev.tamboui.text.Text; +import dev.tamboui.tui.event.KeyEvent; +import dev.tamboui.widgets.Clear; +import dev.tamboui.widgets.block.Block; +import dev.tamboui.widgets.block.BorderType; +import dev.tamboui.widgets.block.Borders; +import dev.tamboui.widgets.paragraph.Paragraph; +import org.apache.camel.util.json.JsonObject; + +/** + * What to try ({@code w} on the Chaos screen, CAMEL-26727): the rules the integration suggests per route from what its + * routes do, the most worth trying first, each with why. Enter types the selected rule in, to change or add. + */ +final class ChaosSuggestList { + + /** What a key did to the list. */ + enum Outcome { + NONE, + CLOSED, + PICKED + } + + /** A suggestion, as the chaos action of the integration gives it. */ + record Item(String route, String kind, String title, String rule, String why, boolean tried) { + } + + private final String name; + private final List<Item> items = new ArrayList<>(); + private int selected; + + ChaosSuggestList(String name, JsonObject reply) { + this.name = name; + if (reply != null && reply.getCollection("suggestions") != null) { + for (Object o : reply.getCollection("suggestions")) { + if (o instanceof JsonObject jo && jo.getString("rule") != null) { + items.add(new Item( + jo.getString("route"), jo.getString("kind"), jo.getString("title"), + jo.getString("rule"), jo.getString("why"), jo.getBooleanOrDefault("tried", false))); + } + } + } + } + + List<Item> items() { + return items; + } + + /** The selected suggestion, or null when there are none. */ + Item selected() { + return items.isEmpty() ? null : items.get(selected); + } + + Outcome handleKey(KeyEvent ke) { + if (ke.isCancel()) { + return Outcome.CLOSED; + } + if (ke.isUp() && selected > 0) { + selected--; + } else if (ke.isDown() && selected < items.size() - 1) { + selected++; + } else if (ke.isConfirm() && !items.isEmpty()) { + return Outcome.PICKED; + } + return Outcome.NONE; + } + + void render(Frame frame, Rect area) { + int width = Math.min(area.width() - 4, 150); + int height = Math.min(area.height() - 2, 30); + Rect popup = DialogHelper.centered(area, width, height); + frame.renderWidget(Clear.INSTANCE, popup); + Block outer = Block.builder().borderType(BorderType.ROUNDED).borders(Borders.ALL).borderStyle(Theme.warning()) + .title(" What to try in " + name + " ").build(); + frame.renderWidget(outer, popup); + Rect inner = outer.inner(popup); + if (items.isEmpty()) { + frame.renderWidget(Paragraph.builder().overflow(Overflow.WRAP_WORD).text(Text.from(List.of(Line.empty(), + Line.from(Span.styled(" Nothing to suggest: the routes call nothing remote, and have no input from" + + " outside, dead letter channel or hand-off to another route", + Theme.muted()))))) + .build(), + inner); + return; + } + List<Rect> cols = Layout.horizontal() + .constraints(Constraint.length(Math.min(56, inner.width() / 2)), Constraint.fill()) + .split(inner); + renderList(frame, cols.get(0)); + renderDetail(frame, cols.get(1)); + } + + private void renderList(Frame frame, Rect area) { + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" The most worth trying first", Style.EMPTY.bold()))); + int selectedLine = 0; + for (int i = 0; i < items.size(); i++) { + Item item = items.get(i); + boolean sel = i == selected; + if (sel) { + selectedLine = lines.size(); + } + Style style = sel ? Theme.selectionBg().bold() : Style.EMPTY; + List<Span> spans = new ArrayList<>(); + spans.add(Span.styled((sel ? " > " : " ") + item.title(), style)); + spans.add(Span.styled(" " + item.route(), Theme.muted())); + if (item.tried()) { + spans.add(Span.styled(" tried", Theme.success())); + } Review Comment: ⚠️ **Dead code — `item.tried()` is always `false` (still open, `renderList`):** Since `ChaosManager.suggest()` never sets the `"tried"` key, `getBooleanOrDefault("tried", false)` always returns `false`. This block never executes. ########## components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java: ########## @@ -461,6 +461,33 @@ public boolean hasRule(String id) { } } + /** + * What to try (CAMEL-26727): rules suggested per route from what the routes do, the most worth trying first, each + * with the known rule it comes from and why, such as an HTTP call without a circuit breaker, a dead letter channel, + * or a hand-off to another route over seda. + * + * @return {@code suggestions}: a list of {@code route}, {@code kind}, {@code recipe}, {@code title}, {@code rule} + * and {@code why} + */ + public Map<String, Object> suggest() { + JsonArray list = new JsonArray(); + for (ChaosAdvisor.Suggestion s : ChaosAdvisor.suggest(camelContext)) { + JsonObject jo = new JsonObject(); + jo.put("route", s.route()); + jo.put("kind", s.kind().key); + if (s.recipe() != null) { + jo.put("recipe", s.recipe()); + } + jo.put("title", s.title()); + jo.put("rule", s.rule()); + jo.put("why", s.why()); Review Comment: ⚠️ **`"tried"` key still never set (still open):** `suggest()` populates each JSON suggestion with `route`, `kind`, `recipe`, `title`, `rule`, and `why` — but never `"tried"`. `ChaosSuggestList` reads `jo.getBooleanOrDefault("tried", false)`, which will always return `false`. The `if (item.tried())` blocks in `renderList` and `renderDetail` are therefore unreachable dead code. If `tried` is intended for a future feature (tracking which rules have been exercised), consider either omitting the rendering code for now, or documenting a follow-up issue for where it will be set. ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ChaosSuggestList.java: ########## @@ -0,0 +1,171 @@ +/* + * 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.dsl.jbang.core.commands.tui; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; + +import dev.tamboui.layout.Constraint; +import dev.tamboui.layout.Layout; +import dev.tamboui.layout.Rect; +import dev.tamboui.style.Overflow; +import dev.tamboui.style.Style; +import dev.tamboui.terminal.Frame; +import dev.tamboui.text.Line; +import dev.tamboui.text.Span; +import dev.tamboui.text.Text; +import dev.tamboui.tui.event.KeyEvent; +import dev.tamboui.widgets.Clear; +import dev.tamboui.widgets.block.Block; +import dev.tamboui.widgets.block.BorderType; +import dev.tamboui.widgets.block.Borders; +import dev.tamboui.widgets.paragraph.Paragraph; +import org.apache.camel.util.json.JsonObject; + +/** + * What to try ({@code w} on the Chaos screen, CAMEL-26727): the rules the integration suggests per route from what its + * routes do, the most worth trying first, each with why. Enter types the selected rule in, to change or add. + */ +final class ChaosSuggestList { + + /** What a key did to the list. */ + enum Outcome { + NONE, + CLOSED, + PICKED + } + + /** A suggestion, as the chaos action of the integration gives it. */ + record Item(String route, String kind, String title, String rule, String why, boolean tried) { + } + + private final String name; + private final List<Item> items = new ArrayList<>(); + private int selected; + + ChaosSuggestList(String name, JsonObject reply) { + this.name = name; + if (reply != null && reply.getCollection("suggestions") != null) { + for (Object o : reply.getCollection("suggestions")) { + if (o instanceof JsonObject jo && jo.getString("rule") != null) { + items.add(new Item( + jo.getString("route"), jo.getString("kind"), jo.getString("title"), + jo.getString("rule"), jo.getString("why"), jo.getBooleanOrDefault("tried", false))); + } + } + } + } + + List<Item> items() { + return items; + } + + /** The selected suggestion, or null when there are none. */ + Item selected() { + return items.isEmpty() ? null : items.get(selected); + } + + Outcome handleKey(KeyEvent ke) { + if (ke.isCancel()) { + return Outcome.CLOSED; + } + if (ke.isUp() && selected > 0) { + selected--; + } else if (ke.isDown() && selected < items.size() - 1) { + selected++; + } else if (ke.isConfirm() && !items.isEmpty()) { + return Outcome.PICKED; + } + return Outcome.NONE; + } + + void render(Frame frame, Rect area) { + int width = Math.min(area.width() - 4, 150); + int height = Math.min(area.height() - 2, 30); + Rect popup = DialogHelper.centered(area, width, height); + frame.renderWidget(Clear.INSTANCE, popup); + Block outer = Block.builder().borderType(BorderType.ROUNDED).borders(Borders.ALL).borderStyle(Theme.warning()) + .title(" What to try in " + name + " ").build(); + frame.renderWidget(outer, popup); + Rect inner = outer.inner(popup); + if (items.isEmpty()) { + frame.renderWidget(Paragraph.builder().overflow(Overflow.WRAP_WORD).text(Text.from(List.of(Line.empty(), + Line.from(Span.styled(" Nothing to suggest: the routes call nothing remote, and have no input from" + + " outside, dead letter channel or hand-off to another route", + Theme.muted()))))) + .build(), + inner); + return; + } + List<Rect> cols = Layout.horizontal() + .constraints(Constraint.length(Math.min(56, inner.width() / 2)), Constraint.fill()) + .split(inner); + renderList(frame, cols.get(0)); + renderDetail(frame, cols.get(1)); + } + + private void renderList(Frame frame, Rect area) { + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" The most worth trying first", Style.EMPTY.bold()))); + int selectedLine = 0; + for (int i = 0; i < items.size(); i++) { + Item item = items.get(i); + boolean sel = i == selected; + if (sel) { + selectedLine = lines.size(); + } + Style style = sel ? Theme.selectionBg().bold() : Style.EMPTY; + List<Span> spans = new ArrayList<>(); + spans.add(Span.styled((sel ? " > " : " ") + item.title(), style)); + spans.add(Span.styled(" " + item.route(), Theme.muted())); + if (item.tried()) { + spans.add(Span.styled(" tried", Theme.success())); + } + lines.add(Line.from(spans)); + } + // keep the selected one in sight + int scroll = Math.max(0, selectedLine - (area.height() - 2)); + frame.renderWidget(Paragraph.builder().text(Text.from(lines)).scroll(scroll) + .block(Block.builder().borders(EnumSet.of(Borders.RIGHT)).borderStyle(Theme.border()).build()).build(), area); + } + + private void renderDetail(Frame frame, Rect area) { + Item item = selected(); + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" " + item.title(), Style.EMPTY.bold()), + Span.styled(" route " + item.route() + " · " + item.kind(), Theme.muted()))); + lines.add(Line.empty()); + lines.add(Line.from(Span.raw(" " + item.why()))); + lines.add(Line.empty()); + lines.add(Line.from(Span.styled(" " + item.rule(), Theme.warning()))); + if (item.tried()) { + lines.add(Line.empty()); + lines.add(Line.from(Span.styled(" A rule of this kind was tried in route " + item.route() + " already", + Theme.success()))); + } Review Comment: ⚠️ **Dead code — `item.tried()` is always `false` (still open, `renderDetail`):** Same root cause as the companion comment in `renderList` — `item.tried()` is always `false`. The detail panel text about "a rule of this kind was tried" is unreachable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
