otelgrpc: only start span if not filtered out

CHANGELOG.md

@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Fixed
 
 - Use `context.Background()` as default context instead of nil in `go.opentelemetry.io/contrib/bridges/otellogr`. (#6527)
+- Don't start spans that never end for filtered out gRPC stats handler in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#6695)
 
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->

instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go

@@ -59,20 +59,26 @@ func (h *serverHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) cont
 
 	name, attrs := internal.ParseFullMethod(info.FullMethodName)
 	attrs = append(attrs, RPCSystemGRPC)
-	ctx, _ = h.tracer.Start(
-		trace.ContextWithRemoteSpanContext(ctx, trace.SpanContextFromContext(ctx)),
-		name,
-		trace.WithSpanKind(trace.SpanKindServer),
-		trace.WithAttributes(append(attrs, h.config.SpanAttributes...)...),
-	)
+
+	record := true
+	if h.config.Filter != nil {
+		record = h.config.Filter(info)
+	}
+
+	if record {
+		ctx, _ = h.tracer.Start(
+			trace.ContextWithRemoteSpanContext(ctx, trace.SpanContextFromContext(ctx)),
+			name,
+			trace.WithSpanKind(trace.SpanKindServer),
+			trace.WithAttributes(append(attrs, h.config.SpanAttributes...)...),
+		)
+	}
 
 	gctx := gRPCContext{
 		metricAttrs: append(attrs, h.config.MetricAttributes...),
-		record:      true,
-	}
-	if h.config.Filter != nil {
-		gctx.record = h.config.Filter(info)
+		record:      record,
 	}
+
 	return context.WithValue(ctx, gRPCContextKey{}, &gctx)
 }
 
@@ -99,19 +105,24 @@ func NewClientHandler(opts ...Option) stats.Handler {
 func (h *clientHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context {
 	name, attrs := internal.ParseFullMethod(info.FullMethodName)
 	attrs = append(attrs, RPCSystemGRPC)
-	ctx, _ = h.tracer.Start(
-		ctx,
-		name,
-		trace.WithSpanKind(trace.SpanKindClient),
-		trace.WithAttributes(append(attrs, h.config.SpanAttributes...)...),
-	)
+
+	record := true
+	if h.config.Filter != nil {
+		record = h.config.Filter(info)
+	}
+
+	if record {
+		ctx, _ = h.tracer.Start(
+			ctx,
+			name,
+			trace.WithSpanKind(trace.SpanKindClient),
+			trace.WithAttributes(append(attrs, h.config.SpanAttributes...)...),
+		)
+	}
 
 	gctx := gRPCContext{
 		metricAttrs: append(attrs, h.config.MetricAttributes...),
-		record:      true,
-	}
-	if h.config.Filter != nil {
-		gctx.record = h.config.Filter(info)
+		record:      record,
 	}
 
 	return inject(context.WithValue(ctx, gRPCContextKey{}, &gctx), h.config.Propagators)

instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go

@@ -17,6 +17,7 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	testpb "google.golang.org/grpc/interop/grpc_testing"
+	"google.golang.org/grpc/stats"
 	"google.golang.org/grpc/status"
 
 	"go.opentelemetry.io/otel/attribute"
@@ -27,6 +28,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/sdk/trace/tracetest"
 	semconv "go.opentelemetry.io/otel/semconv/v1.17.0"
+	oteltrace "go.opentelemetry.io/otel/trace"
 
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/filters"
@@ -1536,3 +1538,111 @@ func TestStatsHandlerConcurrentSafeContextCancellation(t *testing.T) {
 		wg.Wait()
 	}
 }
+
+func TestServerHandlerTagRPC(t *testing.T) {
+	tests := []struct {
+		name   string
+		server stats.Handler
+		ctx    context.Context
+		info   *stats.RPCTagInfo
+		exp    bool
+	}{
+		{
+			name:   "start a span without filters",
+			server: otelgrpc.NewServerHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider())),
+			ctx:    context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/grpc.health.v1.Health/Check",
+			},
+			exp: true,
+		},
+		{
+			name: "don't start a span with filter and match",
+			server: otelgrpc.NewServerHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider()), otelgrpc.WithFilter(func(ri *stats.RPCTagInfo) bool {
+				return ri.FullMethodName != "/grpc.health.v1.Health/Check"
+			})),
+			ctx: context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/grpc.health.v1.Health/Check",
+			},
+			exp: false,
+		},
+		{
+			name: "start a span with filter and no match",
+			server: otelgrpc.NewServerHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider()), otelgrpc.WithFilter(func(ri *stats.RPCTagInfo) bool {
+				return ri.FullMethodName != "/grpc.health.v1.Health/Check"
+			})),
+			ctx: context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/app.v1.Service/Get",
+			},
+			exp: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(t.Name(), func(t *testing.T) {
+			ctx := tt.server.TagRPC(tt.ctx, tt.info)
+
+			got := oteltrace.SpanFromContext(ctx).IsRecording()
+
+			if tt.exp != got {
+				t.Errorf("expected %t, got %t", tt.exp, got)
+			}
+		})
+	}
+}
+
+func TestClientHandlerTagRPC(t *testing.T) {
+	tests := []struct {
+		name   string
+		client stats.Handler
+		ctx    context.Context
+		info   *stats.RPCTagInfo
+		exp    bool
+	}{
+		{
+			name:   "start a span without filters",
+			client: otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider())),
+			ctx:    context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/grpc.health.v1.Health/Check",
+			},
+			exp: true,
+		},
+		{
+			name: "don't start a span with filter and match",
+			client: otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider()), otelgrpc.WithFilter(func(ri *stats.RPCTagInfo) bool {
+				return ri.FullMethodName != "/grpc.health.v1.Health/Check"
+			})),
+			ctx: context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/grpc.health.v1.Health/Check",
+			},
+			exp: false,
+		},
+		{
+			name: "start a span with filter and no match",
+			client: otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(trace.NewTracerProvider()), otelgrpc.WithFilter(func(ri *stats.RPCTagInfo) bool {
+				return ri.FullMethodName != "/grpc.health.v1.Health/Check"
+			})),
+			ctx: context.Background(),
+			info: &stats.RPCTagInfo{
+				FullMethodName: "/app.v1.Service/Get",
+			},
+			exp: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(t.Name(), func(t *testing.T) {
+			ctx := tt.client.TagRPC(tt.ctx, tt.info)
+
+			got := oteltrace.SpanFromContext(ctx).IsRecording()
+
+			if tt.exp != got {
+				t.Errorf("expected %t, got %t", tt.exp, got)
+			}
+		})
+	}
+}