Skip to content

chore(showcase): add tracing tests#1725

Open
quartzmo wants to merge 23 commits intogoogleapis:mainfrom
quartzmo:showcase-tracing-tests
Open

chore(showcase): add tracing tests#1725
quartzmo wants to merge 23 commits intogoogleapis:mainfrom
quartzmo:showcase-tracing-tests

Conversation

@quartzmo
Copy link
Copy Markdown
Member

This PR is based on @westarle 's work on westarle:gapic-generator-go:test-plan-go-tf1-logs. It adds the following:

  1. Upgraded dependencies in showcase/go.mod (specifically google.golang.org/api, cloud.google.com/go/auth, and otelgrpc). This was crucial because the older versions weren't correctly propagating telemetry attributes through the transitive chain.
  2. Updated trace_test.go to be less strict about unexpected attributes. The upgraded dependencies now automatically add gcp.grpc.resend_count and rpc.response.status_code in some cases where the tests previously disallowed them. I removed them from the unexpectedAttrs lists.
  3. Added a telemetry build tag to exclude these tests from make test.
  4. Added a new section to showcase/README.md explaining how the telemetry tests work, including the telemetry build tag, environment variables, and how to find the traces in Cloud Trace.

Verification

I successfully ran the new make test-telemetry target on my cloudtop using my dev project. The integration test passed in ~36 seconds.

I also visited Trace Explorer to verify that the spans were arriving as expected (I had to adjust the time range to "Last 1 hour" to see them). I filtered for the client spans wiht AttemptSequence and they looked as expected with the T4 telemetry attributes we were looking for. Here's an example of attributes from an error span:

cloud.account.id cloudtop-prod-us-west
cloud.availability_zone us-west4-a
cloud.platform gcp_compute_engine
cloud.provider gcp
cloud.region us-west4
error.type CLIENT_TIMEOUT
exception.type *status.Error
gcp.client.artifact github.com/googleapis/gapic-showcase/client
gcp.client.language go
gcp.client.repo googleapis/google-cloud-go
gcp.client.service showcase
gcp.client.version UNKNOWN
gcp.gce.instance.hostname myproject.us-west4-a.c.cloudtop-prod-us-west.internal
gcp.gce.instance.name myproject
gcp.grpc.resend_count 0
gcp.project_id myproject-testing
gcp.resource.destination.id //localhost:7469/sequences/2
host.id xxxx
host.name myproject
host.type projects/xxxxxxxx/machineTypes/e2-custom-24-98304
rpc.method google.showcase.v1beta1.SequenceService/AttemptSequence
rpc.response.status_code DEADLINE_EXCEEDED
rpc.system.name grpc
server.address 127.0.0.1
server.port 7469
service.name test-app
status.message context deadline exceeded
telemetry.sdk.language go
telemetry.sdk.name opentelemetry
telemetry.sdk.version 1.43.0
url.domain showcase.googleapis.com

This proves that the generated client is correctly delegating to the modern transport wrappers under the hood and emitting the necessary telemetry, without us needing to change the generator structure itself

@quartzmo quartzmo requested a review from a team as a code owner April 13, 2026 22:09
Comment thread showcase/trace_test.go Outdated
Comment thread showcase/cloud_trace_test.go Outdated
Comment thread showcase/trace_test.go Outdated
} else {
expectedName = "POST /v1beta1/{name=sequences/*}"
wantAttrs = map[string]any{
"error.type": "context.deadlineExceededError",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@westarle Iserror.type: context.deadlineExceededError an implementation bug? Should the raw client exception by replaced with CLIENT_TIMEOUT as it is in gRPC?

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.

this seems like a bug, could you file one please?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I verified via a local test that the correct mapping logic to CLIENT_TIMEOUT in auth/httptransport is executing. However, the outer otelhttp.NewTransport wrapper overwrites the error.type attribute with the raw Go error type after our custom transport returns.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread showcase/cloud_trace_test.go
Comment thread showcase/cloud_trace_test.go Outdated
Comment thread showcase/cloud_trace_test.go Outdated
Comment thread showcase/observability_fixture_test.go
Comment thread showcase/observability_fixture_test.go Outdated
Comment thread showcase/trace_test.go
Comment thread showcase/trace_test.go Outdated
} else {
expectedName = "POST /v1beta1/{name=sequences/*}"
wantAttrs = map[string]any{
"error.type": "context.deadlineExceededError",
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.

this seems like a bug, could you file one please?

Comment thread showcase/trace_test.go Outdated
@quartzmo quartzmo force-pushed the showcase-tracing-tests branch from aec17e2 to b6d0a91 Compare April 15, 2026 23:09
Comment thread showcase/cloud_trace_test.go Outdated
@westarle
Copy link
Copy Markdown
Contributor

Updated trace_test.go to be less strict about unexpected attributes. The upgraded dependencies now automatically add gcp.grpc.resend_count and rpc.response.status_code in some cases where the tests previously disallowed them. I removed them from the unexpectedAttrs lists.

In a followup, (1) I think we should assert on the "wrong" values. Also (2) I think with your new "constraints" concept, we should be able to assert on every attribute in the map and report any "new" attribute as a failure (so we remember to add an appropriate test.)

@quartzmo quartzmo force-pushed the showcase-tracing-tests branch from 234a8a6 to d727d5b Compare April 17, 2026 17:01
@quartzmo
Copy link
Copy Markdown
Member Author

(1) I think we should assert on the "wrong" values. Also (2) I think with your new "constraints" concept, we should be able to assert on every attribute in the map and report any "new" attribute as a failure (so we remember to add an appropriate test.)

Pushed fix for this in d727d5b

Comment thread showcase/trace_test.go Outdated
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.

3 participants