Skip to content

feat(gax): add o11y integration tests - #9415

Draft
shivanee-p wants to merge 9 commits into
mainfrom
shivaneep-t3-integration-testing
Draft

shivanee-p wants to merge 9 commits into
mainfrom
shivaneep-t3-integration-testing

Conversation

@shivanee-p

@shivanee-p shivanee-p commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Adds end-to-end OpenTelemetry integration tests for GAX T3 tracing using the GAPIC Showcase mock server within test-application

  • adds Otel Integration test suite in telemetry-test.ts
  • Creates ShowcaseOtelHarness to manage tracer providers and span exporters
  • covers unary RPCs and streaming RPCs
  • verifies that T3 child spans are correctly nested within parent T4 trace spans
  • tests feature gating with environmental variables
  • update linter to handle local tarballs

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 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.

Comment thread core/packages/gax/test/test-application/package.json
Comment thread core/packages/gax/test/test-application/src/telemetry-test.ts
Comment thread core/packages/gax/test/test-application/src/telemetry-test.ts
Comment thread core/packages/gax/package.json Outdated
Comment thread core/packages/gax/test/test-application/src/telemetry-test.ts
Comment thread core/packages/gax/test/test-application/src/telemetry-test.ts
@shivanee-p
shivanee-p force-pushed the shivaneep-t3-integration-testing branch from b03ee7b to 6007cda Compare September 22, 2026 21:36
@shivanee-p

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 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.

Comment thread bin/linter.mjs Outdated

for (let i = 0; i < statusCodeList.length; i++) {
const delay = new protos.google.protobuf.Duration();
delay.seconds = delayList[i];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
delay.seconds = delayList[i];
const seconds = Math.floor(delayList[i]);
const nanos = Math.round((delayList[i] - seconds) * 1e9);
delay.seconds = seconds;
delay.nanos = nanos;

Comment thread core/packages/gax/test/test-application/src/telemetry-test.ts Outdated
@shivanee-p
shivanee-p marked this pull request as ready for review September 22, 2026 22:05
@shivanee-p
shivanee-p requested review from a team as code owners September 22, 2026 22:05
@github-actions
github-actions Bot requested a review from feywind September 22, 2026 22:15
@shivanee-p
shivanee-p removed request for a team September 22, 2026 22:25
@shivanee-p
shivanee-p marked this pull request as draft September 22, 2026 23:14

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant