davsclaus commented on code in PR #25507:
URL: https://github.com/apache/camel/pull/25507#discussion_r3788714109
##########
core/camel-main/src/main/java/org/apache/camel/main/BaseMainSupport.java:
##########
@@ -150,6 +150,7 @@ public abstract class BaseMainSupport extends BaseService {
private static final String PREFIX_TRACE = "camel.trace.";
private static final String PREFIX_ROUTE_CONTROLLER =
"camel.routeController.";
private static final String PREFIX_ERROR_REGISTRY = "camel.errorRegistry.";
+ private static final String PREFIX_AI_OBSERVABILITY =
"camel.ai.observability.";
Review Comment:
The property prefix `camel.ai.observability.` uses two-level nesting, but
every existing Camel Main configuration group uses a single-level prefix under
`camel.`:
- `camel.health.`, `camel.opentelemetry2.`, `camel.errorRegistry.`,
`camel.routeController.`, `camel.management.`, etc.
This should be `camel.ai-observability.` (hyphenated, single level) to
follow the established convention. The rename needs to ripple through: this
constant, `GROUP_PREFIXES`, `PrepareCamelMainMojo`,
`AiObservabilityConfigurationProperties`, metadata JSON, and docs.
##########
components/camel-spring-parent/camel-spring-ai/camel-spring-ai-chat/src/main/java/org/apache/camel/component/springai/chat/SpringAiChatProducer.java:
##########
@@ -965,8 +989,6 @@ private <T> void processStructuredOutputRequest(
// Also set headers
exchange.getMessage().setHeader(SpringAiChatConstants.CHAT_RESPONSE,
responseText);
exchange.getMessage().setHeader(SpringAiChatConstants.STRUCTURED_OUTPUT,
structuredOutput);
Review Comment:
The old code called `populateTokenUsage(response, exchange)` after this
line, but that call was removed. `callWithObservability()` records token data
on the span but does NOT populate the exchange headers (`INPUT_TOKEN_COUNT`,
`OUTPUT_TOKEN_COUNT`, `TOTAL_TOKEN_COUNT`).
Compare with:
- The entity path (`processEntityRequest`) which correctly calls both
`recordObservationSuccess` AND `populateTokenUsage`
- The default chat path which calls `populateResponse()` (which includes
`populateTokenUsage`)
Please restore `populateTokenUsage(response, exchange)` here.
##########
components/camel-spring-parent/camel-spring-ai/camel-spring-ai-chat/src/main/java/org/apache/camel/component/springai/chat/SpringAiChatProducer.java:
##########
@@ -852,13 +866,24 @@ private Class<?> getEntityClass(Exchange exchange) {
*/
private <T> void processEntityRequest(
ChatClient.ChatClientRequestSpec request, Exchange exchange,
Class<T> entityClass) {
- // Execute the request and convert to entity
- T entity = request.call().entity(entityClass);
-
- // Set the entity as the body
- exchange.getMessage().setBody(entity);
-
- LOG.debug("Converted response to entity of type: {}",
entityClass.getName());
+ GenAiObservationContext observationContext = buildObservationContext();
+ GenAiObservation observation = GenAiObservability.start(exchange,
observationContext);
+ try {
+ org.springframework.ai.chat.client.ResponseEntity<ChatResponse, T>
responseEntity
Review Comment:
Per project conventions: "Do NOT use fully qualified class names (FQCNs) in
Java code. Always add an import statement and use the simple class name."
Since `org.springframework.http.ResponseEntity` is not imported in this
file, a regular import for `org.springframework.ai.chat.client.ResponseEntity`
would work without conflicts.
```suggestion
ResponseEntity<ChatResponse, T> responseEntity
```
##########
core/camel-main/src/main/java/org/apache/camel/main/BaseMainSupport.java:
##########
@@ -150,6 +150,7 @@ public abstract class BaseMainSupport extends BaseService {
private static final String PREFIX_TRACE = "camel.trace.";
private static final String PREFIX_ROUTE_CONTROLLER =
"camel.routeController.";
private static final String PREFIX_ERROR_REGISTRY = "camel.errorRegistry.";
+ private static final String PREFIX_AI_OBSERVABILITY =
"camel.ai.observability.";
Review Comment:
The property prefix `camel.ai.observability.` uses two-level nesting, but
every existing Camel Main configuration group uses a single-level camelCase
prefix under `camel.`:
- `camel.health.`, `camel.opentelemetry2.`, `camel.errorRegistry.`,
`camel.routeController.`, `camel.management.`, etc.
The canonical name should be `camel.aiObservability.enabled` (camelCase,
matching e.g. `camel.routeController.`), with `camel.ai-observability.enabled`
(dash-style) also accepted. The rename needs to ripple through: this constant,
`GROUP_PREFIXES`, `PrepareCamelMainMojo`,
`AiObservabilityConfigurationProperties`, metadata JSON, and docs.
--
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]
