allthingssecurity opened a new pull request, #28646: URL: https://github.com/apache/camel/pull/28646
# Description [CAMEL-26338](https://issues.apache.org/jira/browse/CAMEL-26338), the second code step of [CAMEL-26330](https://issues.apache.org/jira/browse/CAMEL-26330) (camel-chaos), as in the design document `design/chaos.adoc` (#28539, "Hooks at all levels", "Target and hooks" and "Order of work"). The first step, the send hook ([CAMEL-26331](https://issues.apache.org/jira/browse/CAMEL-26331), #28598), is merged. This PR does not use anything of the send hook. It follows the same review points (a delay waits through `TimeSource` on the calling thread). **Where this differs from the JIRA.** CAMEL-26338 proposes to ask the hook in the channel, next to the check that skips a processor disabled at runtime, to wait there for a delay, and to raise a failure inside the error handler. This PR does not ask the hook next to the disabled check. It asks it in one place only, where the processor of the step is called: in the error handler of the step, or in the channel when the step has no redelivery error handler (table below). The reason: a redelivery does not go through the channel again, so the error handler has to ask anyway, and asking in two places would mean carrying the answer from one to the other. What changes against the JIRA text: a delay is waited for before each attempt of the step, so also before each redelivery, instead of once before the step. The design document on main (`design/chaos.adoc`, "Hooks at all levels") already says "Inside the error handler of the step"; only its "Order of work" list still says "the step hook i n the channel". A step disabled with `camel cmd disable-processor` is still skipped before the hook is asked. The JIRA also says that without a hook the routes do not change; that holds except in one case: with JMX and an error handler that can redeliver, each step gets one more small processor inside its JMX wrapper, also without a hook, so that the hook is asked inside what JMX counts (see **JMX** and **No hook** below; about 7 ns per step there). Every step of a route runs through its channel (`DefaultChannel`), which runs its advices and calls the error handler of the step, which calls the processor. This PR adds a hook that is asked right before the processor of a step is called: a new SPI, `org.apache.camel.spi.StepHook`, installed with `ExtendedCamelContext#setStepHook`. It gets the exchange and the node id of the step, and it can fail the step or delay it. Like the send hook it is general (it names nothing of chaos), so other developer tools and tests can use it. ```java public interface StepHook { long PROCEED = 0; long beforeStep(Exchange exchange, String nodeId); // PROCEED or a delay in millis; an exception on the exchange fails the step } ``` To fail the step the hook sets an exception on the exchange (or throws one): the processor is not called and the step fails with it. A positive value delays the call of the processor. Negative values are reserved: the step fails with an `IllegalArgumentException`, so a meaning can be given to them later without changing what a working hook does. **Where it is asked: one place per step, once per attempt.** The hook is asked only where the processor is called: | Step | Asked by | A failure from the hook | |---|---|---| | has a `RedeliveryErrorHandler` (the default error handler, the dead letter channel, the Spring and JTA transaction error handlers, which extend it) | the error handler, at the three places where it calls its output (`SimpleTask.handleFirst`, the simple delivery and the redelivery of `RedeliveryTask`) | a failure of the processor: `onException`, redelivery and the dead letter channel apply to it, and each redelivery asks the hook again | | has no such error handler: inside `doTry`/`doCatch`/`doFinally`, `onException` and `circuitBreaker`, a `multicast` and its outputs, a route with `noErrorHandler()` (its `NoErrorHandler` only calls the step) | the channel, where it calls the processor (after its advices, so tracing, the debuggers and JMX see the step as for the other steps) | the exception of the exchange, as when the processor fails: `doCatch` gets it inside `doTry`; an output of a `multicast` is handled and redelivered by the error handler that the multicast keeps for each output; otherwise the exchange fails with it, without `onException` | The message the dead letter channel or `onException` gets is the same as when the processor fails. Only what the processor itself would have set is not there, as it did not run: for a `to` step failed by the hook, `CamelFailureEndpoint` is the endpoint the exchange was sent to before (as for a `process` step that fails), not the endpoint of the `to`. (A custom error handler that is not a `RedeliveryErrorHandler` is in the second row: the channel asks, and a failure from the hook skips that error handler as it skips the processor. Camel has no such error handler besides `NoErrorHandler`.) The channel learns which case it is in when it is initialized: `DefaultChannel.postInitChannel` calls a new default method, `ErrorHandlerRedeliveryCustomizer#useStepHook(nodeId)` (the internal SPI the channel already uses to give the error handler its JMX instrumentation), on its error handler. `RedeliveryErrorHandler` keeps the node id and returns `true`; any other error handler returns `false` (the default), and then the channel asks the hook itself. Exactly one place asks for a step (with JMX there is one variant, below). The error handlers that the recipient list, the routing slip, the dynamic router and the multicast create for their outputs are clones or new instances without a node id, so they do not ask (the step is the EIP, which its own channel or error handler asked for). A step disabled with `camel cmd disable-processor` is skipped before the hook is asked, as before. **JMX.** The statistics of a step (`camel get processor`, the processor MBeans) count a failure or delay from the hook as for its processor: a failed attempt, its redelivery and the time of the delay. When the error handler of a step cannot redeliver, the JMX instrumentation is an advice of the channel around the error handler, so it sees the hook anyway. When the error handler can redeliver, JMX wraps the output of the error handler to count each attempt; then the channel does not hand the node id to the error handler (it calls `useStepHook(null)`) but puts a small processor that asks the hook inside that JMX wrapper (`DefaultChannel.StepHookProcessor`), so each attempt is still asked once, inside what JMX counts. This is the only case with one more processor per step (next to the JMX wrapper that is there already); without JMX, or with JMX and an error handler that cannot redeliver, there is none. **Delays.** A positive value makes the thread that runs the step wait, and the processor is then called on that thread, as for a slow step, which holds its thread. (The `delay` EIP differs: by default it schedules the rest of the route on a thread pool, and it waits on the thread only with `asyncDelayed(false)` or for a transacted exchange.) This follows the review of the send hook (#28598) and the design document: the wait goes through `TimeSource.sleep` (camel-util), so a test harness that installs a virtual clock there (such as a `camel-test-dst` world) runs it in virtual time. It is the same for every exchange, transacted or not, and nothing is scheduled on another thread, so a route that stays on one thread in production stays on it under a delay. That thread can be an I/O thread: when a route goes on on the event loop thread of an asynchronous component (for example Netty) after its producer, a delay at a later step holds that event loop, which serves nothing else meanwhile, as a slow processor there would. The `StepHook` javadoc says so. A step that waits belongs to an inflight exchange, so a graceful shutdown waits for it. When the wait is interrupted (for example by `shutdownNow` after the graceful shutdown timed out), the processor is not called, the step fails with the `InterruptedException` and the thread keeps its interrupted status. As for any `InterruptedException` on an exchange, Camel marks the exchange as interrupted and stops routing it instead of handling the failure with `onException`, redelivery or the dead letter channel: the same result as when the synchronous wait of `RedeliveryErrorHandler` for a redelivery is interrupted. If the clock itself fails (a `TimeSource` provider of a test harness that throws), the step fails with that exception. An asynchronous delay can be added later as an opt-in if a use case shows up. **Async contract.** In `StepHelper` (camel-support), with a hook every path calls the callback exactly once, and `process` returns the value the callback is called with: a step failed by the hook (an exception set or thrown, or a reserved negative value; `done(true)`), a delayed step (after the wait, as any other step), an interrupted wait (`done(true)`; the interrupt flag is kept, the `InterruptedException` is set on the exchange, which marks it interrupted), a failing clock (its exception is set on the exchange; `done(true)`) and a processor that throws instead of calling the callback (thrown to the caller, as without a hook). An exchange that already has an exception when the step is called is processed as usual; only an exception the hook sets fails the step. The three places in `RedeliveryErrorHandler` ignore the return value, as they did. **What the javadoc covers.** The `StepHook` javadoc names the steps it is not asked for (a step disabled at runtime, a step the debugger skips, a step an exchange reaches after it was stopped or while Camel forces a shutdown), the error handlers that ask it (`RedeliveryErrorHandler` and the error handlers that extend it: the default error handler, the dead letter channel, and the Spring and JTA transaction error handlers), and the extra small processor with JMX (below). The CamelContext does not start or stop a hook, also one that is a `Service`, and a hook stays installed when the CamelContext is stopped and started again (`ExtendedCamelContext#setStepHook`). The small processor that asks the hook inside the JMX instrumentation (`DefaultChannel.StepHookProcessor`) is a `DelegateAsyncProcessor`, so start, stop and shutdown reach the processor of the step through it, as through the JMX wrapper around it (the point raised for the send hook's recipient-list wrapper on #28598 does not arise here). **No hook: nothing changes.** The channel and the error handler call the processor as before. The cost is one read of a plain field (the node id, `null` unless the hook is asked there) and, where it is set, one read of a volatile field (`getStepHook() == null`), per step attempt; with JMX and an error handler that can redeliver, also the call of the small processor inside the JMX wrapper. That processor is there without a hook too, because where the hook is asked is decided once, when the route starts, and a hook can be installed later on running routes; it passes start, stop and shutdown on to the processor of the step (it is a `DelegateAsyncProcessor`, as the JMX wrapper around it). The hook is a field of `ExtendedCamelContext`, as the send hook, not a context plugin, because it is read for every step. No upgrade guide entry, as nothing changes for users. A quick check, the same way as for the send hook, on an earlier version of this change (the code that runs without a hook has not changed since; no JMH; a `ProducerTemplate` into a route of 10 `process` steps that do nothing with the default error handler, the same with `noErrorHandler()`, the same with an error handler that can redeliver (`maximumRedeliveries(1)`), a `split` of 10 items with one step each, and an empty route; 6 alternating JVM runs of 9 x 100,000 calls each, the average of the 6 runs in ns per call, main / this change). Without JMX: 10 steps 1087 / 1106, without error handler 727 / 749, with redelivery 1480 / 1478, the split 4371 / 4312, the empty route 408 / 408: within the spread between runs of the same code. With JMX (camel-management on the classpath), where a step whose error handler can redeliver gets the small processor that asks the hook inside the JMX wrapper: 10 steps 1776 / 1798, without error handler 1529 / 1535, with redelivery 2019 / 2093 (about 7 ns per step, next to the about 54 ns per step JMX costs there already), the split 5237 / 5151. With a hook installed that lets every step proceed (a reflection proxy, so pessimistic), 2 runs without JMX: 1089, 721, 1459, 4221 and 388. **Not in this PR.** camel-chaos itself (CAMEL-26330 and the steps after this one in the design document). Auto-detecting a `StepHook` bean from the registry (as offered for the send hook). An asynchronous delay (see above). The wait itself is a few lines in `StepHelper` and in the send hook's `SendHelper`; I kept them apart so that the two PRs do not depend on each other. **For review.** - The name: Camel also has a `step` EIP; `StepHook` means every step of a route, as CAMEL-26338 and the design document call it, and its javadoc says so. Tests (camel-core and camel-management, no sleeps in the tests; waits are latches and `MockEndpoint` waits; the 1 to 50 ms delays are delays of the hook under test): - New `StepHookTest` (20 tests): the hook is asked once for each step of a route, in order, with the node id; a failure from the hook goes to `onException`, is redelivered twice (the hook asked three times) and dead-lettered, and a step that fails twice and then proceeds gets `CamelRedeliveryCounter` 2; the dead letter channel gets the same message whether the hook or the processor failed; a hook that throws fails the step, and so does a reserved negative value (redelivered and dead-lettered with the `IllegalArgumentException`); inside `doTry` `doCatch` gets the failure (also for a transacted exchange); `noErrorHandler()`: the caller gets the hook's exception and the hook is asked once (no redelivery), and the steps are asked as usual; steps inside `split` (one item fails, handled by `onException`, the others go on), `multicast` (a failed output is redelivered by the multicast's error handler for that output, asked again each time, then dead-lettered) and `choice`; `recipientList` , `routingSlip` and `dynamicRouter` are each asked once, as a step (the error handlers they keep for their recipients do not ask), and a failure of the `recipientList` step is redelivered; a delayed step runs on the thread that runs the route, also for a transacted exchange; a step disabled at runtime is not asked; without a hook nothing is asked and the routes behave as without the change, and a hook installed later is asked by the running routes. - New `StepHelperTest` (35 tests): the callback contract for each hook outcome (proceed, a reserved negative value, fail, the hook throws, delay) with a synchronous processor, an asynchronous one (completes on another thread) and one that throws, and with a transacted exchange; the synchronous variant used by the channel for a transacted exchange. A delay waits on the thread that calls the step, through `TimeSource`, for a transacted and a non-transacted exchange (the test installs a clock that records the 2500 ms wait without waiting; `TimeSource` is JVM-wide, so the class is `@Isolated`, as `TimeSourceTest` in camel-util). An interrupted wait fails the step with the `InterruptedException` without calling the processor, keeps the interrupt flag and marks the exchange interrupted; a clock that throws fails the step with its exception; a reserved value fails the step with an `IllegalArgumentException`; an exchange that has failed already is processed as usual. A stress test: 16 thr eads x 500 steps with random outcomes and processors; every step completes exactly once, with the value `process` returned. - New `ManagedStepHookTest` (camel-management, 3 tests): a step that the hook fails twice and then lets through is counted in its processor MBean as for a processor that fails twice (failed 2, completed 1, `Redeliveries` 2, the route's `Redeliveries` 1) when the error handler can redeliver; with an error handler that cannot redeliver the failure is counted too; a delay of the hook is processing time of the step in both cases. - New `StepHookStressTest`: 8 threads x 250 exchanges through a route with a dead letter channel with redelivery, an asynchronous producer, `split`, `doTry`, `multicast` and `choice`, with a hook that randomly fails or delays each step: each exchange completes exactly once (one completed or failed event per exchange, every future completes). - The new tests use the new SPI, so they do not compile on main. The tests that changed with the move of the delay to the thread of the step fail against the production code of the earlier version, which scheduled the delay on the error handler's pool: 12 of the 56 camel-core tests (`StepHookTest#testDelayWaitsOnTheThreadThatRunsTheStep` and `#testReservedValueFailsTheStep`; `StepHelperTest#testDelayInterrupted`, `#testReservedValueFailsTheStep`, `#testFailingClockFailsTheStep`, the non-transacted case of `#testDelayWaitsOnTheCallerThreadThroughTimeSource`, and the reserved value in the five parameterized tests and in the stress test). The transacted case of the `TimeSource` test passes there too, as a transacted exchange already waited that way. In an earlier version (which asked the hook outside the JMX wrapper), `ManagedStepHookTest` failed 2 of its 3 tests (the redelivery case and the delay). Mutations of that version, each run alone, were each caught by the tests: the three p laces in the error handler not asking, the channel not asking (`doTry`, `noErrorHandler`, `multicast`), the redelivery not asking again, the channel asking before the error handler instead of handing over to it (`onException`, redelivery, dead letter channel), and both asking; and with JMX on, asking in both places, in none, or outside the JMX wrapper (`ManagedStepHookTest`). - The whole camel-core suite (with its upstream core modules), on this commit and on main (ffef175654d1, the commit this branch starts from), in the same local offline build: the same result for every test class; the only difference is the three new test classes (56 tests). The failures are the same on both and known from my local offline builds: `SimpleSyntaxHintsTest` (6), three OGNL hint tests of `SimpleTest`, `FileProducerChecksumOfWrittenFileTest` (3), `FilePollEnrichDynamicFileNameTest`, and in camel-xml-io and camel-yaml-io the REST model writer tests (`ModelWriterTest` 4, `YamlModelWriterTest` 1). Two tests failed once on this commit and passed on the rerun in the same build: `ThrottlingExceptionRoutePolicyTest#testOpenCircuitToPreventMessageThree` (it also passed in a separate run of the throttling tests) and `ControlBusFailRouteTest#testControlBusFail` (marked as flaky on CI). - camel-management: 461 tests on this commit (with `ManagedStepHookTest`) and 458 on main, 0 failures, the same for every other test class (`-Dcamel.surefire.parallel=false`, one test class at a time, as the test classes share one MBean server). - On the previous version of this commit (the same code; this one changes javadoc only), with the send hook (CAMEL-26331, #28598) merged in locally (no conflict in either order; the generated `ExtendedCamelContextConfigurer` then has both properties, as the build generates it): the tests of both hooks and the tests of the touched areas (`*Redelivery*`, `*DeadLetter*`, `*ErrorHandler*`, `*OnException*`, `*ProducerCache*`, `*EventNotifier*`, `*Channel*`, `*Multicast*`, `*RecipientList*`, `*RoutingSlip*`, `*DynamicRouter*`, `*Split*`, `*Try*`, `*Transacted*`): 1373 tests in camel-core and 142 in camel-management, 0 failures. - This commit changes only the javadoc of `StepHook` and `ExtendedCamelContext#setStepHook` over the previous version, which ran the suites above on a7c1a933ec49 with the same per-class result. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL-26338) filed for the change (usually before you start working on it): [CAMEL-26338](https://issues.apache.org/jira/browse/CAMEL-26338). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules and ran the tests above, including the formatter and import-sort plugins. The new `stepHook` property is in the regenerated `ExtendedCamelContextConfigurer`. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
