CAMEL-23223: Stabilize flaky ManagedMessageHistoryAutoConfigIT#24425
Conversation
Replace the short 1-second Awaitility wait (which only waited for any log to appear) with untilAsserted using a 20-second timeout that retries the full set of metric assertions. This handles slow CI architectures where the OTel periodic reader may not export Camel metrics within 1 second, and avoids failures from incomplete early metric exports during message processing. Also make MemoryLogHandler thread-safe with CopyOnWriteArrayList since publish() is called from the OTel exporter thread while getLogs()/hasLogs() are called from the test thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Guillaume Nodet <gnodet@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 9 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 9). Modules only in current approach (8)
Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
All tested modules (9 modules)
|
There was a problem hiding this comment.
Pull request overview
Stabilizes the ManagedMessageHistoryAutoConfigIT.testMessageHistory integration test in camel-opentelemetry-metrics by making metric assertions resilient to delayed/partial OpenTelemetry periodic exports and by hardening the in-memory log capture against concurrent access.
Changes:
- Replaces “wait for any export → assert once” with an
await().untilAsserted(...)loop that retries the full metric assertions until Camel metrics are available and complete. - Asserts against the last exported Camel
MetricDatarather than every exported entry to avoid transient/incomplete exports during message processing. - Makes
MemoryLogHandlerthread-safe by usingCopyOnWriteArrayListfor cross-thread log publishing/reading.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/camel-opentelemetry-metrics/src/test/java/org/apache/camel/opentelemetry/metrics/integration/messagehistory/ManagedMessageHistoryAutoConfigIT.java | Reworks the Awaitility strategy and assertion target to reduce flakiness on slower CI architectures. |
| components/camel-opentelemetry-metrics/src/test/java/org/apache/camel/opentelemetry/metrics/integration/MemoryLogHandler.java | Switches log storage to a thread-safe list to avoid races between exporter and test threads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await().atMost(Duration.ofSeconds(20)).untilAsserted(() -> { | ||
| List<LogRecord> logs = new ArrayList<>(handler.getLogs()); | ||
| assertFalse(logs.isEmpty(), "No metrics were exported"); | ||
|
|
||
| MetricData lastCamelMetric = null; |
There was a problem hiding this comment.
Addressed both points in c26c8fe:
- Added
logger.removeHandler(handler)in afinallyblock to prevent handler accumulation. - Removed the redundant
new ArrayList<>()wrapper (and the now-unusedArrayListimport) sincegetLogs()already returns a defensive copy.
- Remove handler from logger in finally block to prevent handler accumulation across test runs in the same JVM - Remove redundant new ArrayList<>() wrapper since getLogs() already returns a defensive copy - Remove unused ArrayList import Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Guillaume Nodet <gnodet@gmail.com>
Summary
Fixes the intermittent failure of
ManagedMessageHistoryAutoConfigIT.testMessageHistoryincamel-opentelemetry-metrics, reported across various CI architectures.Root cause: The test used
await().atMost(1000ms).until(handler::hasLogs)which only waited for any log to appear (which could be a non-Camel OTel internal metric), then immediately asserted on all metrics. On slow CI architectures (ppc64le, s390x), the Camel-specific metrics may not have been exported yet within 1 second. Additionally, early metric exports during message processing could contain incomplete data, causing the per-entry assertions to fail.Fix:
await().atMost(20s).untilAsserted(...)that retries the full set of metric assertions until the OTel periodic reader exports Camel metricsMemoryLogHandlerthread-safe withCopyOnWriteArrayListsincepublish()is called from the OTel exporter thread whilegetLogs()/hasLogs()are called from the test threadfinallyblock to prevent handler accumulation across test runs in the same JVMnew ArrayList<>()wrapper sincegetLogs()already returns a defensive copyTest plan
ManagedMessageHistoryAutoConfigITpasses consistently (verified 3 consecutive runs locally)Claude Code on behalf of @davsclaus
🤖 Generated with Claude Code