feat(gax): implement active scope management and attribute validation for T3 attempt spans - #13977
feat(gax): implement active scope management and attribute validation for T3 attempt spans#13977jinseopkim0 wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry context scope management to OpenTelemetryTracingTracer by storing the active scope during an attempt and closing it upon completion, as well as setting the span status to error when an attempt fails. However, the review highlights a critical thread-safety issue: storing the thread-local Scope in an instance variable and closing it asynchronously across different threads (e.g., from the application thread to the executor thread) can lead to context leaks and corruption. Additionally, the new unit tests execute synchronously on a single thread, which masks this multi-threading issue.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces error status tracking on OpenTelemetry spans during failed attempts and adds a corresponding unit test. However, the newly declared attemptScope field is never initialized or used. Feedback recommends implementing the scope activation, ensuring that attemptScope is closed in an exception-safe manner to prevent context leaks, and adding unit tests to verify proper scope management.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates OpenTelemetryTracingTracer to set the span status to StatusCode.ERROR when an attempt fails, and adds a corresponding unit test. It also introduces an unused attemptScope field. The review feedback highlights that attemptScope is declared but never initialized or closed, which could lead to context leaks. Additionally, a null check is recommended for attemptSpan before calling methods on it in recordErrorAndEndAttempt to prevent potential NullPointerExceptions.
aa33fec to
4a8d3f4
Compare
|
|
1 participant
Description
Implements active scope context propagation and attribute validation for individual RPC attempt spans (T3 spans) in
OpenTelemetryTracingTracer.Changes
ScopeduringattemptStarted(...)viaotelContext.makeCurrent().attemptScopeinendAttempt().StatusCode.ERRORonattemptSpanwhen an attempt fails inrecordErrorAndEndAttempt.OpenTelemetryTracingTracerTestverifying scope management and error status propagation.b/541322523