Add proposed changes

open-telemetry · Nov 20, 2023 · ef8cc54 · ef8cc54
1 parent 26f41ee
commit ef8cc54
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Fixed
 
-- Fix `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` sets error state from grpc status incorrectly (#4587)
+- Fix `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` to correctly set the span status depending on the gRPC status. (#4587)
 
 ## [1.21.1/0.46.1/0.15.1/0.1.1] - 2023-11-16
 

diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/config.go b/instrumentation/google.golang.org/grpc/otelgrpc/config.go
@@ -113,7 +113,7 @@ func newConfig(opts []Option, role string) *config {
 	}
 
 	c.rpcResponsesPerRPC, err = c.meter.Int64Histogram("rpc."+role+".responses_per_rpc",
-		metric.WithDescription("Measures the number of messages responded per RPC. Should be 1 for all non-streaming RPCs."),
+		metric.WithDescription("Measures the number of messages received per RPC. Should be 1 for all non-streaming RPCs."),
 		metric.WithUnit("{count}"))
 	if err != nil {
 		otel.Handle(err)

diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go b/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go
@@ -85,7 +85,8 @@ func (h *serverHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) cont
 
 // HandleRPC processes the RPC stats.
 func (h *serverHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
-	h.handleRPC(ctx, rs, true)
+	isServer := true
+	h.handleRPC(ctx, rs, isServer)
 }
 
 type clientHandler struct {
@@ -121,7 +122,8 @@ func (h *clientHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) cont
 
 // HandleRPC processes the RPC stats.
 func (h *clientHandler) HandleRPC(ctx context.Context, rs stats.RPCStats) {
-	h.handleRPC(ctx, rs, false)
+	isServer := false
+	h.handleRPC(ctx, rs, isServer)
 }
 
 // TagConn can attach some information to the given context.
@@ -137,7 +139,7 @@ func (h *clientHandler) HandleConn(context.Context, stats.ConnStats) {
 	// no-op
 }
 
-func (c *config) handleRPC(ctx context.Context, rs stats.RPCStats, isServer bool) {
+func (c *config) handleRPC(ctx context.Context, rs stats.RPCStats, isServer bool) { // nolint: revive  // isServer is not a control flag.
 	span := trace.SpanFromContext(ctx)
 	gctx, _ := ctx.Value(gRPCContextKey{}).(*gRPCContext)
 	var messageId int64

diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go
@@ -891,7 +891,7 @@ func checkClientMetrics(t *testing.T, reader metric.Reader) {
 			},
 			{
 				Name:        "rpc.client.responses_per_rpc",
-				Description: "Measures the number of messages responded per RPC. Should be 1 for all non-streaming RPCs.",
+				Description: "Measures the number of messages received per RPC. Should be 1 for all non-streaming RPCs.",
 				Unit:        "{count}",
 				Data: metricdata.Histogram[int64]{
 					Temporality: metricdata.CumulativeTemporality,
@@ -1244,7 +1244,7 @@ func checkServerMetrics(t *testing.T, reader metric.Reader) {
 			},
 			{
 				Name:        "rpc.server.responses_per_rpc",
-				Description: "Measures the number of messages responded per RPC. Should be 1 for all non-streaming RPCs.",
+				Description: "Measures the number of messages received per RPC. Should be 1 for all non-streaming RPCs.",
 				Unit:        "{count}",
 				Data: metricdata.Histogram[int64]{
 					Temporality: metricdata.CumulativeTemporality,

diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/stats_handler_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/stats_handler_test.go
@@ -114,7 +114,7 @@ func assertStatsHandlerServerMetrics(t *testing.T, reader metric.Reader, service
 			},
 			{
 				Name:        "rpc.server.responses_per_rpc",
-				Description: "Measures the number of messages responded per RPC. Should be 1 for all non-streaming RPCs.",
+				Description: "Measures the number of messages received per RPC. Should be 1 for all non-streaming RPCs.",
 				Unit:        "{count}",
 				Data: metricdata.Histogram[int64]{
 					Temporality: metricdata.CumulativeTemporality,