fix: use dynamic tracer name instead of hardcoded gax-java#12190
fix: use dynamic tracer name instead of hardcoded gax-java#12190diegomarquezp wants to merge 14 commits intomainfrom
Conversation
Refactored SpanTracerFactory to dynamically initialize tracers with metadata containing the artifact name and version when withContext is invoked. Handled the null-tracer fallback strategy by returning a BaseApiTracer when tracer is null. Updated unit and integration tests.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the observability of distributed tracing by ensuring that OpenTelemetry tracers accurately report the originating library's metadata, such as its artifact name and version. This change moves away from a generic, hardcoded tracer name to a dynamic one, which will greatly improve the precision of root-cause analysis and overall trace clarity in complex distributed systems. Additionally, it introduces a robust fallback for uninitialized tracers, improving client stability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracerFactory.java
Outdated
Show resolved
Hide resolved
...java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java
Outdated
Show resolved
Hide resolved
...java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java
Outdated
Show resolved
Hide resolved
diegomarquezp
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
diegomarquezp
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracerFactory.java
Show resolved
Hide resolved
| return new BaseApiTracerFactory(); | ||
| } | ||
| Tracer newTracer = openTelemetry.getTracer(metadata.artifactName(), metadata.version()); | ||
| if (newTracer == null) { |
There was a problem hiding this comment.
In what case this could return null? I think openTelemetry api in general is safe to use. But if we are not sure, it's always good to add a null check as well.
There was a problem hiding this comment.
The docs don't mention a possible null return. I think you are right. I removed this check.
| } | ||
| ApiTracerContext mergedContext = this.apiTracerContext.merge(context); | ||
| LibraryMetadata metadata = mergedContext.libraryMetadata(); | ||
| if (metadata == null || metadata.isEmpty()) { |
There was a problem hiding this comment.
In addition, I think we should skip if artifactName is null but version is populated. But it version is null but artifact name is not, maybe we should still create one?
| return new BaseApiTracerFactory(); | ||
| } | ||
| ApiTracerContext mergedContext = this.apiTracerContext.merge(context); | ||
| LibraryMetadata metadata = mergedContext.libraryMetadata(); |
There was a problem hiding this comment.
Since metadata comes from the Library level context, I think we can do the check before merging.
There was a problem hiding this comment.
Done. Checking on original context.
blakeli0
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
diegomarquezp
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
4 participants
Description
This PR updates
SpanTracerFactoryto dynamically initialize OpenTelemetry tracers using artifact metadata (artifactNameandversion) rather than defaulting to a hardcoded"gax-java"instrumentation scope.Previously, traces initiated by downstream libraries (like
gapic-showcase) would incorrectly report their scope asgax-java. This update ensures that the tracer name accurately reflects the invoking context (e.g.,com.google.cloud:gapic-showcase), significantly improving observability and root-cause analysis in distributed tracing environments.Key Changes
SpanTracerFactoryto construct the OpenTelemetry metric tracer lazily fromApiTracerContextmetadata whenwithContextis invoked.BaseApiTracer(no-op) when the active tracer hasn't been properly initialized, avoidingNullPointerExceptions during client configuration.SpanTracerFactoryTestto mockOpenTelemetrytesting the null-tracer degradation logic.ITOtelTracinginjava-showcaseto assert that spans log the exact metadata injected fromLibraryMetadata.