feat(gax): add o11y integration tests - #9415
shivanee-p wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive integration tests for OpenTelemetry tracing (T3/T4) across unary, streaming, and retry scenarios using both gRPC and HTTP/REST fallback transports. The feedback highlights several critical improvements: adding the missing @opentelemetry/context-async-hooks dependency to package.json, registering the span processor via addSpanProcessor since passing it to the provider constructor is unsupported, setting telemetry environment variables at the application entry point before module imports are evaluated, aligning OpenTelemetry package versions, and using the Status enum instead of magic string comparisons for gRPC status codes.
…entage of T3 spans
b03ee7b to
6007cda
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces telemetry tracing integration tests using OpenTelemetry for both gRPC and HTTP/REST fallback transports, updating the showcase client and test application to support and validate trace context propagation, hierarchy, and environment gating. Additionally, the linter script is updated to skip dependency installation for packages requiring local tarballs. Feedback on these changes highlights a critical bug in the linter where a return statement prematurely exits a loop instead of continuing, a serialization issue in the telemetry tests where fractional seconds are assigned to an integer field, and a redundant type cast that can be simplified.
|
|
||
| for (let i = 0; i < statusCodeList.length; i++) { | ||
| const delay = new protos.google.protobuf.Duration(); | ||
| delay.seconds = delayList[i]; |
There was a problem hiding this comment.
In protobuf Duration, the seconds field is an int64 (integer). Setting delay.seconds = delayList[i] where delayList[i] is a fractional number (like 0.05) will result in the fractional part being truncated to 0 during serialization. This is why the tests had to assert a minimum duration of only 1 ms instead of the expected 50 ms. To correctly represent fractional seconds, split the value into seconds and nanos.
| delay.seconds = delayList[i]; | |
| const seconds = Math.floor(delayList[i]); | |
| const nanos = Math.round((delayList[i] - seconds) * 1e9); | |
| delay.seconds = seconds; | |
| delay.nanos = nanos; |
Adds end-to-end OpenTelemetry integration tests for GAX T3 tracing using the GAPIC Showcase mock server within test-application