impl(o11y): introduce http.response.status_code#4161
impl(o11y): introduce http.response.status_code#4161diegomarquezp wants to merge 11 commits intomainfrom
http.response.status_code#4161Conversation
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 features by introducing detailed request status attributes for both HTTP and gRPC calls. It provides a mechanism to capture and attach specific status codes (like HTTP 200/404 or gRPC OK/NOT_FOUND) to tracing spans, offering a clearer picture of API call outcomes. This improvement will aid in more precise debugging and monitoring of service interactions by making status information readily available within the tracing data. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces observability attributes for request status, which is a valuable addition for monitoring. The changes involve adding new attribute constants, extending the TraceManager to allow adding attributes to existing spans, and implementing the logic to populate these status attributes at the end of a request attempt.
The implementation correctly handles different transports (gRPC and HTTP) and various outcomes (success, cancellation, different exceptions). Tests have been added to cover the new functionality.
However, I've found a critical issue where the new long status attributes would be dropped due to a missing type handler in toOtelAttributes, which violates the rule for conforming to OpenTelemetry Semantic Conventions for data types. I've also suggested a refactoring to improve the clarity and maintainability of the new populateStatusAttributes method, aligning with the preference for explicit null checks for readability in simple cases.
gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java
Show resolved
Hide resolved
gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java
Show resolved
Hide resolved
|
|
gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java
Outdated
Show resolved
Hide resolved
gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java
Outdated
Show resolved
Hide resolved
…into observability/tracing-attr/status-code # Conflicts: # gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTraceManager.java # gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java # gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceManager.java
…-attr/status-code
http.response.status_code





No description provided.