davsclaus commented on code in PR #26148: URL: https://github.com/apache/camel/pull/26148#discussion_r3952112266
########## components/camel-ai/camel-openai/src/main/java/org/apache/camel/component/openai/AbstractOpenAIExchangeEvent.java: ########## @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.openai; + +import java.util.EventObject; + +import org.apache.camel.Exchange; +import org.apache.camel.spi.CamelEvent; + +/** + * Base class for OpenAI agentic loop {@link CamelEvent} notifications. + * <p> + * These are {@link CamelEvent.Type#Custom} events and are not {@link CamelEvent.ExchangeEvent} instances so they do not + * pollute generic exchange lifecycle metrics. {@link #getExchange()} is public so {@code EventNotifier} listeners can + * correlate events with the exchange (for example route id or exchange id) without implementing {@code ExchangeEvent}. + */ +abstract class AbstractOpenAIExchangeEvent extends EventObject implements CamelEvent { + + private final Exchange exchange; + private long timestamp; Review Comment: The class extends `EventObject` (which is `Serializable`) but does not declare a `serialVersionUID`. All three concrete subclasses already define `@Serial private static final long serialVersionUID = 1L` — please add one here too: ```suggestion abstract class AbstractOpenAIExchangeEvent extends EventObject implements CamelEvent { @Serial private static final long serialVersionUID = 1L; ``` ########## tooling/spi-annotations/src/main/java/org/apache/camel/spi/Metadata.java: ########## @@ -32,6 +32,12 @@ @Target({ ElementType.TYPE, ElementType.METHOD, ElementType.FIELD }) public @interface Metadata { + /** + * The catalog kind for this metadata entry (for example {@code header} or {@code exchangeProperty}). When empty, + * tooling applies its default classification. + */ Review Comment: Missing `@since 4.23` tag. Per project conventions, all new public methods/elements added to `core/camel-api` (and its annotation counterpart) must carry a `@since` tag. ```suggestion /** * The catalog kind for this metadata entry (for example {@code header} or {@code exchangeProperty}). When empty, * tooling applies its default classification. * * @since 4.23 */ String kind() default ""; ``` ########## components/camel-ai/camel-openai/src/main/java/org/apache/camel/component/openai/AbstractOpenAIExchangeEvent.java: ########## @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.openai; + +import java.util.EventObject; + +import org.apache.camel.Exchange; +import org.apache.camel.spi.CamelEvent; + +/** + * Base class for OpenAI agentic loop {@link CamelEvent} notifications. + * <p> + * These are {@link CamelEvent.Type#Custom} events and are not {@link CamelEvent.ExchangeEvent} instances so they do not + * pollute generic exchange lifecycle metrics. {@link #getExchange()} is public so {@code EventNotifier} listeners can + * correlate events with the exchange (for example route id or exchange id) without implementing {@code ExchangeEvent}. + */ +abstract class AbstractOpenAIExchangeEvent extends EventObject implements CamelEvent { + + private final Exchange exchange; + private long timestamp; + + protected AbstractOpenAIExchangeEvent(Exchange exchange) { + super(exchange); + this.exchange = exchange; + } + + public Exchange getExchange() { + return exchange; + } + + @Override + public Object getSource() { + return exchange; + } + + @Override Review Comment: `getSource()` is redundant here — `EventObject` already stores the constructor argument as the source and returns it from its own `getSource()`. Since the constructor calls `super(exchange)`, this override adds no value and can be removed. ########## dsl/camel-endpointdsl/src/generated/java/org/apache/camel/builder/endpoint/dsl/OpenAIEndpointBuilderFactory.java: ########## @@ -2512,6 +2512,21 @@ public String openAIAgenticCompletionTokens() { public String openAIAgenticTotalTokens() { return "CamelOpenAIAgenticTotalTokens"; } + /** + * Per-iteration breakdown of the OpenAI MCP agentic loop, including + * tool calls, truncated arguments/results, token usage, and duration + * for each iteration. + * + * The option is a: {@code + * java.util.List<org.apache.camel.component.openai.AgenticIterationTrace>} type. + * + * Group: producer + * + * @return the name of the header {@code OpenAIAgenticTrace}. + */ Review Comment: The `@return` says *"the name of the **header**"* but `CamelOpenAIAgenticTrace` is an exchange **property** (declared `kind = "exchangeProperty"` in `@Metadata`). Callers reading this Javadoc will use `exchange.getIn().getHeader(...)` when they should use `exchange.getProperty(...)`. This file is generated — please fix the metadata/generator source so this regenerates with the correct wording. -- 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]
