addressing review comments

stats/opentelemetry/client_metrics.go

@@ -67,7 +67,6 @@ func (h *clientStatsHandler) initializeMetrics() {
 	rm.registerMetrics(metrics, meter)
 }
 
-// unaryInterceptor records metrics for unary RPC calls.
 func (h *clientStatsHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
 	ci := getCallInfo(ctx)
 	if ci == nil {
@@ -106,7 +105,6 @@ func determineMethod(method string, opts ...grpc.CallOption) string {
 	return "other"
 }
 
-// streamInterceptor records metrics for streaming RPC calls.
 func (h *clientStatsHandler) streamInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
 	ci := getCallInfo(ctx)
 	if ci == nil {
@@ -233,7 +231,7 @@ func (h *clientStatsHandler) setLabelsFromPluginOption(ai *attemptInfo, incoming
 func (h *clientStatsHandler) processRPCEnd(ctx context.Context, ai *attemptInfo, e *stats.End) {
 	ci := getCallInfo(ctx)
 	if ci == nil {
-		logger.Info("ctx passed into client side stats handler metrics event handling has no metrics data present")
+		logger.Error("ctx passed into client side stats handler metrics event handling has no metrics data present")
 		return
 	}
 	latency := float64(time.Since(ai.startTime)) / float64(time.Second)

stats/opentelemetry/client_tracing.go

@@ -20,11 +20,13 @@
 	"context"
 	"log"
 	"strings"
+	"time"
 
 	otelcodes "go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc"
 	grpccodes "google.golang.org/grpc/codes"
+	istats "google.golang.org/grpc/internal/stats"
 	"google.golang.org/grpc/stats"
 	otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
 	"google.golang.org/grpc/status"
@@ -38,23 +40,21 @@
 
 func (h *clientTracingHandler) initializeTraces() {
 	if h.options.TraceOptions.TracerProvider == nil {
-		log.Printf("TraceProvider is not provided in client trace options")
+		log.Printf("TracerProvider is not provided in client TraceOptions")
 		return
 	}
-	h.options.TraceOptions.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(grpc.Version))
 }
 
-// unaryInterceptor records traces for unary RPC calls.
 func (h *clientTracingHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
 	ci := getCallInfo(ctx)
 	if ci == nil {
-		logger.Info("ctx passed into client tracing handler unary interceptor has no call info data present")
+		logger.Info("callInfo not present in context in clientTracingHandler unaryInterceptor")
 		ci = &callInfo{
 			target: cc.CanonicalTarget(),
 			method: determineMethod(method, opts...),
 		}
 		ctx = setCallInfo(ctx, ci)
 	}
 
 	var span trace.Span
 	ctx, span = h.createCallTraceSpan(ctx, method)
@@ -63,23 +63,20 @@
 	return err
 }
 
-// streamInterceptor records traces for streaming RPC calls.
 func (h *clientTracingHandler) streamInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
 	ci := getCallInfo(ctx)
 	if ci == nil {
-		logger.Info("ctx passed into client tracing handler stream interceptor has no call info data present")
+		logger.Info("callInfo not present in context in clientTracingHandler streamInterceptor")
 		ci = &callInfo{
 			target: cc.CanonicalTarget(),
 			method: determineMethod(method, opts...),
 		}
 		ctx = setCallInfo(ctx, ci)
 	}
 
 	var span trace.Span
 	ctx, span = h.createCallTraceSpan(ctx, method)
-	callback := func(err error) {
-		h.perCallTraces(err, span)
-	}
+	callback := func(err error) { h.perCallTraces(err, span) }
 	opts = append([]grpc.CallOption{grpc.OnFinish(callback)}, opts...)
 	return streamer(ctx, desc, cc, method, opts...)
 }
@@ -128,16 +125,29 @@
 func (h *clientTracingHandler) HandleConn(context.Context, stats.ConnStats) {}
 
 // TagRPC implements per RPC attempt context management for traces.
-func (h *clientTracingHandler) TagRPC(ctx context.Context, _ *stats.RPCTagInfo) context.Context {
-	// Fetch the rpcInfo set by a previously registered stats handler
-	// (like clientStatsHandler). Assumes this handler runs after one
-	// that sets the rpcInfo in the context.
+func (h *clientTracingHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context {
+	// Fetch the rpcInfo set by a previously registered stats handler.
 	ri := getRPCInfo(ctx)
-	if ri == nil {
-		logger.Info("ctx passed into client side tracing stats handler has no client attempt data present")
-		return ctx
+	var ai *attemptInfo
+	if ri != nil {
+		ai = ri.ai
+	} else {
+		labels := istats.GetLabels(ctx)
+		if labels == nil {
+			labels = &istats.Labels{
+				TelemetryLabels: map[string]string{
+					"grpc.lb.locality": "",
+				},
+			}
+			ctx = istats.SetLabels(ctx, labels)
+		}
+		ai = &attemptInfo{
+			startTime: time.Now(),
+			xdsLabels: labels.TelemetryLabels,
+			method:    removeLeadingSlash(info.FullMethodName),
+		}
 	}
-	ctx, ai := h.traceTagRPC(ctx, ri.ai)
+	ctx, ai = h.traceTagRPC(ctx, ai)
 	return setRPCInfo(ctx, &rpcInfo{ai: ai})
 }
 
@@ -148,8 +158,8 @@
 	// that sets the rpcInfo in the context.
 	ri := getRPCInfo(ctx)
 	if ri == nil {
 		logger.Error("ctx passed into client side tracing stats handler has no client attempt data present")
 		return
 	}
 	populateSpan(rs, ri.ai)
 }

stats/opentelemetry/opentelemetry.go

@@ -114,8 +114,8 @@ type MetricsOptions struct {
 // configured for an individual metric turned on, the API call in this component
 // will create a default view for that metric.
 //
-// For tracing, provide a TracerProvider in trace options.
-// If no TracerProvider is provided, tracing will be no-op.
+// For the traces supported by this instrumentation code, provide an
+// implementation of a TextMapPropagator and OpenTelemetry TracerProvider.
 func DialOption(o Options) grpc.DialOption {
 	csh := &clientStatsHandler{options: o}
 	csh.initializeMetrics()
@@ -143,8 +143,8 @@ var joinServerOptions = internal.JoinServerOptions.(func(...grpc.ServerOption) g
 // configured for an individual metric turned on, the API call in this component
 // will create a default view for that metric.
 //
-// For tracing, provide a TracerProvider in trace options.
-// If no TracerProvider is provided, tracing will be no-op.
+// For the traces supported by this instrumentation code, provide an
+// implementation of a TextMapPropagator and OpenTelemetry TracerProvider.
 func ServerOption(o Options) grpc.ServerOption {
 	ssh := &serverStatsHandler{options: o}
 	ssh.initializeMetrics()

stats/opentelemetry/server_metrics.go

@@ -90,7 +90,6 @@ func (s *attachLabelsTransportStream) SendHeader(md metadata.MD) error {
 	return s.ServerTransportStream.SendHeader(md)
 }
 
-// unaryInterceptor records metrics for unary RPC calls.
 func (h *serverStatsHandler) unaryInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
 	var metadataExchangeLabels metadata.MD
 	if h.options.MetricsOptions.pluginOption != nil {
@@ -152,7 +151,6 @@ func (s *attachLabelsStream) SendMsg(m any) error {
 	return s.ServerStream.SendMsg(m)
 }
 
-// streamInterceptor records metrics for streaming RPC calls.
 func (h *serverStatsHandler) streamInterceptor(srv any, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
 	var metadataExchangeLabels metadata.MD
 	if h.options.MetricsOptions.pluginOption != nil {

stats/opentelemetry/server_tracing.go

@@ -20,9 +20,11 @@
 	"context"
 	"log"
 	"strings"
+	"time"
 
 	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/stats"
 	otelinternaltracing "google.golang.org/grpc/stats/opentelemetry/internal/tracing"
 )
@@ -33,10 +35,9 @@
 
 func (h *serverTracingHandler) initializeTraces() {
 	if h.options.TraceOptions.TracerProvider == nil {
-		log.Printf("TraceProvider is not provided in server trace options")
+		log.Printf("TracerProvider is not provided in server TraceOptions")
 		return
 	}
-	h.options.TraceOptions.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(grpc.Version))
 }
 
 func (h *serverTracingHandler) unaryInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
@@ -48,16 +49,36 @@
 }
 
 // TagRPC implements per RPC attempt context management for traces.
-func (h *serverTracingHandler) TagRPC(ctx context.Context, _ *stats.RPCTagInfo) context.Context {
-	// Fetch the rpcInfo set by a previously registered stats handler
-	// (like serverStatsHandler). Assumes this handler runs after one
-	// that sets the rpcInfo in the context.
+func (h *serverTracingHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context {
+	// Fetch the rpcInfo set by a previously registered stats handler.
 	ri := getRPCInfo(ctx)
-	if ri == nil {
-		logger.Info("ctx passed into server side tracing stats handler has no server call data present")
-		return ctx
+	var ai *attemptInfo
+	if ri != nil {
+		ai = ri.ai
+	} else {
+		method := info.FullMethodName
+		if h.options.MetricsOptions.MethodAttributeFilter != nil {
+			if !h.options.MetricsOptions.MethodAttributeFilter(method) {
+				method = "other"
+			}
+		}
+		server := internal.ServerFromContext.(func(context.Context) *grpc.Server)(ctx)
+		if server == nil { // Shouldn't happen, defensive programming.
+			logger.Error("ctx passed into server side stats handler has no grpc server ref")
+			method = "other"
+		} else {
+			isRegisteredMethod := internal.IsRegisteredMethod.(func(*grpc.Server, string) bool)
+			if !isRegisteredMethod(server, method) {
+				method = "other"
+			}
+		}
+
+		ai = &attemptInfo{
+			startTime: time.Now(),
+			method:    removeLeadingSlash(method),
+		}
 	}
-	ctx, ai := h.traceTagRPC(ctx, ri.ai)
+	ctx, ai = h.traceTagRPC(ctx, ai)
 	return setRPCInfo(ctx, &rpcInfo{ai: ai})
 }
 
@@ -83,14 +104,12 @@
 
 // HandleRPC handles per-RPC attempt stats events for tracing.
 func (h *serverTracingHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
-	// Fetch the rpcInfo set by a previously registered stats handler
-	// (like serverStatsHandler). Assumes this handler runs after one
-	// that sets the rpcInfo in the context.
+	// Fetch the rpcInfo set by a previously registered stats handler.
 	ri := getRPCInfo(ctx)
 	if ri == nil {
 		logger.Error("ctx passed into server side tracing stats handler has no server call data present")
 		return
 	}
 	populateSpan(rs, ri.ai)
 }