CAMEL-23460: camel-telemetry - add span decorators for Google Cloud (AI/ML & misc batch)#25071
CAMEL-23460: camel-telemetry - add span decorators for Google Cloud (AI/ML & misc batch)#25071oscerd wants to merge 1 commit into
Conversation
…AI/ML & misc batch) Second batch of Google Cloud span decorators, following batch 1 (apache#24974) and the AWS pattern (CAMEL-23387): - GoogleFunctionsSpanDecorator: operation, entryPoint, runtime - GoogleSecretManagerSpanDecorator: operation, secretId, versionId (the secret identifier/version only, never the secret value) - GoogleVertexAISpanDecorator: operation, modelId, location (the prompt and chat messages carry user content and are deliberately not emitted) - GoogleVisionSpanDecorator / GoogleSpeechToTextSpanDecorator / GoogleTextToSpeechSpanDecorator: operation only (the response object may carry large audio/image payloads and is not emitted) Header constants are mirrored from each component's Constants interface to avoid a hard dependency from camel-telemetry to the Google component modules. Decorators are registered alphabetically in the SpanDecorator service file and each has a unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 13 tested, 29 compile-only — current: 13 all testedMaveniverse Scalpel detected 42 affected modules (current approach: 13).
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for the second batch, @oscerd — nice, clean work continuing the pattern from batch 1.
Two minor convention notes (both low severity and consistent with the already-merged batch 1, so non-blocking):
- Test visibility: New test classes and methods should drop the
publicmodifier per project conventions (JUnit 5 doesn't require it). Applies to all 6 test files. - Assertion style: New tests should prefer AssertJ (
assertThat(...)) over JUnit assertions (assertEquals/assertNull) per project conventions. Applies to all 6 test files.
Suggestion (non-blocking): The VertexAI decorator currently tags operation, modelId, and location. GoogleVertexAIConstants also exposes PROMPT_TOKEN_COUNT, CANDIDATES_TOKEN_COUNT, TOTAL_TOKEN_COUNT, FINISH_REASON, and PUBLISHER — all lightweight scalar values commonly used in LLM observability dashboards for cost tracking and debugging. Worth considering as an enhancement (here or follow-up).
Everything else looks good — all header constants and component class names verified against the actual source, service file registration is correctly alphabetized, and the security-sensitive omissions (prompts, chat messages, response objects, secret values) are well-reasoned and documented.
This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.
Claude Code on behalf of Claus Ibsen (@davsclaus). This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| public class GoogleFunctionsSpanDecoratorTest { |
There was a problem hiding this comment.
Minor: per project conventions, new test classes should be package-private (drop public). Same applies to the testPre() method below. This is consistent with batch 1 so non-blocking, but worth aligning for new code.
| public class GoogleFunctionsSpanDecoratorTest { | |
| class GoogleFunctionsSpanDecoratorTest { |
| import org.mockito.Mockito; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNull; |
There was a problem hiding this comment.
Minor: per project conventions, new test code should use AssertJ assertions. For example:
import static org.assertj.core.api.Assertions.assertThat;
assertThat(span.tags().get(GoogleVertexAISpanDecorator.VERTEXAI_OPERATION)).isEqualTo(operation);
assertThat(span.tags()).doesNotContainKey("prompt");Applies to all 6 test files. Non-blocking since batch 1 uses the same pattern.
There was a problem hiding this comment.
assertThat(span.tags())
.containsEntry(entry(GoogleVertexAISpanDecorator.VERTEXAI_OPERATION, operation))
.doesNotContainKey("prompt");
3 participants
Motivation
CAMEL-23460, continuing from batch 1 (#24974, merged). This is the issue's AI/ML & misc batch of Google Cloud span decorators.
Changes
Six new decorators (all
AbstractSpanDecorator):GoogleFunctionsSpanDecoratorGoogleSecretManagerSpanDecoratorGoogleVertexAISpanDecoratorGoogleVisionSpanDecoratorGoogleSpeechToTextSpanDecoratorGoogleTextToSpeechSpanDecoratorTag minimization per the rules established in the AWS work — sensitive or bulky values are deliberately dropped:
promptandchatMessagesheaders carry user content and are not emitted (the unit test asserts they are never tagged); the tuning parameters (temperature/topP/…) are omitted as low-value.operation— theirRESPONSE_OBJECTheader can carry large audio/image payloads and is not emitted.Header constants are mirrored from each component's
*Constantsinterface (with a Javadoc@linkback to the source), socamel-telemetrygains no hard dependency on the Google component modules. Registered alphabetically inMETA-INF/services/org.apache.camel.telemetry.SpanDecorator, one unit test per decorator.The remaining batch — workspace/productivity (Calendar, Drive, Mail, Sheets) — needs a different treatment (Mail's headers are To/From/Cc/Subject, i.e. PII; Drive exposes no data headers) and will be a separate PR.
Testing
mvn clean installincomponents/camel-telemetry: all tests pass (6 new + existing).mvn clean install -DskipTestsfrom root: success, no stale generated files.Metadata/observability addition — main/4.22.0 only, no backport.
Claude Code on behalf of Andrea Cosentino (@oscerd).
🤖 Generated with Claude Code