gnodet-bot commented on code in PR #27714:
URL: https://github.com/apache/camel/pull/27714#discussion_r4246846802
##########
components/camel-ai/camel-ai-tool/src/test/java/org/apache/camel/component/ai/tool/AiToolAuthorizationDeniedEventTest.java:
##########
@@ -199,6 +231,75 @@ public void notify(CamelEvent event) {
AiToolResult result = AiToolExecutor.execute(spec, Map.of(), exchange);
assertThat(result).isInstanceOf(AiToolResult.AuthorizationDenied.class);
+ // the healthy @BeforeEach notifier still received the event despite
the other one throwing
+ assertThat(events).hasSize(1);
Review Comment:
💡 **Suggestion (minor):** The test verifies that the `@BeforeEach` notifier
received the event even when the throwing notifier failed — but since the
`@BeforeEach` notifier is at index 0 and the throwing notifier is at index 1,
delivery to index 0 happens *before* the throw, not after it. The assertion
therefore proves ordering rather than isolation.
To truly demonstrate that `doNotifyEvent` continues delivery past a failing
notifier, a third notifier registered *after* the throwing one (index 2) would
need to receive the event:
```suggestion
// add a trailing notifier (registered after the thrower) to prove
delivery continues past a failure
List<AiEvent> trailing = new CopyOnWriteArrayList<>();
context.getManagementStrategy().addEventNotifier(new
EventNotifierSupport() {
@Override
public void notify(CamelEvent event) {
if (event instanceof AiEvent ai) {
trailing.add(ai);
}
}
});
AiToolResult result = AiToolExecutor.execute(spec, Map.of(),
exchange);
assertThat(result).isInstanceOf(AiToolResult.AuthorizationDenied.class);
// the healthy @BeforeEach notifier still received the event despite
the other one throwing
assertThat(events).hasSize(1);
// the trailing notifier (registered after the thrower) also
received it — proving isolation
assertThat(trailing).hasSize(1);
}
```
##########
core/camel-support/src/main/java/org/apache/camel/support/EventHelper.java:
##########
@@ -1574,6 +1576,49 @@ public static boolean
notifyExchangeAsyncProcessingStartedEvent(CamelContext con
return answer;
}
+ /**
+ * Notifies an {@link AiEvent} to the started event notifiers, following
the same path as every other event
+ * category: it loops the started notifiers, skips those that are disabled
or have {@code ignoreAiEvents} set,
+ * creates the event lazily (via {@code eventSupplier}) only when some
notifier wants it, and delivers through
+ * {@code doNotifyEvent} so one failing notifier does not stop delivery to
the others.
+ * <p>
+ * AI events are their own category: {@code ignoreExchangeEvents} does
<em>not</em> suppress them even though an
+ * {@code AiEvent} is an exchange event. A notifier opts out with {@code
setIgnoreAiEvents(true)}.
+ *
+ * @since 4.24
+ */
+ public static boolean notifyAiEvent(CamelContext context,
Supplier<AiEvent> eventSupplier) {
+ ManagementStrategy management = context != null ?
context.getManagementStrategy() : null;
+ if (management == null) {
+ return false;
+ }
+
+ List<EventNotifier> notifiers = management.getStartedEventNotifiers();
+ if (notifiers == null || notifiers.isEmpty()) {
+ return false;
+ }
+
+ boolean answer = false;
+ CamelEvent event = null;
+ // optimise for loop using index access to avoid creating iterator
object
+ for (int i = 0; i < notifiers.size(); i++) {
+ EventNotifier notifier = notifiers.get(i);
+ if (notifier.isDisabled() || notifier.isIgnoreAiEvents()) {
+ continue;
+ }
+
+ if (event == null) {
+ // only create event once, and only when a notifier wants it
+ event = eventSupplier.get();
+ if (event == null) {
+ return false;
+ }
+ }
+ answer |= doNotifyEvent(notifier, event);
Review Comment:
💡 **Suggestion (nit):** `OpenTelemetryExchangeEventNotifier.isEnabled()`
already filters to `ExchangeEvent` subclasses, so AI events won't reach its
`notify()` regardless. Setting `ignoreAiEvents(true)` in its constructor would
be a minor early-exit optimisation (skipping the `isEnabled()` call entirely),
but it's not a correctness concern.
Worth considering in a follow-up once this API stabilises — no need to block
this PR on it.
--
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]
