apupier opened a new pull request, #29721:
URL: https://github.com/apache/camel/pull/29721

   ## Summary
   
   Fixes the flaky `AsyncCxfTest` (camel-opentelemetry) reported in 
CAMEL-26890. It failed with `expected: <4> but was: <2>` in 
`verifyTraceSpanNumbers`, and with `There must be no leaking span after test` 
(a leaked `cxfrs` CLIENT span).
   
   ## Root cause
   
   OpenTelemetry scopes are bound to the thread that created them. For an async 
producer (`cxfrs` with `synchronous=false`):
   
   - the route thread opens the scope in `ActiveSpanManager.activate()` and, 
once the producer returns, `Tracer` calls `ActiveSpanManager.endScope()` (on 
`ExchangeAsyncProcessingStartedEvent`);
   - the CXF callback thread calls `ActiveSpanManager.deactivate()` (on 
`ExchangeSentEvent`).
   
   Nothing orders the two. If the callback thread wins, `deactivate()` cannot 
close the scope (wrong thread) and restores the parent holder; `endScope()` on 
the route thread then closes the **parent** scope, and the CLIENT scope leaks 
into the thread context and corrupts the following exchanges.
   
   ## Fix
   
   `ActiveSpanManager.Holder` remembers its owner thread:
   
   - `deactivate()` on a foreign thread no longer tries to close the scope; it 
records the holder as pending on the exchange (before restoring the parent).
   - `endScope()` closes the pending holder when called on its owner thread.
   - `closeScope()` is idempotent once closed by the owner thread.
   
   ## Validation
   
   - New unit test 
`ActiveSpanManagerTest.testDeactivateOnOtherThreadBeforeEndScope` reproduces 
the interleaving deterministically.
   - The race did not occur naturally (25/25 passes on unmodified code). A 
temporary 400 ms delay before `endScope()` for `cxfrs` exchanges (not part of 
this PR) made `AsyncCxfTest` fail 2/2 without the fix (`expected: <4> but was: 
<1>` + leaked span) and pass 5/5 with it.
   - `camel-tracing` (93 tests) and `camel-opentelemetry` (25 tests) pass.
   
   ## Limitations
   
   The delay used to force the race is artificial, and the `<4> but was: <2>` 
variant seen on CI is assumed to be the same race with different timing. Worth 
watching the Develocity flaky stats of `AsyncCxfTest` after merge.
   
   ---
   _IBM Bob 2.2.1 on behalf of apupier (AI-generated PR description)_
   


-- 
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]

Reply via email to