CAMEL-23861: GenAI observability for langchain4j and OpenAI (Phase 1) - #25337
CAMEL-23861: GenAI observability for langchain4j and OpenAI (Phase 1)#25337atiaomar1978-hub wants to merge 11 commits into
Conversation
Phase 1 implementation summaryAI-generated comment on behalf of atiaomar1978-hub This PR implements Claus Ibsen's Phase 1 scope from CAMEL-23861:
Review fixes already applied
Tests12 unit tests in Ready for review when CI is green. |
|
@davsclaus @oscerd — could you please review this Phase 1 implementation for CAMEL-23861? Scope per Claus's JIRA guidance: GenAI observability (OTel CLIENT spans + Micrometer metrics) for langchain4j-chat, langchain4j-tools, langchain4j-agent, langchain4j-embeddings, and openai, with global toggle and model headers. AI-generated comment on behalf of atiaomar1978-hub |
davsclaus
left a comment
There was a problem hiding this comment.
Thank you for this contribution — adding GenAI observability is a valuable addition. I have a few concerns that should be addressed before this can be merged.
Summary of findings
- [Critical]
resolveResponseModelalways returnsrequestModel— the actual response model fromChatResponse.modelName()is never used, making theRESPONSE_MODELheader and span attribute identical toREQUEST_MODELin all cases. - [High]
micrometer-coreis a mandatory (compile-scope) dependency — should be<optional>true</optional>to avoid forcing Micrometer onto the classpath of all AI component users. - [Medium] OpenAI streaming observation records no token usage (all nulls).
- [Medium] No tests for the actual producer integrations (langchain4j-chat, langchain4j-tools, langchain4j-agent, langchain4j-embeddings, openai). Per project rules, every PR must include tests for new functionality.
- [Medium]
GenAiModelResolverrelies on fragile reflection and class-name substring matching. - [Low] Missing
@since 4.22on new publicgetChatModel()method inAbstractAgent.
See inline comments for details.
Note: This review covers project rules, conventions, and code-level issues visible from the diff. It does not replace specialized tools such as SonarCloud or CodeRabbit.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
Addressed @davsclaus review feedback in commit
Tests: AI-generated comment on behalf of atiaomar1978-hub |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
Fixed the two CI failures (documentation validation + docs xref-check): Root cause: The upgrade guide referenced Changes in
Verified locally with AI-generated on behalf of atiaomar1978-hub |
davsclaus
left a comment
There was a problem hiding this comment.
Claude Code on behalf of davsclaus
Thank you for addressing the previous review feedback — the micrometer optional fix, @since tag, ModelProvider-based resolution, and streaming token capture are all good improvements.
A few issues remain before this can be merged:
Confirmed issues
-
[Critical] CI build is failing — the "Validate documentation" step fails. This is likely related to the
nav.adocissue below. -
[High]
nav.adocis auto-generated — should not be edited directly. The file header says "this file is auto generated and changes to it will be overwritten — make edits in docs/*nav.adoc.template files instead". The build generates entries automatically from the module's docs structure via<!-- inject:adoc -->markers indocs/others-nav.adoc.template. Remove the manual nav.adoc edit and let the build handle it. -
[Medium]
RESPONSE_MODELheader set torequestModelin agent and embeddings producers. InLangChain4jAgentProducer.populateResultHeaders()andLangChain4jEmbeddingsProducer.populateHeaders(), bothREQUEST_MODELandRESPONSE_MODELheaders are set to the same value. The response model can differ from the request model (e.g.,gpt-4o→gpt-4o-2024-11-20). When the actual response model is unavailable from the result object, either omit theRESPONSE_MODELheader or document that it mirrors the request. Same applies to theGenAiUsageresponse model in the agent observation span. -
[Medium] No tests for producer-level observability integration. Per project rules, every PR must include tests for new functionality. The unit tests for the
camel-ai-observabilitymodule itself are good, but there are no tests verifying that the actual producer instrumentation emits spans/metrics (langchain4j-chat, tools, agent, embeddings, openai producers). -
[Low] Adding observability to deprecated
camel-langchain4j-tools. This component was deprecated in favor ofai-tool+langchain4j-agent. Adding new features to a deprecated component is unusual — consider whether this is worth the maintenance cost.
Note: This review covers project rules, conventions, and code-level issues visible from the diff. It does not replace specialized tools such as SonarCloud or CodeRabbit.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
Addressed round-2 review feedback and CI failures in commit CI fixes
Review fixes
Tests run AI-generated comment on behalf of atiaomar1978-hub |
|
Replies to round-2 review summary items without inline threads: 1. CI build failing — Fixed across 4. Producer-level observability tests — Added 5. Observability on deprecated AI-generated comment on behalf of atiaomar1978-hub |
…e 1) Introduce camel-ai-observability module with OpenTelemetry GenAI span attributes and Micrometer metrics aligned with Spring AI semconv. Integrate per-LLM-call child spans and metrics into langchain4j-chat, langchain4j-tools, langchain4j-agent, langchain4j-embeddings, and openai producers. Add request/response model headers on langchain4j components. Global toggle via camel.ai.observability.enabled. Co-authored-by: Cursor <cursoragent@cursor.com>
Use CamelContext.hasService(Tracer.class) instead of registry lookup so GenAI observability works when camel-opentelemetry2 registers the tracer as a service. Add GenAiObservabilitySpanTest, fix component doc metadata, and clean up imports. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
- Resolve agent chat model from AbstractAgent configuration - Use CLIENT span kind and exchange header propagation extractor - Add observability wrapper for OpenAI streaming path Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
- Mark micrometer-core and langchain4j-core as optional in camel-ai-observability - Add @SInCE 4.22 to AbstractAgent.getChatModel() - Use langchain4j ModelProvider and defaultRequestParameters instead of fragile reflection - Use ChatResponse.modelName() for response model in langchain4j producers - Accumulate OpenAI streaming token usage via stream_options.include_usage Co-authored-by: Cursor <cursoragent@cursor.com>
Add missing others module doc symlinks and correct the upgrade guide xref to components:others:ai-observability.adoc so documentation validation and docs xref-check pass in CI. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
- Commit regenerated catalog and endpoint DSL files for model headers - Load Micrometer support reflectively so optional micrometer-core does not break consumers without Micrometer on the classpath - Omit RESPONSE_MODEL header/span attribute in agent and embeddings producers when the response model is unavailable from the result object - Add LangChain4jChatObservabilityTest for producer-level span/header coverage - Add agent test asserting RESPONSE_MODEL is omitted when unavailable Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
Use next@components:others: prefix so Antora resolves the new page from the pre-release components version during doc validation, matching how other new component docs are linked from the 4.22 upgrade guide. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
f12051a to
3fe1fb0
Compare
davsclaus
left a comment
There was a problem hiding this comment.
Claude Code on behalf of davsclaus
Thank you for iterating on the feedback — the ModelProvider-based resolution, streaming token capture, optional micrometer, and response model header fixes are all solid improvements.
A few issues remain before this can be merged:
Confirmed issues
-
[High]
camel-ai-observabilityis a mandatory compile dependency on all AI components. Every user oflangchain4j-chat,langchain4j-tools,langchain4j-agent,langchain4j-embeddings, andcamel-openaiwill now getcamel-ai-observability+camel-telemetrypulled in transitively, whether they want observability or not. In standard Camel architecture, telemetry is injected at the framework level (viacamel-opentelemetry2and route-level interceptors), not coupled into each component's producer. Consider makingcamel-ai-observabilityoptional or using SPI-based discovery. See inline comment. -
[High]
nav.adocentry is wrongly nested. The***(three-star) prefix nests "AI Observability" under "A2A - Producer Guide" instead of as a peer entry. This file is auto-generated — if the nesting comes from the:group: AIattribute, the template or doc mojo needs fixing. See inline comment. -
[Medium] Unrelated changes included in the PR. The diff includes a new
camel.jbang.mcpproperty incamel-jbang-configuration-metadata.json(doesn't exist onmain, unrelated to GenAI observability), and a test dependency change fromcamel-test-spring-junit6tocamel-test-junit6incamel-langchain4j-chat/pom.xml. These should be reverted or submitted separately. -
[Medium] Missing producer-level observability tests for 3 of 5 components.
LangChain4jChatObservabilityTestis well-structured — but equivalent tests are missing forlangchain4j-tools,langchain4j-embeddings, andopenaiproducers. Per project rules, every PR must include tests for new functionality. -
[Low] Unnecessary
constructor.setAccessible(true). See inline comment. -
[Low] Adding observability to deprecated
camel-langchain4j-tools. This component was deprecated in favor ofai-tool+langchain4j-agent. Adding new features to deprecated components increases maintenance cost.
Open questions
- CI is still pending — build results should be verified.
- The component-level observability approach (each producer calling
GenAiObservability.start()) is an architectural decision that may warrant discussion with other committers.
Note: This review covers project rules, conventions, and code-level issues visible from the diff. It does not replace specialized tools such as SonarCloud or CodeRabbit.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
- Add camel-ai-observability to camel-bom (bom-generator) - Remove stale camel.jbang.mcp from catalog jbang metadata - Apply formatter alignment on AbstractAgent @SInCE tag Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
Introduce camel-ai-observability-api with a reflection-based bridge so AI producers no longer pull camel-telemetry transitively. Move implementation into GenAiObservabilityImpl, restore camel-test-spring-junit6 alongside camel-test-junit6, fix others nav nesting, and add producer-level observability tests for tools, embeddings, and openai. Co-authored-by: Cursor <cursoragent@cursor.com>
davsclaus
left a comment
There was a problem hiding this comment.
Review — CAMEL-23861: GenAI observability for langchain4j and OpenAI (Phase 1)
Thank you for the contribution — this is a substantial and well-structured piece of work. The API/impl module split, consistent try/catch/finally pattern across producers, and test conventions (package-private, AssertJ, no Thread.sleep) are all well done.
I've identified several issues that should be addressed before merging, detailed as inline comments below.
General observations not tied to specific lines
1. Observability added to deprecated camel-langchain4j-tools — This component was deprecated (replaced by camel-ai-tool + langchain4j-agent). Adding new observability code to a deprecated component increases maintenance surface. Please reconsider whether this is intentional or can be dropped.
2. No caching of reflective resolution / metrics backend — Every GenAiObservability.start() call resolves classes via reflection, and resolveMetricsBackend() in GenAiObservabilityImpl constructs a new GenAiMicrometerSupport via reflection on every call. For agentic loops with multiple LLM iterations per exchange, this adds unnecessary overhead. Consider caching per CamelContext.
3. Silent error swallowing with no logging — The catch blocks in GenAiObservability.start() and GenAiObservabilityImpl.resolveMetricsBackend() silently discard all errors. A DEBUG/TRACE level log would aid troubleshooting when reflective loading fails unexpectedly.
4. long to int cast for OpenAI token counts — In OpenAIProducer, usage.promptTokens() returns long but is cast to (int) when creating GenAiUsage. Consider typing GenAiUsage.inputTokens/outputTokens as Long to avoid the lossy cast.
This review focuses on project rules, conventions, and code quality. It does not replace specialized review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.
- Avoid double resolveClass and narrow catch in GenAiObservability.start() - Fix error-path RESPONSE_MODEL span tag overwrite in GenAiObservabilityImpl - Gate OpenAI streaming includeUsage behind observability enabled flag - Clarify agent RESPONSE_MODEL header metadata and upgrade guide - Regenerate catalog ai-observability doc; keep jbang metadata unchanged Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Round-3 review feedback addressed (@davsclaus)AI-generated on behalf of atiaomar1978-hub via Cursor Cloud Agent. Pushed commit Code fixes
CI / generated files
Tests (BUILD SUCCESS)Ready for re-review. CI should re-run on this push. |
Sync LangChain4jAgentEndpointBuilderFactory with updated @metadata description on Headers.RESPONSE_MODEL from review feedback. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
CI fix — uncommitted endpoint DSLAI-generated on behalf of atiaomar1978-hub via Cursor Cloud Agent. CI failed the uncommitted changes check (run 31074465889) because Fixed in
CI should pass the generated-files check on the next run. |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 579 tested, 25 compile-only — current: 76 all testedMaveniverse Scalpel detected 604 affected modules (current approach: 76).
|
Description
Implements Phase 1 of CAMEL-23861 — GenAI observability for Camel AI route producers, aligned with Claus Ibsen's latest JIRA guidance:
camel.ai.observability.enabledAI-generated PR description on behalf of atiaomar1978-hub
What changed
New module:
camel-ai-observabilityShared library providing GenAI observability for Camel AI producers:
camel-telemetry.Tracer+ GenAI semantic convention attributes (gen_ai.operation.name,gen_ai.system,gen_ai.request.model, token usage, etc.)gen_ai.client.operationtimer andgen_ai.client.token.usagecounter (Spring AI–aligned metric names)camel.ai.observability.enabled(defaulttrue; no-op when disabled or no backend present)CamelContext.hasService(Tracer.class)socamel-opentelemetry2service registration is discovered correctlyCLIENTwith exchange header propagation extractor for parent trace linkingProducer integrations (Phase 1)
langchain4j-chatchatModel.chat()calllangchain4j-toolschatWithLLM()langchain4j-agentlangchain4j-embeddingsopenaicreateChatCompletion()and streaming pathNew exchange headers (langchain4j)
CamelLangChain4jChatRequestModel/CamelLangChain4jChatResponseModelCamelLangChain4jToolsRequestModel/CamelLangChain4jToolsResponseModelCamelLangChain4jAgentRequestModel/CamelLangChain4jAgentResponseModelCamelLangChain4jEmbeddingsRequestModel/CamelLangChain4jEmbeddingsResponseModelOpenAI already exposes
MODEL/RESPONSE_MODEL.Documentation
components/camel-ai/camel-ai-observability/src/main/docs/ai-observability.adocdocs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adocDesign notes
MeterRegistrybinding; Micrometer Observation API deferred to follow-upcamel ask/ TUI observability per Claus commentTests
GenAiModelResolverTest— provider/model resolutionGenAiObservabilityTest— global toggle, Micrometer metricsGenAiObservabilitySpanTest— OTel span attributes and error taggingGenAiObservationContextTest— span naming conventionmvn test -pl components/camel-ai/camel-ai-observability -am -DskipITsTarget
main(Camel 4.22.0-SNAPSHOT)Tracking
AI-assisted contributions