Fix gRPC service/method URL path parsing discrepancies (#4135)

CHANGELOG.md

@@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`
 - Do not modify the origin request in RoundTripper in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4033)
 - Handle empty value of `OTEL_PROPAGATORS` environment variable the same way as when the variable is unset in `go.opentelemetry.io/contrib/propagators/autoprop`. (#4101)
+- Fix gRPC service/method URL path parsing discrepancies. (#4135)
 
 ### Deprecated
 

instrumentation/google.golang.org/grpc/otelgrpc/internal/parse.go

@@ -24,13 +24,21 @@ import (
 // ParseFullMethod returns a span name following the OpenTelemetry semantic
 // conventions as well as all applicable span attribute.KeyValue attributes based
 // on a gRPC's FullMethod.
+//
+// Parsing is consistent with grpc-go implementation:
+// https://github.com/grpc/grpc-go/blob/v1.57.0/internal/grpcutil/method.go#L26-L39
 func ParseFullMethod(fullMethod string) (string, []attribute.KeyValue) {
-	name := strings.TrimLeft(fullMethod, "/")
-	service, method, found := strings.Cut(name, "/")
-	if !found {
+	if !strings.HasPrefix(fullMethod, "/") {
+		// Invalid format, does not follow `/package.service/method`.
+		return fullMethod, nil
+	}
+	name := fullMethod[1:]
+	pos := strings.LastIndex(name, "/")
+	if pos < 0 {
 		// Invalid format, does not follow `/package.service/method`.
 		return name, nil
 	}
+	service, method := name[:pos], name[pos+1:]
 
 	var attrs []attribute.KeyValue
 	if service != "" {

instrumentation/google.golang.org/grpc/otelgrpc/internal/parse_test.go

@@ -0,0 +1,70 @@
+// Copyright The OpenTelemetry Authors
+//
+// 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 internal // import "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal"
+
+import (
+	"testing"
+
+	"go.opentelemetry.io/otel/attribute"
+	semconv "go.opentelemetry.io/otel/semconv/v1.17.0"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseFullMethod(t *testing.T) {
+	cases := []struct {
+		input         string
+		expectedName  string
+		expectedAttrs []attribute.KeyValue
+	}{
+		{
+			input:        "no_slash/method",
+			expectedName: "no_slash/method",
+		},
+		{
+			input:        "/slash_but_no_second_slash",
+			expectedName: "slash_but_no_second_slash",
+		},
+		{
+			input:        "/service_only/",
+			expectedName: "service_only/",
+			expectedAttrs: []attribute.KeyValue{
+				semconv.RPCService("service_only"),
+			},
+		},
+		{
+			input:        "//method_only",
+			expectedName: "/method_only",
+			expectedAttrs: []attribute.KeyValue{
+				semconv.RPCMethod("method_only"),
+			},
+		},
+		{
+			input:        "/service/method",
+			expectedName: "service/method",
+			expectedAttrs: []attribute.KeyValue{
+				semconv.RPCService("service"),
+				semconv.RPCMethod("method"),
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.input, func(t *testing.T) {
+			name, attrs := ParseFullMethod(tc.input)
+			assert.Equal(t, tc.expectedName, name)
+			assert.Equal(t, tc.expectedAttrs, attrs)
+		})
+	}
+}

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

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
-	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/sdk/trace"
@@ -217,7 +216,7 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			eventsAttr: []map[attribute.Key]attribute.Value{},
 		},
 		{
-			method:      "serviceName/bar",
+			method:      "/serviceName/bar",
 			name:        "serviceName/bar",
 			interceptor: unaryInterceptor,
 			expectedAttr: []attribute.KeyValue{
@@ -240,7 +239,7 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			},
 		},
 		{
-			method:           "serviceName/bar_error",
+			method:           "/serviceName/bar_error",
 			name:             "serviceName/bar_error",
 			interceptor:      unaryInterceptor,
 			expectedSpanCode: codes.Error,
@@ -1056,64 +1055,3 @@ func TestStreamServerInterceptorEvents(t *testing.T) {
 		})
 	}
 }
-
-func TestParseFullMethod(t *testing.T) {
-	tests := []struct {
-		fullMethod string
-		name       string
-		attr       []attribute.KeyValue
-	}{
-		{
-			fullMethod: "/grpc.test.EchoService/Echo",
-			name:       "grpc.test.EchoService/Echo",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("grpc.test.EchoService"),
-				semconv.RPCMethod("Echo"),
-			},
-		}, {
-			fullMethod: "/com.example.ExampleRmiService/exampleMethod",
-			name:       "com.example.ExampleRmiService/exampleMethod",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("com.example.ExampleRmiService"),
-				semconv.RPCMethod("exampleMethod"),
-			},
-		}, {
-			fullMethod: "/MyCalcService.Calculator/Add",
-			name:       "MyCalcService.Calculator/Add",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("MyCalcService.Calculator"),
-				semconv.RPCMethod("Add"),
-			},
-		}, {
-			fullMethod: "/MyServiceReference.ICalculator/Add",
-			name:       "MyServiceReference.ICalculator/Add",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("MyServiceReference.ICalculator"),
-				semconv.RPCMethod("Add"),
-			},
-		}, {
-			fullMethod: "/MyServiceWithNoPackage/theMethod",
-			name:       "MyServiceWithNoPackage/theMethod",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("MyServiceWithNoPackage"),
-				semconv.RPCMethod("theMethod"),
-			},
-		}, {
-			fullMethod: "/pkg.srv",
-			name:       "pkg.srv",
-			attr:       []attribute.KeyValue(nil),
-		}, {
-			fullMethod: "/pkg.srv/",
-			name:       "pkg.srv/",
-			attr: []attribute.KeyValue{
-				semconv.RPCService("pkg.srv"),
-			},
-		},
-	}
-
-	for _, test := range tests {
-		n, a := internal.ParseFullMethod(test.fullMethod)
-		assert.Equal(t, test.name, n)
-		assert.Equal(t, test.attr, a)
-	}
-}