allthingssecurity opened a new pull request, #28807: URL: https://github.com/apache/camel/pull/28807
# Description [CAMEL-26451](https://issues.apache.org/jira/browse/CAMEL-26451) When a step of a routing slip or dynamic router completes asynchronously, the callback of the step in `RoutingSlip.processExchange` took the snapshot copy for the next step, ran all the remaining steps, copied the result back onto the exchange, and only then called the callback of the producer cache, which fires the step's `ExchangeSentEvent` and releases its producer. Asynchronous completion is the usual case: under the default error handler nearly every step completes asynchronously, as the error handler of each step (`RedeliveryErrorHandler.process`) schedules its work, even for a `mock:` or `log:` step; a step is synchronous only with `noErrorHandler`, inside a `doTry` or under a transaction error handler, with a synchronous producer. The synchronous path already fired the sent event before the next step. With `routingSlip(constant("direct:step1,direct:step2"))`, step1 taking about 10 ms and step2 about 300 ms, an event notifier saw: ``` main: sending step1, sending step2, sent step2 (300 ms), sent step1 (310 ms) expected: sending step1, sent step1 (10 ms), sending step2, sent step2 (300 ms) ``` The same with `dynamicRouter` (which extends `RoutingSlip`) and with `cacheSize=-1`; `recipientList` and `to` give the expected order. So: - the time taken of the sent event of a step covered all the following steps; - the sent event of the first step carried the message and the failure after the last step (the first step runs on the exchange itself, onto which the result is copied), so the first step was reported as failed when a later step failed; - the consumers of the sent events were affected: the exchange event notifier timers of camel-micrometer and camel-opentelemetry-metrics (time and `failed` tag), camel-jfr's send events (pushed and popped on a stack in the exchange, so mispaired), the deprecated camel-tracing, and routes that consume camel-event events; - the producer of a step was released only when the whole routing slip was done; - with camel-telemetry (camel-opentelemetry2, camel-micrometer-observability, camel-telemetry-dev), the snapshot for the next step was taken while the send span of the previous step was still on the span stack, and that stack was copied back onto the exchange: the end events of the routing slip and its route pulled the wrong spans, and the outermost span (such as the send of a `ProducerTemplate`) was never ended. Changes: - The step's callback now calls the producer cache callback right away (the step's sent event, the producer released). - The remaining steps are run in the callback that the producer cache calls afterwards (`doInAsyncProducer`'s callback), in a new private method `continueRoutingSlip`. It holds the loop that was in the step's callback, unchanged except that the callback of a next step that completes asynchronously now leads to the routing slip's original callback directly, instead of through the previous step's producer cache callback (which has already run). - 4.24 upgrade guide: what users of the sent events see (lower step times, the first step no longer failed by a later step), and that there is no option to keep the old order. - A stale comment in the moved loop (it named a `nextExchange` that does not exist) is corrected. Tests (camel-core): - `RoutingSlipSentEventOrderTest`, 6 tests: routing slip with asynchronous steps, with steps that delay synchronously (they still complete asynchronously), with `cacheSize(-1)`, a dynamic router, the message the sent event of each step sees, and that a later failing step does not make the sent event of an earlier step failed. - `RoutingSlipEventNotifierTest` and `DynamicRouterEventNotifierTest` (three `mock` steps) now also check the order of the events. - All 8 tests fail with the `RoutingSlip` of main. - The three test classes 20 times in one JVM: 160 tests, 0 failures. - camel-core `*RoutingSlip*`, `*DynamicRouter*`, `*Slip*`, `*EventNotifier*`, `*RecipientList*`, `*Notify*` tests: 351, 0 failures (2 skipped). - camel-core `processor/**` and the error-handling tests of `issues/` (redeliver, try, catch, finally, error handler, exception), run on the first version of this change (the production code differs since only by a parameter name and a comment): 3002 tests, 1 failure, `BacklogTracerCalledRouteTimestampTest`, which uses no routing slip and also fails intermittently with the `RoutingSlip` of main (1 of 3 runs); it passed 20 of 20 with this change. The formatter and import-sort plugins ran. How it was found: a Lean model of the span stack of camel-telemetry, extended to the snapshot copies of the routing slip. With the snapshot taken before the step's sent event, the step's span stays on the exchange's stack after the routing slip; with the sent event first, the exchange gets back the stack it had before the routing slip, for any number of steps (proved). A tracing probe showed the broken spans, and a plain event notifier showed the cause in camel-core. With camel-telemetry, routing slips and dynamic routers also need the camel-telemetry fix proposed together with this one (the span of an asynchronous step is ended before the next step): with both changes a probe over routing slips and dynamic routers to routes with an asynchronous `delay` and `threads` shows every span ended by its own end; with either one alone it does not. A property check over random routes (nested split, multicast, filter, loop, doTry/doCatch, wireTap, routing slip, recipient list and called routes, with asynchronous `delay` and `threads` steps) with a recording tracer: with this change and the companion one (CAMEL-26450), 0 of 1250 routes have a span ended by another step's end or left open (four seeds; the last two also with enrich, asynchronous throttle, dynamic router, idempotent consumer and toD); with the code of main, 131 of 150. Related: CAMEL-26327 (#28750) fixed a hang of a redelivered routing slip step in the same callback; the redelivery tests of the routing slip and dynamic router pass with this change, and a comparison probe over redelivery, nested slips and dynamic routers, `doTry`, `ignoreInvalidEndpoints` and `cacheSize(-1)` showed identical results apart from the event order. The sibling EIPs were checked: Multicast, Recipient List and Split fire the sent event of a part before the next part, `toD` and `enrich` only post-process, and `to`, Pipeline and the load balancers go through `SendProcessor`. Not changed: the snapshot for the next step is still taken right after a step returns, also when the step continues asynchronously and the snapshot is thrown away (as before); `getNextCallback` still stops a prototype endpoint that the step's callback has already stopped (stopping is idempotent). Backport: the same code is on camel-4.14.x and camel-4.18.x. The event order changes for event notifiers, so main only unless a backport is wanted. # 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-26451) filed for the change (usually before you start working on it): [CAMEL-26451](https://issues.apache.org/jira/browse/CAMEL-26451). # 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 listed above, including the formatter and import-sort plugins. 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]
