davsclaus commented on code in PR #25846:
URL: https://github.com/apache/camel/pull/25846#discussion_r3890313253
##########
components/camel-exec/src/main/java/org/apache/camel/component/exec/impl/DefaultExecBinding.java:
##########
@@ -45,12 +47,26 @@ public class DefaultExecBinding implements ExecBinding {
private static final Logger LOG =
LoggerFactory.getLogger(DefaultExecBinding.class);
+ private static final String[] CONTROL_HEADERS = {
+ EXEC_COMMAND_EXECUTABLE,
+ EXEC_COMMAND_ARGS,
+ EXEC_COMMAND_OUT_FILE,
+ EXEC_COMMAND_WORKING_DIR,
+ EXEC_COMMAND_TIMEOUT,
+ EXEC_COMMAND_EXIT_VALUES,
+ EXEC_USE_STDERR_ON_EMPTY_STDOUT,
+ EXEC_COMMAND_LOG_LEVEL
+ };
+
+ private final AtomicBoolean ignoredControlHeadersWarned = new
AtomicBoolean();
Review Comment:
The new "warn once" diagnostic is scoped too broadly to be useful across
multiple endpoints.
`ignoredControlHeadersWarned` is an instance field on `DefaultExecBinding`,
but `ExecComponent` creates exactly one `DefaultExecBinding` (`private
ExecBinding binding = new DefaultExecBinding();`) and hands that *same
instance* to every endpoint it creates (`endpoint.setBinding(binding)` in
`ExecComponent.createEndpoint`). Since `ExecComponent` is a per-`CamelContext`
singleton, **every `exec:` endpoint in a CamelContext shares one binding, and
therefore one `AtomicBoolean`.**
I verified this with a reproduction: created two different `exec:` endpoints
(`exec:hostname`, `exec:echo`) from the same component, confirmed
`endpointA.getBinding() == endpointB.getBinding()`, then triggered an
ignored-control-header condition on each. Only the *first* endpoint logged the
WARN — the second, with a completely different URI and a different ignored
header, produced no log output at all, because the `compareAndSet` guard had
already flipped.
Impact: in a CamelContext with multiple `exec:` endpoints, only the first
one to hit a misconfigured/ignored control header ever gets the diagnostic —
every other misconfigured endpoint (the exact scenario this PR is meant to help
diagnose) silently gets no warning. Not covered by
`DefaultExecBindingTest`/`ExecProducerTest` since each only exercises a single
endpoint per binding instance.
Suggest tracking warned state per-endpoint instead of per-binding-instance,
e.g. a `Set<ExecEndpoint>` (or endpoint URI string) built with
`ConcurrentHashMap.newKeySet()`, rather than a single `AtomicBoolean`.
_This review was generated by an AI agent and may contain inaccuracies.
Please verify all suggestions before applying._
--
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]
