From 9186633a2d51e5ef627865b609de2c8a67bd4f6d Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 4 Feb 2026 12:07:08 -0700 Subject: [PATCH 01/18] feat(auth): add OpenTelemetry gRPC and HTTP wrappers for T4 tracing Enable grpctransport and httptransport to inject static attributes, capture dynamic attributes from context, and enforce error semantics. refs: googleapis/gax-go#472 refs: googleapis/gapic-generator-go#1706 --- auth/go.mod | 3 +- auth/go.sum | 10 +- auth/grpctransport/grpctransport.go | 82 ++++- auth/grpctransport/grpctransport_otel_test.go | 295 ++++++++++++++---- auth/httptransport/httptransport_otel_test.go | 244 ++++++++++++++- auth/httptransport/transport.go | 70 ++++- 6 files changed, 640 insertions(+), 64 deletions(-) diff --git a/auth/go.mod b/auth/go.mod index 4b47ee4eb2ae..adb78beead63 100644 --- a/auth/go.mod +++ b/auth/go.mod @@ -28,9 +28,10 @@ require ( go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/otel/metric v1.40.0 // indirect golang.org/x/crypto v0.47.0 // indirect - golang.org/x/oauth2 v0.32.0 // indirect + golang.org/x/oauth2 v0.34.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect + google.golang.org/api v0.264.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect ) diff --git a/auth/go.sum b/auth/go.sum index f60f17b3308c..83fe9814f8f0 100644 --- a/auth/go.sum +++ b/auth/go.sum @@ -58,8 +58,8 @@ golang.org/x/crypto v0.47.0 h1:V6e3FRj+n4dbpw86FJ8Fv7XVOql7TEwpHapKoMJ/GO8= golang.org/x/crypto v0.47.0/go.mod h1:ff3Y9VzzKbwSSEzWqJsJVBnWmRwRSHt/6Op5n9bQc4A= golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/oauth2 v0.32.0 h1:jsCblLleRMDrxMN29H3z/k1KliIvpLgCkE6R8FXXNgY= -golang.org/x/oauth2 v0.32.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= +golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= @@ -70,6 +70,12 @@ golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= +google.golang.org/api v0.264.0 h1:+Fo3DQXBK8gLdf8rFZ3uLu39JpOnhvzJrLMQSoSYZJM= +google.golang.org/api v0.264.0/go.mod h1:fAU1xtNNisHgOF5JooAs8rRaTkl2rT3uaoNGo9NS3R8= +google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 h1:VQZ/yAbAtjkHgH80teYd2em3xtIkkHd7ZhqfH2N9CsM= +google.golang.org/genproto v0.0.0-20260128011058-8636f8732409/go.mod h1:rxKD3IEILWEu3P44seeNOAwZN4SaoKaQ/2eTg4mM6EM= +google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409 h1:merA0rdPeUV3YIIfHHcH4qBkiQAc1nfCKSI7lB4cV2M= +google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409/go.mod h1:fl8J1IvUjCilwZzQowmw2b7HQB2eAuYBabMXzWurF+I= google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 h1:H86B94AW+VfJWDqFeEbBPhEtHzJwJfTbgE2lZa54ZAQ= google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 9b4b1f06e889..eaa00379e8b1 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -24,17 +24,26 @@ import ( "log/slog" "net/http" "os" + "strconv" + "strings" "cloud.google.com/go/auth" "cloud.google.com/go/auth/credentials" "cloud.google.com/go/auth/internal" "cloud.google.com/go/auth/internal/transport" "cloud.google.com/go/auth/internal/transport/headers" + "github.com/googleapis/gax-go/v2" + "github.com/googleapis/gax-go/v2/callctx" "github.com/googleapis/gax-go/v2/internallog" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" grpccreds "google.golang.org/grpc/credentials" grpcinsecure "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/stats" + "google.golang.org/grpc/status" ) const ( @@ -198,7 +207,7 @@ type InternalOptions struct { // service. DefaultScopes []string // SkipValidation bypasses validation on Options. It should only be used - // internally for clients that needs more control over their transport. + // internally for clients that need more control over their transport. SkipValidation bool // TelemetryAttributes specifies a map of telemetry attributes to be added // to all OpenTelemetry signals, such as tracing and metrics, for purposes @@ -430,5 +439,74 @@ func addOpenTelemetryStatsHandler(dialOpts []grpc.DialOption, opts *Options) []g if opts.DisableTelemetry { return dialOpts } - return append(dialOpts, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) + if !gax.IsFeatureEnabled("TRACING") { + return append(dialOpts, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) + } + var attrs []attribute.KeyValue + if opts.InternalOptions != nil { + for k, v := range opts.InternalOptions.TelemetryAttributes { + attrs = append(attrs, attribute.String(k, v)) + } + } + otelOpts := []otelgrpc.Option{ + otelgrpc.WithSpanAttributes(attrs...), + } + return append(dialOpts, grpc.WithStatsHandler(&otelHandler{ + Handler: otelgrpc.NewClientHandler(otelOpts...), + })) +} + +// otelHandler is a wrapper around the OpenTelemetry gRPC client handler that +// adds custom Google Cloud-specific attributes to spans and metrics. +type otelHandler struct { + stats.Handler +} + +// TagRPC intercepts the RPC start to extract dynamic attributes like resource +// name and retry count from the outgoing context metadata and attach them to +// the current span. +func (h *otelHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context { + ctx = h.Handler.TagRPC(ctx, info) + span := trace.SpanFromContext(ctx) + if !span.IsRecording() { + return ctx + } + if resName, ok := callctx.TelemetryFromContext(ctx, "resource_name"); ok { + span.SetAttributes(attribute.String("gcp.resource.name", resName)) + } + if resendCountStr, ok := callctx.TelemetryFromContext(ctx, "resend_count"); ok { + if count, err := strconv.Atoi(resendCountStr); err == nil { + span.SetAttributes(attribute.Int("gcp.grpc.resend_count", count)) + } + } + return ctx +} + +// HandleRPC intercepts the RPC completion to capture and format error-related +// attributes ensuring they conform to Google Cloud observability standards. +func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { + end, ok := s.(*stats.End) + if !ok || end.Error == nil { + h.Handler.HandleRPC(ctx, s) + return + } + span := trace.SpanFromContext(ctx) + if !span.IsRecording() { + h.Handler.HandleRPC(ctx, s) + return + } + // Extract status code and message + st, _ := status.FromError(end.Error) + span.SetAttributes( + attribute.String("error.type", strings.ToUpper(st.Code().String())), + attribute.String("status.message", st.Message()), + attribute.String("grpc.status", strings.ToUpper(st.Code().String())), + attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), + ) + if md, ok := metadata.FromOutgoingContext(ctx); ok { + if v := md[":authority"]; len(v) > 0 { + span.SetAttributes(attribute.String("url.domain", v[0])) + } + } + h.Handler.HandleRPC(ctx, s) } diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index b9f8311a57cd..8cf6d3f446bf 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -21,11 +21,14 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/googleapis/gax-go/v2" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + + "github.com/googleapis/gax-go/v2/callctx" oteltrace "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" @@ -48,7 +51,11 @@ const ( valLocalhost = "127.0.0.1" ) -func TestDial_OpenTelemetry(t *testing.T) { +func TestDial_OpenTelemetry_Enabled(t *testing.T) { + gax.TestOnlyResetIsFeatureEnabled() + defer gax.TestOnlyResetIsFeatureEnabled() + t.Setenv("GOOGLE_SDK_GO_EXPERIMENTAL_TRACING", "true") + // Ensure any lingering HTTP/2 connections are closed to avoid goroutine leaks. defer http.DefaultTransport.(*http.Transport).CloseIdleConnections() @@ -92,22 +99,10 @@ func TestDial_OpenTelemetry(t *testing.T) { Code: codes.Unset, }, Attributes: []attribute.KeyValue{ - // Note on Events (Logs): - // The otelgrpc instrumentation also records "message" events (Sent/Received) - // containing message sizes (compressed/uncompressed). These appear in the - // "Logs" or "Events" tab in Cloud Trace. This test does not explicitly verify - // them, but they are present in the generated span. - - // In Cloud Trace, this status code maps to the visual "Status" field - // (e.g., a green checkmark for 0/OK, or an error icon for other codes). keyRPCStatusCode.Int64(0), - // In Cloud Trace, "rpc.service" and "rpc.method" are combined to form - // the Span Name (e.g., "echo.Echoer/Echo"). keyRPCMethod.String("Echo"), keyRPCService.String("echo.Echoer"), - // "rpc.system" is displayed as a standard attribute key in the "Attributes" tab. keyRPCSystem.String(valRPCSystemGRPC), - // "server.address" and "server.port" are displayed as standard attribute keys. keyServerAddr.String(valLocalhost), }, }.Snapshot(), @@ -127,19 +122,14 @@ func TestDial_OpenTelemetry(t *testing.T) { Description: "test error", }, Attributes: []attribute.KeyValue{ - // Note on Events (Logs): - // The otelgrpc instrumentation also records "message" events (Sent/Received) - // containing message sizes (compressed/uncompressed). These appear in the - // "Logs" or "Events" tab in Cloud Trace. This test does not explicitly verify - // them, but they are present in the generated span. - - // In Cloud Trace, non-zero status codes (like 13 for INTERNAL) are displayed - // as errors in the "Status" field. keyRPCStatusCode.Int64(13), keyRPCMethod.String("Echo"), keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), + attribute.String("error.type", "INTERNAL"), + attribute.String("status.message", "test error"), + attribute.String("grpc.status", "INTERNAL"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, @@ -153,6 +143,36 @@ func TestDial_OpenTelemetry(t *testing.T) { }, wantSpans: 0, }, + { + name: "telemetry enabled metadata enrichment", + echoer: successfulEchoer, + opts: &Options{ + DisableAuthentication: true, + InternalOptions: &InternalOptions{ + TelemetryAttributes: map[string]string{ + "gcp.client.version": "1.0.0", + }, + }, + }, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Unset, + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(0), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + attribute.String("gcp.resource.name", "my-resource"), + attribute.String("gcp.client.version", "1.0.0"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, } for _, tt := range tests { @@ -170,14 +190,20 @@ func TestDial_OpenTelemetry(t *testing.T) { tt.opts.Endpoint = l.Addr().String() tt.opts.GRPCDialOpts = []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + pool, err := Dial(context.Background(), false, tt.opts) if err != nil { t.Fatalf("Dial() = %v, want nil", err) } defer pool.Close() + ctx := context.Background() + if tt.name == "telemetry enabled metadata enrichment" { + ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + } + client := echo.NewEchoerClient(pool) - _, err = client.Echo(context.Background(), &echo.EchoRequest{Message: "hello"}) + _, err = client.Echo(ctx, &echo.EchoRequest{Message: "hello"}) if (err != nil) != tt.wantErr { t.Errorf("client.Echo() error = %v, wantErr %v", err, tt.wantErr) } @@ -192,8 +218,6 @@ func TestDial_OpenTelemetry(t *testing.T) { if diff := cmp.Diff(tt.wantSpan.Name(), span.Name); diff != "" { t.Errorf("span.Name mismatch (-want +got):\n%s", diff) } - // In Cloud Trace, SpanKind "Client" identifies this as an outgoing request, - // often affecting the icon used in the trace visualization. if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) } @@ -201,35 +225,6 @@ func TestDial_OpenTelemetry(t *testing.T) { t.Errorf("span.Status mismatch (-want +got):\n%s", diff) } - // Note: Real-world spans in Cloud Trace will contain additional attributes - // that are not present in this unit test. - // - // 1. Resource Attributes: - // - "g.co/r/generic_node/location" (e.g., "global") - // - "g.co/r/generic_node/namespace" - // - "g.co/r/generic_node/node_id" - // - "service.name" (e.g., "my-application") - // - "telemetry.sdk.language" (e.g., "go") - // - "telemetry.sdk.name" (e.g., "opentelemetry") - // - "telemetry.sdk.version" (e.g., "1.20.0") - // These are defined by the TracerProvider's Resource configuration. This test uses - // a basic TracerProvider, so these attributes contain default values (e.g., - // service.name="unknown_service:grpctransport.test") rather than production values. - // - // 2. Instrumentation Scope: - // - "otel.scope.name" (e.g., "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc") - // - "otel.scope.version" (e.g., "0.46.0") - // These identify the instrumentation library itself and are part of the - // OpenTelemetry data model, separate from Span attributes. - // - // 3. Exporter Attributes: - // - "g.co/agent" (e.g., "opentelemetry-go 1.20.0; google-cloud-trace-exporter 1.20.0") - // These are injected by specific exporters (like the Google Cloud Trace exporter) - // and are not present when using the InMemoryExporter. - // - // This test focuses on verifying the "rpc.*" and "server.*" attributes, which are - // generated by the otelgrpc instrumentation library itself. - gotAttrs := map[attribute.Key]attribute.Value{} for _, attr := range span.Attributes { gotAttrs[attr.Key] = attr.Value @@ -253,3 +248,197 @@ func TestDial_OpenTelemetry(t *testing.T) { }) } } + +func TestDial_OpenTelemetry_Disabled(t *testing.T) { + gax.TestOnlyResetIsFeatureEnabled() + defer gax.TestOnlyResetIsFeatureEnabled() + t.Setenv("GOOGLE_SDK_GO_EXPERIMENTAL_TRACING", "false") + + // Ensure any lingering HTTP/2 connections are closed to avoid goroutine leaks. + defer http.DefaultTransport.(*http.Transport).CloseIdleConnections() + + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + defer tp.Shutdown(context.Background()) + + // Restore the global tracer provider after the test to avoid side effects. + defer func(prev oteltrace.TracerProvider) { otel.SetTracerProvider(prev) }(otel.GetTracerProvider()) + otel.SetTracerProvider(tp) + + successfulEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + return &echo.EchoReply{Message: req.Message}, nil + }, + } + errorEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + return nil, status.Error(grpccodes.Internal, "test error") + }, + } + + tests := []struct { + name string + echoer echo.EchoerServer + opts *Options + wantErr bool + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key + }{ + { + name: "telemetry enabled success (but gated off)", + echoer: successfulEchoer, + opts: &Options{DisableAuthentication: true}, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Unset, + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(0), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, + { + name: "telemetry enabled error (but gated off)", + echoer: errorEchoer, + opts: &Options{DisableAuthentication: true}, + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "test error", + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(13), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + // Standard OTel attributes only, NO strict error.type/status.message/grpc.status + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, + { + name: "telemetry disabled", + echoer: successfulEchoer, + opts: &Options{ + DisableAuthentication: true, + DisableTelemetry: true, + }, + wantSpans: 0, + }, + { + name: "telemetry enabled metadata enrichment (but gated off)", + echoer: successfulEchoer, + opts: &Options{ + DisableAuthentication: true, + InternalOptions: &InternalOptions{ + TelemetryAttributes: map[string]string{ + "gcp.client.version": "1.0.0", + }, + }, + }, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Unset, + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(0), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + // NO gcp.* attributes + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exporter.Reset() + + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("Failed to listen: %v", err) + } + s := grpc.NewServer() + echo.RegisterEchoerServer(s, tt.echoer) + go s.Serve(l) + defer s.Stop() + + tt.opts.Endpoint = l.Addr().String() + tt.opts.GRPCDialOpts = []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + + pool, err := Dial(context.Background(), false, tt.opts) + if err != nil { + t.Fatalf("Dial() = %v, want nil", err) + } + defer pool.Close() + + ctx := context.Background() + if tt.name == "telemetry enabled metadata enrichment (but gated off)" { + ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + } + + client := echo.NewEchoerClient(pool) + _, err = client.Echo(ctx, &echo.EchoRequest{Message: "hello"}) + if (err != nil) != tt.wantErr { + t.Errorf("client.Echo() error = %v, wantErr %v", err, tt.wantErr) + } + + spans := exporter.GetSpans() + if len(spans) != tt.wantSpans { + t.Fatalf("len(spans) = %d, want %d", len(spans), tt.wantSpans) + } + + if tt.wantSpans > 0 { + span := exporter.GetSpans()[0] + if diff := cmp.Diff(tt.wantSpan.Name(), span.Name); diff != "" { + t.Errorf("span.Name mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { + t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + t.Errorf("span.Status mismatch (-want +got):\n%s", diff) + } + + gotAttrs := map[attribute.Key]attribute.Value{} + for _, attr := range span.Attributes { + gotAttrs[attr.Key] = attr.Value + } + for _, wantAttr := range tt.wantSpan.Attributes() { + if gotVal, ok := gotAttrs[wantAttr.Key]; !ok { + t.Errorf("missing attribute: %s", wantAttr.Key) + } else { + if diff := cmp.Diff(wantAttr.Value, gotVal, cmp.AllowUnexported(attribute.Value{})); diff != "" { + t.Errorf("attribute %s mismatch (-want +got):\n%s", wantAttr.Key, diff) + } + } + } + for _, wantKey := range tt.wantAttrKeys { + if _, ok := gotAttrs[wantKey]; !ok { + t.Errorf("missing attribute key: %s", wantKey) + } + } + } + }) + } +} diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 06cd219ba15a..41ce0fcfe75f 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -21,11 +21,14 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/googleapis/gax-go/v2" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + + "github.com/googleapis/gax-go/v2/callctx" oteltrace "go.opentelemetry.io/otel/trace" ) @@ -43,7 +46,11 @@ const ( valLocalhost = "127.0.0.1" ) -func TestNewClient_OpenTelemetry(t *testing.T) { +func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { + gax.TestOnlyResetIsFeatureEnabled() + defer gax.TestOnlyResetIsFeatureEnabled() + t.Setenv("GOOGLE_SDK_GO_EXPERIMENTAL_TRACING", "true") + defer http.DefaultTransport.(*http.Transport).CloseIdleConnections() exporter := tracetest.NewInMemoryExporter() @@ -83,9 +90,10 @@ func TestNewClient_OpenTelemetry(t *testing.T) { // "server.address", "server.port", and "url.full" are displayed as // standard attribute keys in the "Attributes" tab. keyServerAddr.String(valLocalhost), + attribute.String("rpc.system", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, }, { name: "telemetry enabled error", @@ -105,10 +113,12 @@ func TestNewClient_OpenTelemetry(t *testing.T) { keyHTTPResponseStatus.Int(500), keyNetProtoVersion.String(valHTTP11), keyServerAddr.String(valLocalhost), - keyErrorType.String("500"), // otelhttp adds this on error + keyErrorType.String("500"), + attribute.String("status.message", "500 Internal Server Error"), + attribute.String("rpc.system", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, }, { name: "telemetry disabled", @@ -119,6 +129,50 @@ func TestNewClient_OpenTelemetry(t *testing.T) { statusCode: http.StatusOK, wantSpans: 0, }, + { + name: "telemetry enabled metadata enrichment", + opts: &Options{ + DisableAuthentication: true, + InternalOptions: &InternalOptions{ + TelemetryAttributes: map[string]string{ + "gcp.client.version": "1.0.0", + }, + }, + }, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + attribute.String("gcp.resource.name", "my-resource"), + attribute.String("gcp.client.version", "1.0.0"), + attribute.String("rpc.system", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + }, + { + name: "telemetry enabled url template", + opts: &Options{ + DisableAuthentication: true, + }, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "GET /my/template", + SpanKind: oteltrace.SpanKindClient, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + attribute.String("url.template", "/my/template"), + attribute.String("rpc.system", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + }, } for _, tt := range tests { @@ -136,7 +190,15 @@ func TestNewClient_OpenTelemetry(t *testing.T) { t.Fatalf("NewClient() = %v, want nil", err) } - req, err := http.NewRequest("GET", server.URL, nil) + ctx := context.Background() + if tt.name == "telemetry enabled metadata enrichment" { + ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + } + if tt.name == "telemetry enabled url template" { + ctx = callctx.WithTelemetryContext(ctx, "url_template", "/my/template") + } + + req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) if err != nil { t.Fatalf("http.NewRequest() = %v, want nil", err) } @@ -215,3 +277,175 @@ func TestNewClient_OpenTelemetry(t *testing.T) { }) } } + +func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { + gax.TestOnlyResetIsFeatureEnabled() + defer gax.TestOnlyResetIsFeatureEnabled() + t.Setenv("GOOGLE_SDK_GO_EXPERIMENTAL_TRACING", "false") + + defer http.DefaultTransport.(*http.Transport).CloseIdleConnections() + + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + defer tp.Shutdown(context.Background()) + + // Restore the global tracer provider after the test to avoid side effects. + defer func(prev oteltrace.TracerProvider) { otel.SetTracerProvider(prev) }(otel.GetTracerProvider()) + otel.SetTracerProvider(tp) + + tests := []struct { + name string + opts *Options + statusCode int + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key + }{ + { + name: "telemetry enabled success (but gated off)", + opts: &Options{DisableAuthentication: true}, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Unset, + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + keyNetProtoVersion.String(valHTTP11), + keyServerAddr.String(valLocalhost), + // NO rpc.system + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, // NO url.domain + }, + { + name: "telemetry enabled error (but gated off)", + opts: &Options{DisableAuthentication: true}, + statusCode: http.StatusInternalServerError, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "", + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(500), + keyNetProtoVersion.String(valHTTP11), + keyServerAddr.String(valLocalhost), + // NO rpc.system, status.message, error.type + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, // NO url.domain + }, + { + name: "telemetry disabled", + opts: &Options{ + DisableAuthentication: true, + DisableTelemetry: true, + }, + statusCode: http.StatusOK, + wantSpans: 0, + }, + { + name: "telemetry enabled metadata enrichment (but gated off)", + opts: &Options{ + DisableAuthentication: true, + InternalOptions: &InternalOptions{ + TelemetryAttributes: map[string]string{ + "gcp.client.version": "1.0.0", + }, + }, + }, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + // NO gcp.* attributes, NO rpc.system + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr}, // NO url.domain + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exporter.Reset() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.statusCode) + })) + defer server.Close() + + tt.opts.Endpoint = server.URL + client, err := NewClient(tt.opts) + if err != nil { + t.Fatalf("NewClient() = %v, want nil", err) + } + + ctx := context.Background() + if tt.name == "telemetry enabled metadata enrichment (but gated off)" { + ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + } + + req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) + if err != nil { + t.Fatalf("http.NewRequest() = %v, want nil", err) + } + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("client.Do() = %v, want nil", err) + } + resp.Body.Close() + + spans := exporter.GetSpans() + if len(spans) != tt.wantSpans { + t.Fatalf("len(spans) = %d, want %d", len(spans), tt.wantSpans) + } + + if tt.wantSpans > 0 { + span := exporter.GetSpans()[0] + if diff := cmp.Diff(tt.wantSpan.Name(), span.Name); diff != "" { + t.Errorf("span.Name mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { + t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + t.Errorf("span.Status mismatch (-want +got):\n%s", diff) + } + + gotAttrs := map[attribute.Key]attribute.Value{} + for _, attr := range span.Attributes { + gotAttrs[attr.Key] = attr.Value + } + for _, wantAttr := range tt.wantSpan.Attributes() { + if gotVal, ok := gotAttrs[wantAttr.Key]; !ok { + t.Errorf("missing attribute: %s", wantAttr.Key) + } else { + // Use simple value comparison for non-dynamic fields + if diff := cmp.Diff(wantAttr.Value, gotVal, cmp.AllowUnexported(attribute.Value{})); diff != "" { + t.Errorf("attribute %s mismatch (-want +got):\n%s", wantAttr.Key, diff) + } + } + } + for _, wantKey := range tt.wantAttrKeys { + if _, ok := gotAttrs[wantKey]; !ok { + t.Errorf("missing attribute key: %s", wantKey) + } + } + } + }) + } +} diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 3feb997c76d4..1f3efd5006e1 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -17,9 +17,11 @@ package httptransport import ( "context" "crypto/tls" + "fmt" "net" "net/http" "os" + "strconv" "time" "cloud.google.com/go/auth" @@ -28,7 +30,11 @@ import ( "cloud.google.com/go/auth/internal/transport" "cloud.google.com/go/auth/internal/transport/cert" "cloud.google.com/go/auth/internal/transport/headers" + "github.com/googleapis/gax-go/v2" + "github.com/googleapis/gax-go/v2/callctx" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "golang.org/x/net/http2" ) @@ -173,7 +179,69 @@ func addOpenTelemetryTransport(trans http.RoundTripper, opts *Options) http.Roun if opts.DisableTelemetry { return trans } - return otelhttp.NewTransport(trans) + if !gax.IsFeatureEnabled("TRACING") { + return otelhttp.NewTransport(trans) + } + var staticAttrs []attribute.KeyValue + if opts.InternalOptions != nil { + for k, v := range opts.InternalOptions.TelemetryAttributes { + staticAttrs = append(staticAttrs, attribute.String(k, v)) + } + } + return otelhttp.NewTransport(&otelAttributeTransport{ + base: trans, + staticAttrs: staticAttrs, + }) +} + +// otelAttributeTransport is a wrapper around an http.RoundTripper that adds +// custom Google Cloud-specific attributes to OpenTelemetry spans. +type otelAttributeTransport struct { + base http.RoundTripper + staticAttrs []attribute.KeyValue +} + +// RoundTrip intercepts the HTTP request and response to enrich the active +// OpenTelemetry span with static and dynamic attributes, as well as detailed +// error information. +func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, error) { + span := trace.SpanFromContext(req.Context()) + if span.IsRecording() { + span.SetAttributes(t.staticAttrs...) + span.SetAttributes(attribute.String("rpc.system", "http")) + span.SetAttributes(attribute.String("url.domain", req.URL.Host)) + if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { + span.SetAttributes(attribute.String("gcp.resource.name", resName)) + } + if resendCountStr, ok := callctx.TelemetryFromContext(req.Context(), "resend_count"); ok { + if count, err := strconv.Atoi(resendCountStr); err == nil { + span.SetAttributes(attribute.Int("gcp.grpc.resend_count", count)) + } + } + if urlTemplate, ok := callctx.TelemetryFromContext(req.Context(), "url_template"); ok { + span.SetAttributes(attribute.String("url.template", urlTemplate)) + span.SetName(fmt.Sprintf("%s %s", req.Method, urlTemplate)) + } + } + + resp, err := t.base.RoundTrip(req) + + if span.IsRecording() { + if err != nil { + span.SetAttributes( + attribute.String("error.type", "ERROR"), + attribute.String("status.message", err.Error()), + attribute.String("exception.type", fmt.Sprintf("%T", err)), + ) + } else if resp.StatusCode >= 400 { + span.SetAttributes( + attribute.String("error.type", strconv.Itoa(resp.StatusCode)), + attribute.String("status.message", resp.Status), + ) + } + } + + return resp, err } type authTransport struct { From b50160825af42820dc85f1a2fcb0aac921e90b95 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 12:12:53 -0600 Subject: [PATCH 02/18] fixes for Gemini comments --- auth/grpctransport/grpctransport.go | 15 ++++-- auth/grpctransport/grpctransport_otel_test.go | 44 +++++++++------- auth/httptransport/httptransport_otel_test.go | 52 ++++++++++--------- 3 files changed, 61 insertions(+), 50 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index eaa00379e8b1..c906c84cd954 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -471,14 +471,18 @@ func (h *otelHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) contex if !span.IsRecording() { return ctx } + var attrs []attribute.KeyValue if resName, ok := callctx.TelemetryFromContext(ctx, "resource_name"); ok { - span.SetAttributes(attribute.String("gcp.resource.name", resName)) + attrs = append(attrs, attribute.String("gcp.resource.name", resName)) } if resendCountStr, ok := callctx.TelemetryFromContext(ctx, "resend_count"); ok { if count, err := strconv.Atoi(resendCountStr); err == nil { - span.SetAttributes(attribute.Int("gcp.grpc.resend_count", count)) + attrs = append(attrs, attribute.Int("gcp.grpc.resend_count", count)) } } + if len(attrs) > 0 { + span.SetAttributes(attrs...) + } return ctx } @@ -497,16 +501,17 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { } // Extract status code and message st, _ := status.FromError(end.Error) - span.SetAttributes( + attrs := []attribute.KeyValue{ attribute.String("error.type", strings.ToUpper(st.Code().String())), attribute.String("status.message", st.Message()), attribute.String("grpc.status", strings.ToUpper(st.Code().String())), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), - ) + } if md, ok := metadata.FromOutgoingContext(ctx); ok { if v := md[":authority"]; len(v) > 0 { - span.SetAttributes(attribute.String("url.domain", v[0])) + attrs = append(attrs, attribute.String("url.domain", v[0])) } } + span.SetAttributes(attrs...) h.Handler.HandleRPC(ctx, s) } diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index 8cf6d3f446bf..b244fbaea437 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -79,13 +79,14 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { } tests := []struct { - name string - echoer echo.EchoerServer - opts *Options - wantErr bool - wantSpans int - wantSpan sdktrace.ReadOnlySpan - wantAttrKeys []attribute.Key + name string + echoer echo.EchoerServer + opts *Options + telemetryCtxValues map[string]string + wantErr bool + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key }{ { name: "telemetry enabled success", @@ -154,7 +155,8 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { }, }, }, - wantSpans: 1, + telemetryCtxValues: map[string]string{"resource_name": "my-resource"}, + wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "echo.Echoer/Echo", SpanKind: oteltrace.SpanKindClient, @@ -198,8 +200,8 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { defer pool.Close() ctx := context.Background() - if tt.name == "telemetry enabled metadata enrichment" { - ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + for k, v := range tt.telemetryCtxValues { + ctx = callctx.WithTelemetryContext(ctx, k, v) } client := echo.NewEchoerClient(pool) @@ -277,13 +279,14 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { } tests := []struct { - name string - echoer echo.EchoerServer - opts *Options - wantErr bool - wantSpans int - wantSpan sdktrace.ReadOnlySpan - wantAttrKeys []attribute.Key + name string + echoer echo.EchoerServer + opts *Options + telemetryCtxValues map[string]string + wantErr bool + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key }{ { name: "telemetry enabled success (but gated off)", @@ -350,7 +353,8 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { }, }, }, - wantSpans: 1, + telemetryCtxValues: map[string]string{"resource_name": "my-resource"}, + wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "echo.Echoer/Echo", SpanKind: oteltrace.SpanKindClient, @@ -393,8 +397,8 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { defer pool.Close() ctx := context.Background() - if tt.name == "telemetry enabled metadata enrichment (but gated off)" { - ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + for k, v := range tt.telemetryCtxValues { + ctx = callctx.WithTelemetryContext(ctx, k, v) } client := echo.NewEchoerClient(pool) diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 41ce0fcfe75f..9bd598e5002c 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -62,12 +62,13 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { otel.SetTracerProvider(tp) tests := []struct { - name string - opts *Options - statusCode int - wantSpans int - wantSpan sdktrace.ReadOnlySpan - wantAttrKeys []attribute.Key + name string + opts *Options + telemetryCtxValues map[string]string + statusCode int + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key }{ { name: "telemetry enabled success", @@ -139,8 +140,9 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { }, }, }, - statusCode: http.StatusOK, - wantSpans: 1, + telemetryCtxValues: map[string]string{"resource_name": "my-resource"}, + statusCode: http.StatusOK, + wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, @@ -159,8 +161,9 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { opts: &Options{ DisableAuthentication: true, }, - statusCode: http.StatusOK, - wantSpans: 1, + telemetryCtxValues: map[string]string{"url_template": "/my/template"}, + statusCode: http.StatusOK, + wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "GET /my/template", SpanKind: oteltrace.SpanKindClient, @@ -191,11 +194,8 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { } ctx := context.Background() - if tt.name == "telemetry enabled metadata enrichment" { - ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") - } - if tt.name == "telemetry enabled url template" { - ctx = callctx.WithTelemetryContext(ctx, "url_template", "/my/template") + for k, v := range tt.telemetryCtxValues { + ctx = callctx.WithTelemetryContext(ctx, k, v) } req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) @@ -294,12 +294,13 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { otel.SetTracerProvider(tp) tests := []struct { - name string - opts *Options - statusCode int - wantSpans int - wantSpan sdktrace.ReadOnlySpan - wantAttrKeys []attribute.Key + name string + opts *Options + telemetryCtxValues map[string]string + statusCode int + wantSpans int + wantSpan sdktrace.ReadOnlySpan + wantAttrKeys []attribute.Key }{ { name: "telemetry enabled success (but gated off)", @@ -363,8 +364,9 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { }, }, }, - statusCode: http.StatusOK, - wantSpans: 1, + telemetryCtxValues: map[string]string{"resource_name": "my-resource"}, + statusCode: http.StatusOK, + wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, @@ -394,8 +396,8 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { } ctx := context.Background() - if tt.name == "telemetry enabled metadata enrichment (but gated off)" { - ctx = callctx.WithTelemetryContext(ctx, "resource_name", "my-resource") + for k, v := range tt.telemetryCtxValues { + ctx = callctx.WithTelemetryContext(ctx, k, v) } req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) From f4fd2abb162f82fcd688322b0fa7de9c23c4e8a9 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 13:15:34 -0600 Subject: [PATCH 03/18] fix for Gemini comment in httptransport --- auth/httptransport/transport.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 1f3efd5006e1..0491449571bf 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -207,21 +207,22 @@ type otelAttributeTransport struct { func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, error) { span := trace.SpanFromContext(req.Context()) if span.IsRecording() { - span.SetAttributes(t.staticAttrs...) - span.SetAttributes(attribute.String("rpc.system", "http")) - span.SetAttributes(attribute.String("url.domain", req.URL.Host)) + var attrs []attribute.KeyValue + attrs = append(attrs, t.staticAttrs...) + attrs = append(attrs, attribute.String("rpc.system", "http"), attribute.String("url.domain", req.URL.Host)) if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { - span.SetAttributes(attribute.String("gcp.resource.name", resName)) + attrs = append(attrs, attribute.String("gcp.resource.name", resName)) } if resendCountStr, ok := callctx.TelemetryFromContext(req.Context(), "resend_count"); ok { if count, err := strconv.Atoi(resendCountStr); err == nil { - span.SetAttributes(attribute.Int("gcp.grpc.resend_count", count)) + attrs = append(attrs, attribute.Int("gcp.grpc.resend_count", count)) } } if urlTemplate, ok := callctx.TelemetryFromContext(req.Context(), "url_template"); ok { - span.SetAttributes(attribute.String("url.template", urlTemplate)) + attrs = append(attrs, attribute.String("url.template", urlTemplate)) span.SetName(fmt.Sprintf("%s %s", req.Method, urlTemplate)) } + span.SetAttributes(attrs...) } resp, err := t.base.RoundTrip(req) From 90baac0dd85f4203c2cc70d8f80a00b81a3dd2e4 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 13:43:15 -0600 Subject: [PATCH 04/18] update gcp.resource.name to gcp.resource.destination.id --- auth/grpctransport/grpctransport.go | 2 +- auth/grpctransport/grpctransport_otel_test.go | 2 +- auth/httptransport/httptransport_otel_test.go | 2 +- auth/httptransport/transport.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index c906c84cd954..9b1cd12a29e4 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -473,7 +473,7 @@ func (h *otelHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) contex } var attrs []attribute.KeyValue if resName, ok := callctx.TelemetryFromContext(ctx, "resource_name"); ok { - attrs = append(attrs, attribute.String("gcp.resource.name", resName)) + attrs = append(attrs, attribute.String("gcp.resource.destination.id", resName)) } if resendCountStr, ok := callctx.TelemetryFromContext(ctx, "resend_count"); ok { if count, err := strconv.Atoi(resendCountStr); err == nil { diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index b244fbaea437..81a1d1a3379e 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -169,7 +169,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("gcp.resource.name", "my-resource"), + attribute.String("gcp.resource.destination.id", "my-resource"), attribute.String("gcp.client.version", "1.0.0"), }, }.Snapshot(), diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 9bd598e5002c..350132cf613e 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -149,7 +149,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), keyHTTPResponseStatus.Int(200), - attribute.String("gcp.resource.name", "my-resource"), + attribute.String("gcp.resource.destination.id", "my-resource"), attribute.String("gcp.client.version", "1.0.0"), attribute.String("rpc.system", "http"), }, diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 0491449571bf..a74e36c13ebc 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -211,7 +211,7 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e attrs = append(attrs, t.staticAttrs...) attrs = append(attrs, attribute.String("rpc.system", "http"), attribute.String("url.domain", req.URL.Host)) if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { - attrs = append(attrs, attribute.String("gcp.resource.name", resName)) + attrs = append(attrs, attribute.String("gcp.resource.destination.id", resName)) } if resendCountStr, ok := callctx.TelemetryFromContext(req.Context(), "resend_count"); ok { if count, err := strconv.Atoi(resendCountStr); err == nil { From 372467b6ceccd23d9d2352344a95663eb7a2fa79 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 13:45:58 -0600 Subject: [PATCH 05/18] update grpc.status to rpc.response.status_code --- auth/grpctransport/grpctransport.go | 2 +- auth/grpctransport/grpctransport_otel_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 9b1cd12a29e4..f62d2878fbe7 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -504,7 +504,7 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { attrs := []attribute.KeyValue{ attribute.String("error.type", strings.ToUpper(st.Code().String())), attribute.String("status.message", st.Message()), - attribute.String("grpc.status", strings.ToUpper(st.Code().String())), + attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), } if md, ok := metadata.FromOutgoingContext(ctx); ok { diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index 81a1d1a3379e..7c803b8457d6 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -130,7 +130,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyServerAddr.String(valLocalhost), attribute.String("error.type", "INTERNAL"), attribute.String("status.message", "test error"), - attribute.String("grpc.status", "INTERNAL"), + attribute.String("rpc.response.status_code", "INTERNAL"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, From 3dc0cb8e5613fe12e4a5b1b70bc0433feb0f87ef Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 13:48:37 -0600 Subject: [PATCH 06/18] update rpc.system to rpc.system.name --- auth/httptransport/httptransport_otel_test.go | 8 ++++---- auth/httptransport/transport.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 350132cf613e..8e9179cbe67e 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -91,7 +91,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { // "server.address", "server.port", and "url.full" are displayed as // standard attribute keys in the "Attributes" tab. keyServerAddr.String(valLocalhost), - attribute.String("rpc.system", "http"), + attribute.String("rpc.system.name", "http"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, @@ -116,7 +116,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { keyServerAddr.String(valLocalhost), keyErrorType.String("500"), attribute.String("status.message", "500 Internal Server Error"), - attribute.String("rpc.system", "http"), + attribute.String("rpc.system.name", "http"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, @@ -151,7 +151,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { keyHTTPResponseStatus.Int(200), attribute.String("gcp.resource.destination.id", "my-resource"), attribute.String("gcp.client.version", "1.0.0"), - attribute.String("rpc.system", "http"), + attribute.String("rpc.system.name", "http"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, @@ -171,7 +171,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { keyHTTPRequestMetod.String(valHTTPGet), keyHTTPResponseStatus.Int(200), attribute.String("url.template", "/my/template"), - attribute.String("rpc.system", "http"), + attribute.String("rpc.system.name", "http"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index a74e36c13ebc..99286ee9b49c 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -209,7 +209,7 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e if span.IsRecording() { var attrs []attribute.KeyValue attrs = append(attrs, t.staticAttrs...) - attrs = append(attrs, attribute.String("rpc.system", "http"), attribute.String("url.domain", req.URL.Host)) + attrs = append(attrs, attribute.String("rpc.system.name", "http"), attribute.String("url.domain", req.URL.Host)) if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { attrs = append(attrs, attribute.String("gcp.resource.destination.id", resName)) } From 1e8618304107ef40d972ef6b5abe64fe90a14dc2 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 14:09:01 -0600 Subject: [PATCH 07/18] update gcp.grpc.resend_count to http.request.resend_count --- auth/httptransport/httptransport_otel_test.go | 39 +++++++++++++++++++ auth/httptransport/transport.go | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 8e9179cbe67e..4177f8211089 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -176,6 +176,26 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, }, + { + name: "telemetry enabled resend count", + opts: &Options{ + DisableAuthentication: true, + }, + telemetryCtxValues: map[string]string{"resend_count": "2"}, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + attribute.Int("http.request.resend_count", 2), + attribute.String("rpc.system.name", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + }, } for _, tt := range tests { @@ -378,6 +398,25 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerAddr}, // NO url.domain }, + { + name: "telemetry enabled resend count (but gated off)", + opts: &Options{ + DisableAuthentication: true, + }, + telemetryCtxValues: map[string]string{"resend_count": "2"}, + statusCode: http.StatusOK, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyHTTPResponseStatus.Int(200), + // NO http.request.resend_count + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr}, // NO url.domain + }, } for _, tt := range tests { diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 99286ee9b49c..c37193461b35 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -215,7 +215,7 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e } if resendCountStr, ok := callctx.TelemetryFromContext(req.Context(), "resend_count"); ok { if count, err := strconv.Atoi(resendCountStr); err == nil { - attrs = append(attrs, attribute.Int("gcp.grpc.resend_count", count)) + attrs = append(attrs, attribute.Int("http.request.resend_count", count)) } } if urlTemplate, ok := callctx.TelemetryFromContext(req.Context(), "url_template"); ok { From 8e646bed2325168e96aa2d0f6b455ae8b48ee363 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 17:34:36 -0600 Subject: [PATCH 08/18] populate static known attributes from generator values including url.domain --- auth/grpctransport/grpctransport.go | 14 ++------- auth/grpctransport/grpctransport_otel_test.go | 20 ++++++++++++- auth/httptransport/httptransport_otel_test.go | 30 +++++++++++++++---- auth/httptransport/transport.go | 6 ++-- auth/internal/transport/transport.go | 23 ++++++++++++++ 5 files changed, 71 insertions(+), 22 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index f62d2878fbe7..03fbf6c00790 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -41,7 +41,6 @@ import ( "google.golang.org/grpc" grpccreds "google.golang.org/grpc/credentials" grpcinsecure "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/metadata" "google.golang.org/grpc/stats" "google.golang.org/grpc/status" ) @@ -442,14 +441,12 @@ func addOpenTelemetryStatsHandler(dialOpts []grpc.DialOption, opts *Options) []g if !gax.IsFeatureEnabled("TRACING") { return append(dialOpts, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) } - var attrs []attribute.KeyValue + var staticAttrs []attribute.KeyValue if opts.InternalOptions != nil { - for k, v := range opts.InternalOptions.TelemetryAttributes { - attrs = append(attrs, attribute.String(k, v)) - } + staticAttrs = transport.StaticTelemetryAttributes(opts.InternalOptions.TelemetryAttributes) } otelOpts := []otelgrpc.Option{ - otelgrpc.WithSpanAttributes(attrs...), + otelgrpc.WithSpanAttributes(staticAttrs...), } return append(dialOpts, grpc.WithStatsHandler(&otelHandler{ Handler: otelgrpc.NewClientHandler(otelOpts...), @@ -507,11 +504,6 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), } - if md, ok := metadata.FromOutgoingContext(ctx); ok { - if v := md[":authority"]; len(v) > 0 { - attrs = append(attrs, attribute.String("url.domain", v[0])) - } - } span.SetAttributes(attrs...) h.Handler.HandleRPC(ctx, s) } diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index 7c803b8457d6..40e76253d5d4 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -151,7 +151,13 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { DisableAuthentication: true, InternalOptions: &InternalOptions{ TelemetryAttributes: map[string]string{ - "gcp.client.version": "1.0.0", + "gcp.client.service": "echo", + "gcp.client.version": "1.0.0", + "gcp.client.repo": "googleapis/google-cloud-go", + "gcp.client.artifact": "c.g/auth/grpctransport", + "gcp.client.language": "go", + "url.domain": "echo.googleapis.com", + "ignored.key": "should not be included", }, }, }, @@ -170,7 +176,12 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), attribute.String("gcp.resource.destination.id", "my-resource"), + attribute.String("gcp.client.service", "echo"), attribute.String("gcp.client.version", "1.0.0"), + attribute.String("gcp.client.repo", "googleapis/google-cloud-go"), + attribute.String("gcp.client.artifact", "c.g/auth/grpctransport"), + attribute.String("gcp.client.language", "go"), + attribute.String("url.domain", "echo.googleapis.com"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, @@ -246,6 +257,9 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { t.Errorf("missing attribute key: %s", wantKey) } } + if _, ok := gotAttrs[attribute.Key("ignored.key")]; ok { + t.Errorf("found unexpected attribute key: ignored.key") + } } }) } @@ -350,6 +364,7 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { InternalOptions: &InternalOptions{ TelemetryAttributes: map[string]string{ "gcp.client.version": "1.0.0", + "ignored.key": "should not be included", }, }, }, @@ -442,6 +457,9 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { t.Errorf("missing attribute key: %s", wantKey) } } + if _, ok := gotAttrs[attribute.Key("ignored.key")]; ok { + t.Errorf("found unexpected attribute key: ignored.key") + } } }) } diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 4177f8211089..08e97bc8efac 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -94,7 +94,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { attribute.String("rpc.system.name", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, }, { name: "telemetry enabled error", @@ -119,7 +119,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { attribute.String("rpc.system.name", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull, attribute.Key("url.domain")}, + wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, }, { name: "telemetry disabled", @@ -136,7 +136,13 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { DisableAuthentication: true, InternalOptions: &InternalOptions{ TelemetryAttributes: map[string]string{ - "gcp.client.version": "1.0.0", + "gcp.client.service": "myservice", + "gcp.client.version": "1.0.0", + "gcp.client.repo": "googleapis/google-cloud-go", + "gcp.client.artifact": "c.g/auth/httptransport", + "gcp.client.language": "go", + "url.domain": "myservice.googleapis.com", + "ignored.key": "should not be included", }, }, }, @@ -150,11 +156,16 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { keyHTTPRequestMetod.String(valHTTPGet), keyHTTPResponseStatus.Int(200), attribute.String("gcp.resource.destination.id", "my-resource"), + attribute.String("gcp.client.service", "myservice"), attribute.String("gcp.client.version", "1.0.0"), + attribute.String("gcp.client.repo", "googleapis/google-cloud-go"), + attribute.String("gcp.client.artifact", "c.g/auth/httptransport"), + attribute.String("gcp.client.language", "go"), attribute.String("rpc.system.name", "http"), + attribute.String("url.domain", "myservice.googleapis.com"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + wantAttrKeys: []attribute.Key{keyServerAddr}, }, { name: "telemetry enabled url template", @@ -174,7 +185,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { attribute.String("rpc.system.name", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + wantAttrKeys: []attribute.Key{keyServerAddr}, }, { name: "telemetry enabled resend count", @@ -194,7 +205,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { attribute.String("rpc.system.name", "http"), }, }.Snapshot(), - wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("url.domain")}, + wantAttrKeys: []attribute.Key{keyServerAddr}, }, } @@ -293,6 +304,9 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { t.Errorf("missing attribute key: %s", wantKey) } } + if _, ok := gotAttrs[attribute.Key("ignored.key")]; ok { + t.Errorf("found unexpected attribute key: ignored.key") + } } }) } @@ -381,6 +395,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { InternalOptions: &InternalOptions{ TelemetryAttributes: map[string]string{ "gcp.client.version": "1.0.0", + "ignored.key": "should not be included", }, }, }, @@ -486,6 +501,9 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { t.Errorf("missing attribute key: %s", wantKey) } } + if _, ok := gotAttrs[attribute.Key("ignored.key")]; ok { + t.Errorf("found unexpected attribute key: ignored.key") + } } }) } diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index c37193461b35..f9aa1141cb99 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -184,9 +184,7 @@ func addOpenTelemetryTransport(trans http.RoundTripper, opts *Options) http.Roun } var staticAttrs []attribute.KeyValue if opts.InternalOptions != nil { - for k, v := range opts.InternalOptions.TelemetryAttributes { - staticAttrs = append(staticAttrs, attribute.String(k, v)) - } + staticAttrs = transport.StaticTelemetryAttributes(opts.InternalOptions.TelemetryAttributes) } return otelhttp.NewTransport(&otelAttributeTransport{ base: trans, @@ -209,7 +207,7 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e if span.IsRecording() { var attrs []attribute.KeyValue attrs = append(attrs, t.staticAttrs...) - attrs = append(attrs, attribute.String("rpc.system.name", "http"), attribute.String("url.domain", req.URL.Host)) + attrs = append(attrs, attribute.String("rpc.system.name", "http")) if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { attrs = append(attrs, attribute.String("gcp.resource.destination.id", resName)) } diff --git a/auth/internal/transport/transport.go b/auth/internal/transport/transport.go index 5c8721efa9f2..752e3b8d8266 100644 --- a/auth/internal/transport/transport.go +++ b/auth/internal/transport/transport.go @@ -24,8 +24,31 @@ import ( "time" "cloud.google.com/go/auth/credentials" + "go.opentelemetry.io/otel/attribute" ) +var knownKeys = []string{ + "gcp.client.service", + "gcp.client.version", + "gcp.client.repo", + "gcp.client.artifact", + "gcp.client.language", + "url.domain", +} + +func StaticTelemetryAttributes(m map[string]string) []attribute.KeyValue { + var staticAttrs []attribute.KeyValue + if m == nil { + return staticAttrs + } + for _, k := range knownKeys { + if v, ok := m[k]; ok { + staticAttrs = append(staticAttrs, attribute.String(k, v)) + } + } + return staticAttrs +} + // CloneDetectOptions clones a user set detect option into some new memory that // we can internally manipulate before sending onto the detect package. func CloneDetectOptions(oldDo *credentials.DetectOptions) *credentials.DetectOptions { From 2276c046dc0187723bfe90524941c24a199790df Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 9 Mar 2026 17:42:38 -0600 Subject: [PATCH 09/18] add internal function doc --- auth/internal/transport/transport.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/auth/internal/transport/transport.go b/auth/internal/transport/transport.go index 752e3b8d8266..08a5b024978d 100644 --- a/auth/internal/transport/transport.go +++ b/auth/internal/transport/transport.go @@ -36,6 +36,8 @@ var knownKeys = []string{ "url.domain", } +// StaticTelemetryAttributes selectively converts known keys from a map of +// strings to Open Telemetry attributes. func StaticTelemetryAttributes(m map[string]string) []attribute.KeyValue { var staticAttrs []attribute.KeyValue if m == nil { From dabf100ccb75ff013166f7c24c0e5350b1073a40 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 10 Mar 2026 09:51:52 -0600 Subject: [PATCH 10/18] update gax-go/v2 dep to 2.18.0 --- auth/go.mod | 10 +++++----- auth/go.sum | 42 +++++++++++++++++++++--------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/auth/go.mod b/auth/go.mod index adb78beead63..38094f63e68a 100644 --- a/auth/go.mod +++ b/auth/go.mod @@ -7,7 +7,7 @@ require ( github.com/google/go-cmp v0.7.0 github.com/google/s2a-go v0.1.9 github.com/googleapis/enterprise-certificate-proxy v0.3.11 - github.com/googleapis/gax-go/v2 v2.17.0 + github.com/googleapis/gax-go/v2 v2.18.0 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 go.opentelemetry.io/otel v1.40.0 @@ -15,7 +15,7 @@ require ( go.opentelemetry.io/otel/trace v1.40.0 golang.org/x/net v0.49.0 golang.org/x/time v0.14.0 - google.golang.org/grpc v1.78.0 + google.golang.org/grpc v1.79.1 google.golang.org/protobuf v1.36.11 ) @@ -28,10 +28,10 @@ require ( go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/otel/metric v1.40.0 // indirect golang.org/x/crypto v0.47.0 // indirect - golang.org/x/oauth2 v0.34.0 // indirect + golang.org/x/oauth2 v0.35.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect - google.golang.org/api v0.264.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect + google.golang.org/api v0.267.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260217215200-42d3e9bedb6d // indirect ) diff --git a/auth/go.sum b/auth/go.sum index 83fe9814f8f0..76171c05dc11 100644 --- a/auth/go.sum +++ b/auth/go.sum @@ -2,15 +2,15 @@ cloud.google.com/go/compute/metadata v0.9.0 h1:pDUj4QMoPejqq20dK0Pg2N4yG9zIkYGdB cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f h1:Y8xYupdHxryycyPlc9Y+bSQAYZnetRJ70VMVKm5CKI0= -github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f/go.mod h1:HlzOvOjVBOfTGSRXRyY0OiCS/3J1akRGQQpRO/7zyF4= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/Tv1FZd4SCg8axKApyNyRsAt/w= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/envoyproxy/go-control-plane v0.13.5-0.20251024222203-75eaa193e329 h1:K+fnvUM0VZ7ZFJf0n4L/BRlnsb9pL/GuDG6FqaH+PwM= -github.com/envoyproxy/go-control-plane/envoy v1.35.0 h1:ixjkELDE+ru6idPxcHLj8LBVc2bFP7iBytj353BoHUo= -github.com/envoyproxy/go-control-plane/envoy v1.35.0/go.mod h1:09qwbGVuSWWAyN5t/b3iyVfz5+z8QWGrzkoqm/8SbEs= -github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= -github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= +github.com/envoyproxy/go-control-plane v0.14.0 h1:hbG2kr4RuFj222B6+7T83thSPqLjwBIfQawTkC++2HA= +github.com/envoyproxy/go-control-plane/envoy v1.36.0 h1:yg/JjO5E7ubRyKX3m07GF3reDNEnfOboJ0QySbH736g= +github.com/envoyproxy/go-control-plane/envoy v1.36.0/go.mod h1:ty89S1YCCVruQAm9OtKeEkQLTb+Lkz0k8v9W0Oxsv98= +github.com/envoyproxy/protoc-gen-validate v1.3.0 h1:TvGH1wof4H33rezVKWSpqKz5NXWg5VPuZ0uONDT6eb4= +github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -28,8 +28,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/enterprise-certificate-proxy v0.3.11 h1:vAe81Msw+8tKUxi2Dqh/NZMz7475yUvmRIkXr4oN2ao= github.com/googleapis/enterprise-certificate-proxy v0.3.11/go.mod h1:RFV7MUdlb7AgEq2v7FmMCfeSMCllAzWxFgRdusoGks8= -github.com/googleapis/gax-go/v2 v2.17.0 h1:RksgfBpxqff0EZkDWYuz9q/uWsTVz+kf43LsZ1J6SMc= -github.com/googleapis/gax-go/v2 v2.17.0/go.mod h1:mzaqghpQp4JDh3HvADwrat+6M3MOIDp5YKHhb9PAgDY= +github.com/googleapis/gax-go/v2 v2.18.0 h1:jxP5Uuo3bxm3M6gGtV94P4lliVetoCB4Wk2x8QA86LI= +github.com/googleapis/gax-go/v2 v2.18.0/go.mod h1:uSzZN4a356eRG985CzJ3WfbFSpqkLTjsnhWGJR6EwrE= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -58,8 +58,8 @@ golang.org/x/crypto v0.47.0 h1:V6e3FRj+n4dbpw86FJ8Fv7XVOql7TEwpHapKoMJ/GO8= golang.org/x/crypto v0.47.0/go.mod h1:ff3Y9VzzKbwSSEzWqJsJVBnWmRwRSHt/6Op5n9bQc4A= golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= -golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ= +golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= @@ -70,16 +70,16 @@ golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= -google.golang.org/api v0.264.0 h1:+Fo3DQXBK8gLdf8rFZ3uLu39JpOnhvzJrLMQSoSYZJM= -google.golang.org/api v0.264.0/go.mod h1:fAU1xtNNisHgOF5JooAs8rRaTkl2rT3uaoNGo9NS3R8= -google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 h1:VQZ/yAbAtjkHgH80teYd2em3xtIkkHd7ZhqfH2N9CsM= -google.golang.org/genproto v0.0.0-20260128011058-8636f8732409/go.mod h1:rxKD3IEILWEu3P44seeNOAwZN4SaoKaQ/2eTg4mM6EM= -google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409 h1:merA0rdPeUV3YIIfHHcH4qBkiQAc1nfCKSI7lB4cV2M= -google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409/go.mod h1:fl8J1IvUjCilwZzQowmw2b7HQB2eAuYBabMXzWurF+I= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 h1:H86B94AW+VfJWDqFeEbBPhEtHzJwJfTbgE2lZa54ZAQ= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= -google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= -google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U= +google.golang.org/api v0.267.0 h1:w+vfWPMPYeRs8qH1aYYsFX68jMls5acWl/jocfLomwE= +google.golang.org/api v0.267.0/go.mod h1:Jzc0+ZfLnyvXma3UtaTl023TdhZu6OMBP9tJ+0EmFD0= +google.golang.org/genproto v0.0.0-20260217215200-42d3e9bedb6d h1:vsOm753cOAMkt76efriTCDKjpCbK18XGHMJHo0JUKhc= +google.golang.org/genproto v0.0.0-20260217215200-42d3e9bedb6d/go.mod h1:0oz9d7g9QLSdv9/lgbIjowW1JoxMbxmBVNe8i6tORJI= +google.golang.org/genproto/googleapis/api v0.0.0-20260217215200-42d3e9bedb6d h1:EocjzKLywydp5uZ5tJ79iP6Q0UjDnyiHkGRWxuPBP8s= +google.golang.org/genproto/googleapis/api v0.0.0-20260217215200-42d3e9bedb6d/go.mod h1:48U2I+QQUYhsFrg2SY6r+nJzeOtjey7j//WBESw+qyQ= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260217215200-42d3e9bedb6d h1:t/LOSXPJ9R0B6fnZNyALBRfZBH0Uy0gT+uR+SJ6syqQ= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260217215200-42d3e9bedb6d/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.79.1 h1:zGhSi45ODB9/p3VAawt9a+O/MULLl9dpizzNNpq7flY= +google.golang.org/grpc v1.79.1/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 448149cc62bdad807561b3977ca3f0b3b14c7499 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 10 Mar 2026 14:48:54 -0600 Subject: [PATCH 11/18] add knownKeys test and docs --- auth/internal/transport/transport.go | 5 +++++ auth/internal/transport/transport_test.go | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/auth/internal/transport/transport.go b/auth/internal/transport/transport.go index 08a5b024978d..fb0a8a1e0748 100644 --- a/auth/internal/transport/transport.go +++ b/auth/internal/transport/transport.go @@ -27,6 +27,11 @@ import ( "go.opentelemetry.io/otel/attribute" ) +// knownKeys provides keys for reading telemetry attributes from Context. +// It provides an implicit contract with generated client library code +// using the same keys. The keys in this collection should not be removed +// or modified. New keys may be added, but they will need to be explicitly +// used in code referencing this collection in order to appear in telemetry. var knownKeys = []string{ "gcp.client.service", "gcp.client.version", diff --git a/auth/internal/transport/transport_test.go b/auth/internal/transport/transport_test.go index 9a4643ab2e01..798417e0268a 100644 --- a/auth/internal/transport/transport_test.go +++ b/auth/internal/transport/transport_test.go @@ -112,3 +112,17 @@ func TestCloneDetectOptions(t *testing.T) { t.Fatalf("Scopes should not reference the same slice") } } + +func TestStaticTelemetryAttributes_KnownKeys(t *testing.T) { + want := []string{ + "gcp.client.service", + "gcp.client.version", + "gcp.client.repo", + "gcp.client.artifact", + "gcp.client.language", + "url.domain", + } + if !reflect.DeepEqual(knownKeys, want) { + t.Errorf("got %v, want %v", knownKeys, want) + } +} From 29ae1a497edbc20cef693ad9f277e6ef0378ddb2 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 10 Mar 2026 15:29:21 -0600 Subject: [PATCH 12/18] set rpc.response.status_code OK when HandleRPC succeeds --- auth/grpctransport/grpctransport.go | 24 ++++++++++++------- auth/grpctransport/grpctransport_otel_test.go | 1 + 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 03fbf6c00790..2634409dfdb7 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -487,7 +487,7 @@ func (h *otelHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) contex // attributes ensuring they conform to Google Cloud observability standards. func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { end, ok := s.(*stats.End) - if !ok || end.Error == nil { + if !ok { h.Handler.HandleRPC(ctx, s) return } @@ -496,13 +496,21 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { h.Handler.HandleRPC(ctx, s) return } - // Extract status code and message - st, _ := status.FromError(end.Error) - attrs := []attribute.KeyValue{ - attribute.String("error.type", strings.ToUpper(st.Code().String())), - attribute.String("status.message", st.Message()), - attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), - attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), + + var attrs []attribute.KeyValue + if end.Error != nil { + // Extract status code and message + st, _ := status.FromError(end.Error) + attrs = []attribute.KeyValue{ + attribute.String("error.type", strings.ToUpper(st.Code().String())), + attribute.String("status.message", st.Message()), + attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), + attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), + } + } else { + attrs = []attribute.KeyValue{ + attribute.String("rpc.response.status_code", "OK"), + } } span.SetAttributes(attrs...) h.Handler.HandleRPC(ctx, s) diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index 40e76253d5d4..d27eb1270e26 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -182,6 +182,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { attribute.String("gcp.client.artifact", "c.g/auth/grpctransport"), attribute.String("gcp.client.language", "go"), attribute.String("url.domain", "echo.googleapis.com"), + attribute.String("rpc.response.status_code", "OK"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, From 1556dc4c97c1430d0c18f0ff1d1bc2de2fd2593b Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 10 Mar 2026 16:01:14 -0600 Subject: [PATCH 13/18] improve error handling in HTTP RoundTrip --- auth/httptransport/httptransport_otel_test.go | 193 ++++++++++++++++-- auth/httptransport/transport.go | 26 ++- 2 files changed, 197 insertions(+), 22 deletions(-) diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 08e97bc8efac..7aa12c62ad7c 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -19,8 +19,10 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/googleapis/gax-go/v2" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -66,6 +68,8 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { opts *Options telemetryCtxValues map[string]string statusCode int + errorType string // "timeout", "cancel", "connection" + wantErr bool wantSpans int wantSpan sdktrace.ReadOnlySpan wantAttrKeys []attribute.Key @@ -82,14 +86,9 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { Code: codes.Unset, }, Attributes: []attribute.KeyValue{ - // In Cloud Trace, this often forms part of the Span Name. keyHTTPRequestMetod.String(valHTTPGet), - // In Cloud Trace, this status code maps to the visual "Status" field - // (e.g., a green checkmark for 200, or an error icon for 5xx). keyHTTPResponseStatus.Int(200), keyNetProtoVersion.String(valHTTP11), - // "server.address", "server.port", and "url.full" are displayed as - // standard attribute keys in the "Attributes" tab. keyServerAddr.String(valLocalhost), attribute.String("rpc.system.name", "http"), }, @@ -100,6 +99,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { name: "telemetry enabled error", opts: &Options{DisableAuthentication: true}, statusCode: http.StatusInternalServerError, + wantErr: false, // The RoundTrip itself doesn't return an error for 500, it returns a response. wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "HTTP GET", @@ -110,7 +110,6 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { }, Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), - // In Cloud Trace, 5xx status codes are displayed as errors in the "Status" field. keyHTTPResponseStatus.Int(500), keyNetProtoVersion.String(valHTTP11), keyServerAddr.String(valLocalhost), @@ -121,6 +120,68 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, }, + { + name: "telemetry enabled client timeout", + opts: &Options{DisableAuthentication: true}, + errorType: "timeout", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "context deadline exceeded", + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyErrorType.String("context.deadlineExceededError"), + attribute.String("rpc.system.name", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("status.message"), attribute.Key("exception.type")}, + }, + { + name: "telemetry enabled client cancelled", + opts: &Options{DisableAuthentication: true}, + errorType: "cancel", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "context canceled", + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyErrorType.String("*errors.errorString"), + attribute.String("rpc.system.name", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("status.message"), attribute.Key("exception.type")}, + }, + { + name: "telemetry enabled client connection error", + opts: &Options{DisableAuthentication: true}, + errorType: "connection", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + keyErrorType.String("*net.OpError"), + attribute.String("rpc.system.name", "http"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr, attribute.Key("status.message"), attribute.Key("exception.type")}, + }, { name: "telemetry disabled", opts: &Options{ @@ -214,10 +275,19 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { exporter.Reset() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(tt.statusCode) + if tt.errorType == "timeout" { + time.Sleep(100 * time.Millisecond) + } + if tt.statusCode != 0 { + w.WriteHeader(tt.statusCode) + } })) defer server.Close() + if tt.errorType == "connection" { + server.Close() + } + tt.opts.Endpoint = server.URL client, err := NewClient(tt.opts) if err != nil { @@ -225,6 +295,15 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { } ctx := context.Background() + var cancel context.CancelFunc + if tt.errorType == "timeout" { + ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) + defer cancel() + } else if tt.errorType == "cancel" { + ctx, cancel = context.WithCancel(ctx) + cancel() + } + for k, v := range tt.telemetryCtxValues { ctx = callctx.WithTelemetryContext(ctx, k, v) } @@ -235,10 +314,12 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { } resp, err := client.Do(req) - if err != nil { - t.Fatalf("client.Do() = %v, want nil", err) + if (err != nil) != tt.wantErr { + t.Errorf("client.Do() error = %v, wantErr %v", err, tt.wantErr) + } + if resp != nil && resp.Body != nil { + resp.Body.Close() } - resp.Body.Close() spans := exporter.GetSpans() if len(spans) != tt.wantSpans { @@ -253,7 +334,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status, cmpopts.IgnoreFields(sdktrace.Status{}, "Description")); diff != "" { t.Errorf("span.Status mismatch (-want +got):\n%s", diff) } @@ -332,6 +413,8 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { opts *Options telemetryCtxValues map[string]string statusCode int + errorType string // "timeout", "cancel", "connection" + wantErr bool wantSpans int wantSpan sdktrace.ReadOnlySpan wantAttrKeys []attribute.Key @@ -361,6 +444,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { name: "telemetry enabled error (but gated off)", opts: &Options{DisableAuthentication: true}, statusCode: http.StatusInternalServerError, + wantErr: false, wantSpans: 1, wantSpan: tracetest.SpanStub{ Name: "HTTP GET", @@ -379,6 +463,63 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort, keyURLFull}, // NO url.domain }, + { + name: "telemetry enabled client timeout (but gated off)", + opts: &Options{DisableAuthentication: true}, + errorType: "timeout", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + // NO rpc.system, exception.type, error.type + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr}, + }, + { + name: "telemetry enabled client cancelled (but gated off)", + opts: &Options{DisableAuthentication: true}, + errorType: "cancel", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + // NO rpc.system, exception.type, error.type + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr}, + }, + { + name: "telemetry enabled client connection error (but gated off)", + opts: &Options{DisableAuthentication: true}, + errorType: "connection", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "HTTP GET", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + }, + Attributes: []attribute.KeyValue{ + keyHTTPRequestMetod.String(valHTTPGet), + // NO rpc.system, exception.type, error.type + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerAddr}, + }, { name: "telemetry disabled", opts: &Options{ @@ -439,10 +580,19 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { exporter.Reset() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(tt.statusCode) + if tt.errorType == "timeout" { + time.Sleep(100 * time.Millisecond) + } + if tt.statusCode != 0 { + w.WriteHeader(tt.statusCode) + } })) defer server.Close() + if tt.errorType == "connection" { + server.Close() + } + tt.opts.Endpoint = server.URL client, err := NewClient(tt.opts) if err != nil { @@ -450,6 +600,15 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { } ctx := context.Background() + var cancel context.CancelFunc + if tt.errorType == "timeout" { + ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) + defer cancel() + } else if tt.errorType == "cancel" { + ctx, cancel = context.WithCancel(ctx) + cancel() + } + for k, v := range tt.telemetryCtxValues { ctx = callctx.WithTelemetryContext(ctx, k, v) } @@ -460,10 +619,12 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { } resp, err := client.Do(req) - if err != nil { - t.Fatalf("client.Do() = %v, want nil", err) + if (err != nil) != tt.wantErr { + t.Errorf("client.Do() error = %v, wantErr %v", err, tt.wantErr) + } + if resp != nil && resp.Body != nil { + resp.Body.Close() } - resp.Body.Close() spans := exporter.GetSpans() if len(spans) != tt.wantSpans { @@ -478,7 +639,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status, cmpopts.IgnoreFields(sdktrace.Status{}, "Description")); diff != "" { t.Errorf("span.Status mismatch (-want +got):\n%s", diff) } diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index f9aa1141cb99..4c8769c9b7d7 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -17,6 +17,7 @@ package httptransport import ( "context" "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -227,16 +228,29 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e if span.IsRecording() { if err != nil { + var errorType string + switch { + case errors.Is(err, context.DeadlineExceeded): + errorType = "CLIENT_TIMEOUT" + case errors.Is(err, context.Canceled): + errorType = "CLIENT_CANCELLED" + default: + errorType = "CLIENT_CONNECTION_ERROR" + } span.SetAttributes( - attribute.String("error.type", "ERROR"), + attribute.String("error.type", errorType), attribute.String("status.message", err.Error()), attribute.String("exception.type", fmt.Sprintf("%T", err)), ) - } else if resp.StatusCode >= 400 { - span.SetAttributes( - attribute.String("error.type", strconv.Itoa(resp.StatusCode)), - attribute.String("status.message", resp.Status), - ) + } else { + span.SetAttributes(attribute.Int("http.response.status_code", resp.StatusCode)) + if resp.StatusCode >= 400 { + errorType := strconv.Itoa(resp.StatusCode) + span.SetAttributes( + attribute.String("error.type", errorType), + attribute.String("status.message", resp.Status), + ) + } } } From 40572b0ca52509b24a0e92ec936e105130c56609 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 10 Mar 2026 16:34:04 -0600 Subject: [PATCH 14/18] goimports -w . --- auth/httptransport/httptransport_otel_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/httptransport/httptransport_otel_test.go b/auth/httptransport/httptransport_otel_test.go index 7aa12c62ad7c..843c491cfe5b 100644 --- a/auth/httptransport/httptransport_otel_test.go +++ b/auth/httptransport/httptransport_otel_test.go @@ -172,7 +172,7 @@ func TestNewClient_OpenTelemetry_Enabled(t *testing.T) { Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ - Code: codes.Error, + Code: codes.Error, }, Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), @@ -473,7 +473,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ - Code: codes.Error, + Code: codes.Error, }, Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), @@ -492,7 +492,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ - Code: codes.Error, + Code: codes.Error, }, Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), @@ -511,7 +511,7 @@ func TestNewClient_OpenTelemetry_Disabled(t *testing.T) { Name: "HTTP GET", SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ - Code: codes.Error, + Code: codes.Error, }, Attributes: []attribute.KeyValue{ keyHTTPRequestMetod.String(valHTTPGet), From dec02c869eead876461a8ecd5f58b3bb363987ab Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 11 Mar 2026 10:37:04 -0600 Subject: [PATCH 15/18] delegate staticAttrs to NewTransport --- auth/httptransport/transport.go | 12 ++++++------ auth/internal/transport/transport_test.go | 5 +++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 4c8769c9b7d7..2ece1ea360eb 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -187,17 +187,18 @@ func addOpenTelemetryTransport(trans http.RoundTripper, opts *Options) http.Roun if opts.InternalOptions != nil { staticAttrs = transport.StaticTelemetryAttributes(opts.InternalOptions.TelemetryAttributes) } + otelOpts := []otelhttp.Option{ + otelhttp.WithSpanOptions(trace.WithAttributes(staticAttrs...)), + } return otelhttp.NewTransport(&otelAttributeTransport{ - base: trans, - staticAttrs: staticAttrs, - }) + base: trans, + }, otelOpts...) } // otelAttributeTransport is a wrapper around an http.RoundTripper that adds // custom Google Cloud-specific attributes to OpenTelemetry spans. type otelAttributeTransport struct { - base http.RoundTripper - staticAttrs []attribute.KeyValue + base http.RoundTripper } // RoundTrip intercepts the HTTP request and response to enrich the active @@ -207,7 +208,6 @@ func (t *otelAttributeTransport) RoundTrip(req *http.Request) (*http.Response, e span := trace.SpanFromContext(req.Context()) if span.IsRecording() { var attrs []attribute.KeyValue - attrs = append(attrs, t.staticAttrs...) attrs = append(attrs, attribute.String("rpc.system.name", "http")) if resName, ok := callctx.TelemetryFromContext(req.Context(), "resource_name"); ok { attrs = append(attrs, attribute.String("gcp.resource.destination.id", resName)) diff --git a/auth/internal/transport/transport_test.go b/auth/internal/transport/transport_test.go index 798417e0268a..fb4db0205ad3 100644 --- a/auth/internal/transport/transport_test.go +++ b/auth/internal/transport/transport_test.go @@ -114,6 +114,11 @@ func TestCloneDetectOptions(t *testing.T) { } func TestStaticTelemetryAttributes_KnownKeys(t *testing.T) { + // Existing keys are used by generated code and cannot be removed or modified. + // New keys may be added, but they will need to also be added to + // auth/internal/transport/transport.go and transport wrappers + // auth/httptransport/transport.go and auth/grpctransport/grpctransport.go + // before they will appear in telemetry. want := []string{ "gcp.client.service", "gcp.client.version", From 70cbdedac2f846a1b36373108a2245d52c3cd845 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 11 Mar 2026 14:44:01 -0600 Subject: [PATCH 16/18] improve error handling in grpc HandleRPC --- auth/grpctransport/grpctransport.go | 12 +- auth/grpctransport/grpctransport_otel_test.go | 142 +++++++++++++++++- 2 files changed, 151 insertions(+), 3 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 2634409dfdb7..edf854110202 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -39,6 +39,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" + "google.golang.org/grpc/codes" grpccreds "google.golang.org/grpc/credentials" grpcinsecure "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/stats" @@ -501,8 +502,17 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { if end.Error != nil { // Extract status code and message st, _ := status.FromError(end.Error) + var errorType string + switch st.Code() { + case codes.DeadlineExceeded: + errorType = "CLIENT_TIMEOUT" + case codes.Canceled: + errorType = "CLIENT_CANCELLED" + default: + errorType = strings.ToUpper(st.Code().String()) + } attrs = []attribute.KeyValue{ - attribute.String("error.type", strings.ToUpper(st.Code().String())), + attribute.String("error.type", errorType), attribute.String("status.message", st.Message()), attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index d27eb1270e26..fa33ed81de8b 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -19,8 +19,10 @@ import ( "net" "net/http" "testing" + "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/googleapis/gax-go/v2" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -77,12 +79,24 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { return nil, status.Error(grpccodes.Internal, "test error") }, } + timeoutEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil + }, + } + cancelEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + return nil, status.Error(grpccodes.Canceled, "context canceled") + }, + } tests := []struct { name string echoer echo.EchoerServer opts *Options telemetryCtxValues map[string]string + errorType string // "timeout", "cancel" wantErr bool wantSpans int wantSpan sdktrace.ReadOnlySpan @@ -135,6 +149,58 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, }, + { + name: "telemetry enabled client timeout", + echoer: timeoutEchoer, + opts: &Options{DisableAuthentication: true}, + errorType: "timeout", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "rpc error: code = DeadlineExceeded desc = context deadline exceeded", + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(4), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + attribute.String("error.type", "CLIENT_TIMEOUT"), + attribute.String("rpc.response.status_code", "DEADLINEEXCEEDED"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort, attribute.Key("status.message"), attribute.Key("exception.type")}, + }, + { + name: "telemetry enabled client cancelled", + echoer: cancelEchoer, + opts: &Options{DisableAuthentication: true}, + errorType: "cancel", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "rpc error: code = Canceled desc = context canceled", + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(1), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + attribute.String("error.type", "CLIENT_CANCELLED"), + attribute.String("rpc.response.status_code", "CANCELED"), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort, attribute.Key("status.message"), attribute.Key("exception.type")}, + }, { name: "telemetry disabled", echoer: successfulEchoer, @@ -212,6 +278,12 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { defer pool.Close() ctx := context.Background() + var cancel context.CancelFunc + if tt.errorType == "timeout" { + ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) + defer cancel() + } + for k, v := range tt.telemetryCtxValues { ctx = callctx.WithTelemetryContext(ctx, k, v) } @@ -235,7 +307,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status, cmpopts.IgnoreFields(sdktrace.Status{}, "Description")); diff != "" { t.Errorf("span.Status mismatch (-want +got):\n%s", diff) } @@ -292,12 +364,24 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { return nil, status.Error(grpccodes.Internal, "test error") }, } + timeoutEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil + }, + } + cancelEchoer := &fakeEchoService{ + Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { + return nil, status.Error(grpccodes.Canceled, "context canceled") + }, + } tests := []struct { name string echoer echo.EchoerServer opts *Options telemetryCtxValues map[string]string + errorType string // "timeout", "cancel" wantErr bool wantSpans int wantSpan sdktrace.ReadOnlySpan @@ -348,6 +432,54 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort}, }, + { + name: "telemetry enabled client timeout (but gated off)", + echoer: timeoutEchoer, + opts: &Options{DisableAuthentication: true}, + errorType: "timeout", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "rpc error: code = DeadlineExceeded desc = context deadline exceeded", + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(4), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, + { + name: "telemetry enabled client cancelled (but gated off)", + echoer: cancelEchoer, + opts: &Options{DisableAuthentication: true}, + errorType: "cancel", + wantErr: true, + wantSpans: 1, + wantSpan: tracetest.SpanStub{ + Name: "echo.Echoer/Echo", + SpanKind: oteltrace.SpanKindClient, + Status: sdktrace.Status{ + Code: codes.Error, + Description: "rpc error: code = Canceled desc = context canceled", + }, + Attributes: []attribute.KeyValue{ + keyRPCStatusCode.Int64(1), + keyRPCMethod.String("Echo"), + keyRPCService.String("echo.Echoer"), + keyRPCSystem.String(valRPCSystemGRPC), + keyServerAddr.String(valLocalhost), + }, + }.Snapshot(), + wantAttrKeys: []attribute.Key{keyServerPort}, + }, { name: "telemetry disabled", echoer: successfulEchoer, @@ -413,6 +545,12 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { defer pool.Close() ctx := context.Background() + var cancel context.CancelFunc + if tt.errorType == "timeout" { + ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) + defer cancel() + } + for k, v := range tt.telemetryCtxValues { ctx = callctx.WithTelemetryContext(ctx, k, v) } @@ -436,7 +574,7 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { if diff := cmp.Diff(tt.wantSpan.SpanKind(), span.SpanKind); diff != "" { t.Errorf("span.SpanKind mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff(tt.wantSpan.Status(), span.Status); diff != "" { + if diff := cmp.Diff(tt.wantSpan.Status(), span.Status, cmpopts.IgnoreFields(sdktrace.Status{}, "Description")); diff != "" { t.Errorf("span.Status mismatch (-want +got):\n%s", diff) } From 9f6ee63536497e579e1eafb97421d371ecfbcc28 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 12 Mar 2026 10:50:42 -0600 Subject: [PATCH 17/18] fix usage of status.FromError --- auth/grpctransport/grpctransport.go | 64 ++++++++++++++++--- auth/grpctransport/grpctransport_otel_test.go | 26 ++++---- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index edf854110202..f9077bb558a8 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -25,7 +25,6 @@ import ( "net/http" "os" "strconv" - "strings" "cloud.google.com/go/auth" "cloud.google.com/go/auth/credentials" @@ -56,6 +55,30 @@ const ( quotaProjectHeaderKey = "X-goog-user-project" ) +// codeToStr is a reversal of the `strToCode` map in +// https://github.com/grpc/grpc-go/blob/master/codes/codes.go +// The gRPC specification has exactly 17 status codes, defined +// as a contiguous block of integers from 0 to 16. +var codeToStr = [...]string{ + "OK", // codes.OK = 0 + "CANCELED", // codes.Canceled = 1 + "UNKNOWN", // codes.Unknown = 2 + "INVALID_ARGUMENT", // codes.InvalidArgument = 3 + "DEADLINE_EXCEEDED", // codes.DeadlineExceeded = 4 + "NOT_FOUND", // codes.NotFound = 5 + "ALREADY_EXISTS", // codes.AlreadyExists = 6 + "PERMISSION_DENIED", // codes.PermissionDenied = 7 + "RESOURCE_EXHAUSTED", // codes.ResourceExhausted = 8 + "FAILED_PRECONDITION", // codes.FailedPrecondition = 9 + "ABORTED", // codes.Aborted = 10 + "OUT_OF_RANGE", // codes.OutOfRange = 11 + "UNIMPLEMENTED", // codes.Unimplemented = 12 + "INTERNAL", // codes.Internal = 13 + "UNAVAILABLE", // codes.Unavailable = 14 + "DATA_LOSS", // codes.DataLoss = 15 + "UNAUTHENTICATED", // codes.Unauthenticated = 16 +} + var ( // Set at init time by dial_socketopt.go. If nil, socketopt is not supported. timeoutDialerOption grpc.DialOption @@ -500,21 +523,33 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { var attrs []attribute.KeyValue if end.Error != nil { - // Extract status code and message - st, _ := status.FromError(end.Error) + st, ok := status.FromError(end.Error) + statusCodeStr := codeToCanonicalStr(st.Code()) + var errorType string - switch st.Code() { - case codes.DeadlineExceeded: + // 1. Check for specific local transport breakdowns by interrogating the + // entire error chain. This catches both raw context errors and those + // wrapped by the gRPC library. + if errors.Is(end.Error, context.DeadlineExceeded) { errorType = "CLIENT_TIMEOUT" - case codes.Canceled: + } else if errors.Is(end.Error, context.Canceled) { errorType = "CLIENT_CANCELLED" - default: - errorType = strings.ToUpper(st.Code().String()) + } else if !ok || st.Code() == codes.Unknown || st.Code() == codes.Internal { + // 2. If the error isn't a context breakdown and the gRPC framework + // doesn't "understand" it (returning ok=false or a generic catch-all + // bucket like Unknown/Internal), we "pack" the actual Go error type + // name into error.type for high-fidelity debugging (e.g., "*net.OpError"). + errorType = fmt.Sprintf("%T", end.Error) + } else { + // 3. Otherwise, it is a well-understood gRPC protocol error (e.g., + // PERMISSION_DENIED) likely returned by the server. + errorType = statusCodeStr } + attrs = []attribute.KeyValue{ attribute.String("error.type", errorType), attribute.String("status.message", st.Message()), - attribute.String("rpc.response.status_code", strings.ToUpper(st.Code().String())), + attribute.String("rpc.response.status_code", statusCodeStr), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), } } else { @@ -525,3 +560,14 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { span.SetAttributes(attrs...) h.Handler.HandleRPC(ctx, s) } + +// codeToCanonicalStr returns the canonical name for each of the 17 gRPC +// status codes defined in https://github.com/grpc/grpc-go/blob/master/codes/codes.go. +// For any codes.Code that converts to an out-of-bounds int, +// it returns "UNKNOWN". +func codeToCanonicalStr(code codes.Code) string { + if int(code) >= 0 && int(code) < len(codeToStr) { + return codeToStr[code] + } + return "UNKNOWN" +} diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index fa33ed81de8b..bec26b2668b8 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -81,13 +81,12 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { } timeoutEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - time.Sleep(100 * time.Millisecond) - return &echo.EchoReply{Message: req.Message}, nil + return nil, context.DeadlineExceeded }, } cancelEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, status.Error(grpccodes.Canceled, "context canceled") + return nil, context.Canceled }, } @@ -142,7 +141,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("error.type", "INTERNAL"), + attribute.String("error.type", "*status.Error"), attribute.String("status.message", "test error"), attribute.String("rpc.response.status_code", "INTERNAL"), }, @@ -161,7 +160,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ Code: codes.Error, - Description: "rpc error: code = DeadlineExceeded desc = context deadline exceeded", + Description: "context deadline exceeded", }, Attributes: []attribute.KeyValue{ keyRPCStatusCode.Int64(4), @@ -169,8 +168,8 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("error.type", "CLIENT_TIMEOUT"), - attribute.String("rpc.response.status_code", "DEADLINEEXCEEDED"), + attribute.String("error.type", "DEADLINE_EXCEEDED"), + attribute.String("rpc.response.status_code", "DEADLINE_EXCEEDED"), }, }.Snapshot(), wantAttrKeys: []attribute.Key{keyServerPort, attribute.Key("status.message"), attribute.Key("exception.type")}, @@ -187,7 +186,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ Code: codes.Error, - Description: "rpc error: code = Canceled desc = context canceled", + Description: "context canceled", }, Attributes: []attribute.KeyValue{ keyRPCStatusCode.Int64(1), @@ -195,7 +194,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("error.type", "CLIENT_CANCELLED"), + attribute.String("error.type", "CANCELED"), attribute.String("rpc.response.status_code", "CANCELED"), }, }.Snapshot(), @@ -366,13 +365,12 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { } timeoutEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - time.Sleep(100 * time.Millisecond) - return &echo.EchoReply{Message: req.Message}, nil + return nil, context.DeadlineExceeded }, } cancelEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, status.Error(grpccodes.Canceled, "context canceled") + return nil, context.Canceled }, } @@ -444,7 +442,7 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ Code: codes.Error, - Description: "rpc error: code = DeadlineExceeded desc = context deadline exceeded", + Description: "context deadline exceeded", }, Attributes: []attribute.KeyValue{ keyRPCStatusCode.Int64(4), @@ -468,7 +466,7 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { SpanKind: oteltrace.SpanKindClient, Status: sdktrace.Status{ Code: codes.Error, - Description: "rpc error: code = Canceled desc = context canceled", + Description: "context canceled", }, Attributes: []attribute.KeyValue{ keyRPCStatusCode.Int64(1), From c92ecffbe0ec5599bbdb875b336ffe85855f91c0 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 12 Mar 2026 14:37:24 -0600 Subject: [PATCH 18/18] check local ctx.Err() first for grpc error.type --- auth/grpctransport/grpctransport.go | 21 ++++++++++-------- auth/grpctransport/grpctransport_otel_test.go | 22 ++++++++++++++----- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index f9077bb558a8..1792c47286d3 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -524,32 +524,35 @@ func (h *otelHandler) HandleRPC(ctx context.Context, s stats.RPCStats) { var attrs []attribute.KeyValue if end.Error != nil { st, ok := status.FromError(end.Error) - statusCodeStr := codeToCanonicalStr(st.Code()) + rpcStatusCode := codeToCanonicalStr(st.Code()) var errorType string - // 1. Check for specific local transport breakdowns by interrogating the - // entire error chain. This catches both raw context errors and those - // wrapped by the gRPC library. - if errors.Is(end.Error, context.DeadlineExceeded) { + // 1. Check if the local context expired or was cancelled. This is the only + // reliable way to distinguish a local client timeout from a server timeout + // because gRPC does not wrap context errors in its status.Error types. + if errors.Is(ctx.Err(), context.DeadlineExceeded) { errorType = "CLIENT_TIMEOUT" - } else if errors.Is(end.Error, context.Canceled) { + } else if errors.Is(ctx.Err(), context.Canceled) { errorType = "CLIENT_CANCELLED" } else if !ok || st.Code() == codes.Unknown || st.Code() == codes.Internal { // 2. If the error isn't a context breakdown and the gRPC framework // doesn't "understand" it (returning ok=false or a generic catch-all // bucket like Unknown/Internal), we "pack" the actual Go error type - // name into error.type for high-fidelity debugging (e.g., "*net.OpError"). + // name into error.type (e.g., "*net.OpError"). This is per the error.type + // [spec](https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/#error-type). + // "When error.type is set to a type (e.g., an exception type), its canonical + // class name identifying the type within the artifact SHOULD be used." errorType = fmt.Sprintf("%T", end.Error) } else { // 3. Otherwise, it is a well-understood gRPC protocol error (e.g., // PERMISSION_DENIED) likely returned by the server. - errorType = statusCodeStr + errorType = rpcStatusCode } attrs = []attribute.KeyValue{ attribute.String("error.type", errorType), attribute.String("status.message", st.Message()), - attribute.String("rpc.response.status_code", statusCodeStr), + attribute.String("rpc.response.status_code", rpcStatusCode), attribute.String("exception.type", fmt.Sprintf("%T", end.Error)), } } else { diff --git a/auth/grpctransport/grpctransport_otel_test.go b/auth/grpctransport/grpctransport_otel_test.go index bec26b2668b8..c47ca9c2e435 100644 --- a/auth/grpctransport/grpctransport_otel_test.go +++ b/auth/grpctransport/grpctransport_otel_test.go @@ -81,12 +81,14 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { } timeoutEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, context.DeadlineExceeded + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil }, } cancelEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, context.Canceled + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil }, } @@ -168,7 +170,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("error.type", "DEADLINE_EXCEEDED"), + attribute.String("error.type", "CLIENT_TIMEOUT"), attribute.String("rpc.response.status_code", "DEADLINE_EXCEEDED"), }, }.Snapshot(), @@ -194,7 +196,7 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { keyRPCService.String("echo.Echoer"), keyRPCSystem.String(valRPCSystemGRPC), keyServerAddr.String(valLocalhost), - attribute.String("error.type", "CANCELED"), + attribute.String("error.type", "CLIENT_CANCELLED"), attribute.String("rpc.response.status_code", "CANCELED"), }, }.Snapshot(), @@ -281,6 +283,9 @@ func TestDial_OpenTelemetry_Enabled(t *testing.T) { if tt.errorType == "timeout" { ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) defer cancel() + } else if tt.errorType == "cancel" { + ctx, cancel = context.WithCancel(ctx) + time.AfterFunc(10*time.Millisecond, cancel) } for k, v := range tt.telemetryCtxValues { @@ -365,12 +370,14 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { } timeoutEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, context.DeadlineExceeded + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil }, } cancelEchoer := &fakeEchoService{ Fn: func(ctx context.Context, req *echo.EchoRequest) (*echo.EchoReply, error) { - return nil, context.Canceled + time.Sleep(100 * time.Millisecond) + return &echo.EchoReply{Message: req.Message}, nil }, } @@ -547,6 +554,9 @@ func TestDial_OpenTelemetry_Disabled(t *testing.T) { if tt.errorType == "timeout" { ctx, cancel = context.WithTimeout(ctx, 10*time.Millisecond) defer cancel() + } else if tt.errorType == "cancel" { + ctx, cancel = context.WithCancel(ctx) + time.AfterFunc(10*time.Millisecond, cancel) } for k, v := range tt.telemetryCtxValues {