-
Notifications
You must be signed in to change notification settings - Fork 69
feat(observability): add no-op implementation of tracing #4045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @diegomarquezp, 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 lays the groundwork for distributed tracing within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a no-op implementation for tracing, which is a great first step towards full observability. The new classes and interfaces for tracing are well-structured. I've found a few areas for improvement, mainly concerning code correctness in a test setup, and some opportunities to improve encapsulation and prepare for the full implementation by aligning more closely with the existing metrics implementation. My comments provide specific suggestions for these points.
java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java
Show resolved
Hide resolved
gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingRecorder.java
Outdated
Show resolved
Hide resolved
| @BetaApi | ||
| @InternalApi | ||
| public class TracingTracerFactory implements ApiTracerFactory { | ||
| protected TracingRecorder tracingRecorder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tracingRecorder field is protected and mutable. Since it's initialized in the constructor and not expected to be changed by subclasses, it should be private final to ensure immutability and proper encapsulation.
| protected TracingRecorder tracingRecorder; | |
| private final TracingRecorder tracingRecorder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blakeli0 https://github.com/googleapis/sdk-platform-java/pull/2403/files#diff-fdc1dfb7a2b23501ab66d8e37f0aedc5ca87489d845479734f583773bab0a51a shows that the metrics counterpart also has the recorder as protected. Do you know if we intend to have these classes extended at some point? I imagine a new metrics sdk may require a sub class.
gax-java/gax/src/main/java/com/google/api/gax/tracing/TracingTracerFactory.java
Show resolved
Hide resolved
…metryTracingRecorder.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…eapis/sdk-platform-java into observability/tracing-noop
|
|
|
WIP as there are pending design decisions for the feature flag. |



This PR introduces a basic implementation for tracing in GAX. This is a minimal no-op placeholder intended to establish the project's architecture before adding functional tracing logic.
Changes:
New Tracing Components:
TracingRecordermimic the structure of the metrics counterpart.Feature Flag Implementation:
TracingUtilsto manage theGOOGLE_CLOUD_ENABLE_TRACINGfeature flag. Tracing is disabled by default and can be enabled via an environment variable or system property.TracingTracerFactoryto respect this flag, returning a no-opBaseApiTracerwhen tracing is disabled.Note on Implementation:
The
newTracermethod inTracingTracerFactoryand the tracing components themselves are currently implemented as placeholders. This PR focuses on establishing the basic structure and configuration mechanisms; actual tracing logic and attribute propagation will be implemented in subsequent phases.How to enable tracing:
To enable the tracing infrastructure, set the following environment variable or system property to
true:GOOGLE_CLOUD_ENABLE_TRACING=true