fix: add null pointer check for gRPC stream client span (#287)

* fix: add null pointer check for gRPC stream client span * Update test/grpc/v1.44.0/test_grpc_client_stream.go Co-authored-by: Yi Yang <[email protected]> --------- Co-authored-by: Yi Yang <[email protected]>
alibaba · Jan 14, 2025 · 940ec10 · 940ec10
1 parent 5003682
commit 940ec10
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 10 deletions.
diff --git a/pkg/rules/grpc/grpc_config.go b/pkg/rules/grpc/grpc_config.go
@@ -94,15 +94,19 @@ func (c *grpcOtelConfig) handleRPC(ctx context.Context, rs stats.RPCStats, isSer
 			}
 
 		} else {
+			methodName := ""
+			if gctx != nil {
+				methodName = gctx.methodName
+			}
 			if isServer {
 				grpcServerInstrument.End(ctx, grpcRequest{
-					methodName: gctx.methodName,
+					methodName: methodName,
 				}, grpcResponse{
 					statusCode: 200,
 				}, nil)
 			} else {
 				grpcClientInstrument.End(ctx, grpcRequest{
-					methodName: gctx.methodName,
+					methodName: methodName,
 				}, grpcResponse{
 					statusCode: 200,
 				}, nil)

diff --git a/test/grpc/v1.44.0/grpc.pb.go b/test/grpc/v1.44.0/grpc.pb.go
diff --git a/test/grpc/v1.44.0/grpc.proto b/test/grpc/v1.44.0/grpc.proto
@@ -4,6 +4,7 @@ option go_package = "/pkgs";
 
 service HelloGrpc {
   rpc Hello(Req) returns (Resp) {}
+  rpc StreamHello(Req) returns (stream Resp) {}
 }
 
 message Req {

diff --git a/test/grpc/v1.44.0/grpc_common.go b/test/grpc/v1.44.0/grpc_common.go
@@ -19,6 +19,7 @@ import (
 	"errors"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
+	"io"
 	"log"
 	"net"
 )
@@ -34,6 +35,19 @@ func (s *service) Hello(ctx context.Context, req *Req) (*Resp, error) {
 	return &Resp{Message: "Hello Gprc"}, nil
 }
 
+func (s *service) StreamHello(req *Req, res HelloGrpc_StreamHelloServer) error {
+	if req.Error {
+		return errors.New("error Grpc")
+	}
+	for i := 0; i < 2; i++ {
+		err := res.Send(&Resp{Message: "Hello Gprc"})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func setupGRPC() {
 	lis, err := net.Listen("tcp", "0.0.0.0:9003")
 	if err != nil {
@@ -74,3 +88,25 @@ func sendErrReq(ctx context.Context) string {
 	}
 	return "error resp"
 }
+
+func sendStreamReq(ctx context.Context) string {
+	conn, err := grpc.NewClient("localhost:9003", grpc.WithTransportCredentials(insecure.NewCredentials()))
+	if err != nil {
+		panic(err)
+	}
+	c := NewHelloGrpcClient(conn)
+	stream, err := c.StreamHello(ctx, &Req{})
+	if err != nil {
+		panic(err)
+	}
+	for {
+		_, err := stream.Recv()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			panic(err)
+		}
+	}
+	return "stream success"
+}
diff --git a/test/grpc/v1.44.0/grpc_grpc.pb.go b/test/grpc/v1.44.0/grpc_grpc.pb.go
diff --git a/test/grpc/v1.44.0/test_grpc_client_stream.go b/test/grpc/v1.44.0/test_grpc_client_stream.go
@@ -0,0 +1,37 @@
+// Copyright (c) 2025 Alibaba Group Holding Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"context"
+	"github.com/alibaba/opentelemetry-go-auto-instrumentation/test/verifier"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
+	"go.opentelemetry.io/otel/trace"
+	"time"
+)
+
+func main() {
+	// starter server
+	go setupGRPC()
+	time.Sleep(3 * time.Second)
+	// use a http client to request to the server
+	sendStreamReq(context.Background())
+	// verify trace
+	verifier.WaitAndAssertTraces(func(stubs []tracetest.SpanStubs) {
+		// filter out client
+		verifier.Assert(len(stubs[0]) == 1, "Except client grpc system to be 1, got %d", len(stubs))
+		verifier.Assert(stubs[0][0].SpanKind == trace.SpanKindServer, "Except client grpc system to be server, got %v", stubs[0][0].SpanKind)
+	}, 1)
+}
diff --git a/test/grpc_tests.go b/test/grpc_tests.go
@@ -25,6 +25,7 @@ func init() {
 		NewGeneralTestCase("grpc-fail-status-test", grpc_module_name, "v1.44.0", "", "1.21", "", TestGrpcStatus),
 		NewLatestDepthTestCase("grpc-latest-depth", grpc_dependency_name, grpc_module_name, "v1.44.0", "v1.69.2", "1.21", "", TestBasicGrpc),
 		NewMuzzleTestCase("grpc-muzzle", grpc_dependency_name, grpc_module_name, "v1.44.0", "", "1.21", "", []string{"go", "build", "test_grpc_basic.go", "grpc_common.go", "grpc.pb.go", "grpc_grpc.pb.go"}),
+		NewGeneralTestCase("grpc-client-stream-test", grpc_module_name, "v1.44.0", "", "1.21", "", TestGrpcClientStream),
 	)
 }
 
@@ -41,3 +42,10 @@ func TestGrpcStatus(t *testing.T, env ...string) {
 	env = append(env, "GOLANG_PROTOBUF_REGISTRATION_CONFLICT=warn")
 	RunApp(t, "test_grpc_fail_status", env...)
 }
+
+func TestGrpcClientStream(t *testing.T, env ...string) {
+	UseApp("grpc/v1.44.0")
+	RunGoBuild(t, "go", "build", "test_grpc_client_stream.go", "grpc_common.go", "grpc.pb.go", "grpc_grpc.pb.go")
+	env = append(env, "GOLANG_PROTOBUF_REGISTRATION_CONFLICT=warn")
+	RunApp(t, "test_grpc_client_stream", env...)
+}